# Phase 8 Re-Audit: Tasks 6-7 (ProjectRegistry + InstanceManager) -- Post-Amendment

## Summary

The AP-18/AP-19 amendments correctly identified that both services are already implemented as Effect modules and rewrote the tasks as "wire + fill gaps." However, the gap lists are incomplete (missing 5+ methods from InstanceManagementDeps, missing `isStarting`), and the PortScanner wiring approach (AP-19 step 5) will fail at the type level because `PortScannerConfig` callbacks require `Effect.Effect<void, never, never>` (no requirements), so they cannot access Tags. A cross-cutting wiring Layer is needed for the DaemonEventBus bridge (AP-19 step 6) but its home is unspecified.

## Findings

| # | Category | Action | Issue | File:Line | Amendment / Question |
|---|----------|--------|-------|-----------|----------------------|
| 1 | Missing Wiring | Amend Plan | `makeProjectRegistryLive()` only provides `ProjectRegistryTag` (a bare `Ref`). The free functions (`addWithoutRelay`, `remove`, `startRelay`, etc.) also require `DaemonEventBusTag` and `RelayCacheTag` at call sites. Plan step 4 says "wire into `makeDaemonLive`" but does not note these are required co-layers. `DaemonEventBusLive` is already composed (line 384) and `makeRelayCacheLayer` is conditionally composed (lines 394-398), but `relayFactory` is optional in `DaemonLiveOptions`. If ProjectRegistry is wired, `relayFactory` must become mandatory or `startRelay` calls will fail at runtime with missing `RelayCacheTag`. | `project-registry-service.ts:490`, `daemon-layers.ts:394-398` | Add note: "When ProjectRegistryLive is wired, `relayFactory` in DaemonLiveOptions must be mandatory (or startRelay must be guarded). `DaemonEventBusLive` and `RelayCacheLayer` are required co-layers." |
| 2 | Missing Wiring | Amend Plan | AP-18 step 3 lists missing methods: `broadcastToAll`, `waitForRelay`, `evictOldestSessions`, `replaceRelay`. But `isStarting` (`project-registry.ts:286`) is also used at `daemon-main.ts:632` and has no Effect equivalent. It checks whether a relay creation is in-flight. | `daemon-main.ts:632`, `project-registry.ts:286` | Add `isStarting` to the missing methods list, or document replacement: check `getEntry(slug)` returns `_tag === "Registering"`. |
| 3 | Implicit Assumptions | Amend Plan | AP-19 step 3 says "Create `InstanceMgmtLive` adapter to `InstanceManagementDeps` interface." The `InstanceManagementDeps` interface requires 7 methods: `getInstances`, `addInstance`, `removeInstance`, `startInstance`, `stopInstance`, `updateInstance`, `persistConfig`. The existing Effect service only implements 4 of these (`addInstance`, `removeInstance`, `getInstance`, `getInstances`). Missing: `startInstance`, `stopInstance`, `updateInstance`, `persistConfig`. The plan does not list these as gaps to fill. | `instance-manager-service.ts` (entire file), `handlers/types.ts:26-37` | Add to AP-19: "Implement missing methods before building adapter: `startInstance` (spawn process), `stopInstance` (interrupt/kill process), `updateInstance` (partial field update on Ref state), `persistConfig` (delegate to ConfigPersistence Layer from Task 2)." |
| 4 | Implicit Assumptions | Amend Plan | Effect `addInstance` returns `void` on success (line 122-168). `InstanceManagementDeps.addInstance` returns `OpenCodeInstance` synchronously. The adapter cannot return the created instance without reading it back from state. | `instance-manager-service.ts:122-168`, `handlers/types.ts:28` | Amend: modify Effect `addInstance` to return the created `OpenCodeInstance`, or note that the adapter must chain `addInstance` then `getInstance` to satisfy the return type. |
| 5 | Missing Wiring | Amend Plan | `daemon-main.ts` uses `instanceManager.getExternalUrl(id)` (line 397) and `instanceManager.getInstanceUrl(id)` (lines 544, 550). Neither exists in the Effect service nor in `InstanceManagementDeps`. These are not mentioned in the plan at all. | `daemon-main.ts:397,544,550` | Add to gap-fill list or document as pure helper functions that derive URL from `instance.port`. |
| 6 | Fragile Code | Amend Plan | AP-19 step 5 says "construct `onDiscovered/onLost` callbacks that call InstanceManager via Tags." This will fail at the type level. `PortScannerConfig.onDiscovered` and `onLost` are typed as `(port: number) => Effect.Effect<void>` which is `Effect.Effect<void, never, never>` -- zero requirements. You cannot use `yield* InstanceManagerStateTag` inside these callbacks because that adds `InstanceManagerStateTag` to the R type parameter. The callbacks must be closures that close over concrete `Ref` / `FiberMap` values obtained during Layer construction. | `port-scanner-layer.ts:20-27` | Amend step 5: "Construct `onDiscovered`/`onLost` as closures that close over the `Ref` and `FiberMap` values obtained via `yield* InstanceManagerStateTag` and `yield* PollerFibersTag` BEFORE constructing the `PortScannerConfig`. Do NOT use Tag access inside the callback bodies." |
| 7 | State Issues | Amend Plan | AP-19 step 6 says "Use `DaemonEventBus` for instance->registry wiring (subscribe to `InstanceStatusChanged`, reset error-state projects) -- avoids circular Layer dependency." This is correct in principle, but the plan does not specify WHERE the subscription fiber lives. If it's in `InstanceManagerLive`, it creates an implicit dependency on `ProjectRegistryTag`. If it's in `ProjectRegistryLive`, it creates an implicit dependency on `InstanceManagerStateTag`. The subscription fiber also needs scope management. | N/A | Amend: specify a dedicated `DaemonWiringLayer` (or equivalent) that sits above both services, subscribes to `DaemonEventBus`, and handles cross-cutting instance-to-registry concerns. This avoids circular Layer deps. |
| 8 | Implicit Assumptions | Amend Plan | `daemon-main.ts:779` calls `instanceManager.drain()`. The Effect service has no `drain` method. This is handled implicitly by scope finalization (`FiberMap.make` is scoped), but the plan should document this explicitly so implementers know not to add a `drain` method. | `daemon-main.ts:779`, `instance-manager-service.ts:102-108` | Add note: "`drain()` is replaced by scope-based finalization -- `FiberMap.make` is scoped, fibers interrupted on Layer scope close. No explicit `drain` method needed." |
| 9 | Missing Wiring | Amend Plan | AP-19 step 8 says "Add `PushManagerLive` Layer (user decision: create Layer using existing `PushManagerTag`)." `PushManagerLive` already exists as a complete Layer factory at `push-service.ts:37` taking `PushManagerConfig`. The plan implies creating a new Layer. | `push-service.ts:37` | Amend: "Wire existing `PushManagerLive` from `push-service.ts` into `makeDaemonLive`, providing a concrete `sendPush` implementation." |
| 10 | Implicit Assumptions | Accept | Effect `getInstances` returns `Iterable` (HashMap.values) but `InstanceManagementDeps.getInstances` returns `ReadonlyArray`. The adapter will need `Array.from()`. Straightforward, no plan change needed. | `instance-manager-service.ts:214-218`, `handlers/types.ts:27` | -- |
| 11 | Fragile Code | Accept | `touchLastUsed` in `project-registry-service.ts:385` uses `Date.now()` directly instead of `Clock.currentTimeMillis` used elsewhere. Makes it untestable with Effect's test clock. Pre-existing, not introduced by amendments. | `project-registry-service.ts:385` | -- |

## No issues found in:

(All assigned categories had findings.)
