# Tasks 8, 9, 10 -- Audit Report

**Summary:** Tasks 8-10 have significant gaps in wiring, missing handlers, unaccounted dependencies, and lost behavior. Task 8 omits several handlers and the critical dispatch-to-Effect bridge. Task 9 omits PushNotificationManager and scanner dependencies. Task 10 omits critical error-recovery behavior and has no mechanism to obtain the HTTP server instance from the Layer system.

---

## Findings

| # | Category | Action | Issue | File:Line | Amendment / Question |
|---|----------|--------|-------|-----------|----------------------|
| 1 | Missing Wiring | Amend Plan | **Task 8: No dispatch bridge specified.** The plan says "IPC dispatch Layer calls `runtime.runPromise(handler(args))`" but never specifies HOW. Currently `makeIpcServerLive` receives `DaemonIPCContext` (a bag of closures) and passes it to `startIPCServer` which calls `buildIPCHandlers(ctx, getStatus)` which returns `IPCHandlerMap` consumed by `createCommandRouter`. Task 8 must either: (a) create an `IpcDispatchLive` Layer that builds the `IPCHandlerMap` by wrapping each Effect handler in `runtime.runPromise`, OR (b) modify `makeIpcServerLive` to accept Effect handlers directly. Neither is specified. | `src/lib/effect/daemon-layers.ts:231-243`, `src/lib/daemon/daemon-ipc.ts:121-124` | Add a concrete step: create `IpcDispatchLive` Layer that constructs `IPCHandlerMap` by wrapping each Effect handler in `Effect.runPromise`. Show how `makeIpcServerLive` signature changes to accept this instead of `DaemonIPCContext`. |
| 2 | Missing Wiring | Amend Plan | **Task 8: `getStatus` handler omitted.** The plan's code snippet shows `handleAddProject`, `handleRemoveProject`, `handleSetPinHash`, `handleSetKeepAwake` but omits `getStatus` which is complex (reads port, host, tailscaleIP, sessionCount, clientCount, pinEnabled, tlsEnabled, keepAwake, project list with statuses). This is the most state-heavy handler and needs explicit design. | `src/lib/effect/daemon-main.ts:720-761` | Add `handleGetStatus` Effect that reads `DaemonConfigRefTag`, `ProjectRegistryTag`, and `DaemonLifecycleContext` (for clientCount). Document which Tags provide each piece of data. |
| 3 | Missing Wiring | Amend Plan | **Task 8: Missing handlers in plan.** Plan shows 4 handlers as examples, says "similar for all other handlers." But `IPCHandlerMap` has 19 handlers including complex ones: `setKeepAwakeCommand` (creates new KeepAwake instance), `instanceAdd` (generates unique ID with collision avoidance), `restartWithConfig` (applies config then schedules shutdown), `setPin` (hashes PIN via dynamic import). At minimum, the plan should enumerate all 19 and flag the non-trivial ones. | `src/lib/daemon/daemon-ipc.ts:73-111` | Add enumeration of all 19 IPCHandlerMap entries. Flag `setKeepAwakeCommand`, `instanceAdd`, `setPin`, `restartWithConfig`, `getStatus` as requiring explicit implementation notes. |
| 4 | Incorrect Code | Amend Plan | **Task 8: `handleSetKeepAwake` does not interact with KeepAwake Layer.** The plan's code only updates `DaemonConfigRef` and publishes `ConfigChanged`. But the imperative code calls `keepAwakeManager?.setEnabled(enabled)` and returns `{ supported, active }`. The `KeepAwakeLive` Layer (keep-awake-layer.ts) has `activate()`/`deactivate()`/`isActive()`/`isSupported()` methods but does NOT watch `DaemonConfigRef`. The handler must call `KeepAwakeTag` directly. | `src/lib/effect/daemon-main.ts:993-1000`, `src/lib/effect/keep-awake-layer.ts:62-107` | Amend `handleSetKeepAwake` to: `yield* KeepAwakeTag` then call `activate()`/`deactivate()` based on `enabled`, query `isSupported()`/`isActive()` for the return value. |
| 5 | Incorrect Code | Amend Plan | **Task 8: `handleSetKeepAwakeCommand` is far more complex than plan suggests.** The imperative code (lines 1002-1016) deactivates the old manager, creates an entirely NEW `KeepAwake` instance with new command/args, wires error handler, re-activates if needed. This is a full service replacement, not just a config update. The Effect equivalent needs to either: recreate KeepAwakeLive somehow, or the KeepAwake service needs a `reconfigure(command, args)` method. | `src/lib/effect/daemon-main.ts:1002-1016` | Add design note: either extend `KeepAwakeService` interface with `reconfigure(command, args)` method, or use `ScopedRef` to swap the KeepAwake service. This is a design decision. |
| 6 | Missing Wiring | Amend Plan | **Task 9: PushNotificationManager dependency omitted.** The relay factory (lines 556-625) conditionally passes `pushManager` to `createProjectRelay`. Task 9's `RelayFactoryLive` code snippet mentions only `DaemonConfigRefTag` and `ProjectRegistryTag` but not `PushNotificationManager`, `VersionChecker` (`getCachedUpdate`), or `PortScanner` (`triggerScan`). These are all passed to `createProjectRelay`. | `src/lib/effect/daemon-main.ts:606-621` | Add dependencies to RelayFactoryLive: PushNotificationManagerTag (new or existing), VersionCheckerTag (for getCachedUpdate), PortScannerTag (for triggerScan). Or ask user if PushNotificationManager should be converted to a Layer in this phase. |
| 7 | Missing Wiring | Amend Plan | **Task 9: relay factory needs `httpServer` reference.** `buildRelayFactory` passes `ctx.httpServer!` to `createProjectRelay` (line 571). Task 9's `RelayFactoryLive` has no mechanism to obtain the HTTP server instance. This is the same problem as Task 10 finding #10. | `src/lib/effect/daemon-main.ts:571` | RelayFactoryLive must depend on an `HttpServerRefTag` or equivalent that provides the `http.Server` instance. |
| 8 | Missing Wiring | Amend Plan | **Task 9: relay factory creates and manages PersistenceLayer.** Lines 564-568 open a SQLite persistence DB per project and register an abort listener to close it. This lifecycle management is not mentioned in the plan. | `src/lib/effect/daemon-main.ts:564-568` | Add step: persistence DB lifecycle must be managed as a scoped resource within the relay, with cleanup on relay teardown. |
| 9 | Fragile Code | Amend Plan | **Task 9: relay factory has AbortSignal-based lifecycle.** The existing factory returns `(signal: AbortSignal) => Promise<ProjectRelay>`. The plan's Effect version returns `Effect.Effect<ProjectRelay>` with no mention of how cancellation/cleanup maps. The `ProjectRegistry.startRelay` passes an AbortSignal from an `AbortController` it owns. The Effect version needs `Scope`-based cleanup instead. | `src/lib/effect/daemon-main.ts:561` | Document how AbortSignal-based cleanup converts to Effect Scope. The factory's return type should be a scoped Effect that adds finalizers for persistence DB and relay cleanup. |
| 10 | State Issues | Amend Plan | **Task 10: No `HttpServerRefTag` exists.** `WebSocketRoutingLive` needs the `http.Server` instance to call `.on("upgrade", ...)`. Currently, the HTTP server is created inside `startHttpServer(ctx)` which mutates `ctx.httpServer`. The plan mentions `HttpServerRefTag` in Task 11 but Task 10 depends on it. Task 10 runs BEFORE Task 11. | `src/lib/daemon/daemon-lifecycle.ts:370-391`, plan Task 11 | Either (a) move `HttpServerRefTag` creation to an earlier task (Task 10 or a new sub-task), or (b) reorder Task 10 to run after Task 11, or (c) have Task 10 read from `DaemonLifecycleContext.httpServer` as a temporary bridge. |
| 11 | Fragile Code | Amend Plan | **Task 10: ProjectDiscoveryLive loses retry-on-error-state logic.** The imperative `discoverProjects` (lines 1501-1507) iterates all projects in "error" state and resets them with `addWithoutRelay(..., { silent: true })` for lazy retry. The plan's `ProjectDiscoveryLive` only shows the happy-path discovery fork with a `catchAll` that logs and swallows. The error-state reset loop is critical for reliability. | `src/lib/effect/daemon-main.ts:1501-1507` | Add error-state project reset logic to `ProjectDiscoveryLive` implementation notes. |
| 12 | Fragile Code | Amend Plan | **Task 10: WebSocket routing loses `503 Service Unavailable` response.** The imperative handler (lines 1264-1269) writes an HTTP 503 response before destroying the socket when relay startup fails. The plan's sketch only mentions `catchAll` with logging. The 503 response is important for client-side retry behavior. | `src/lib/effect/daemon-main.ts:1264-1268` | Add explicit error handling: on relay wait failure, write `HTTP/1.1 503 Service Unavailable\r\n\r\n` to socket before destroying. |
| 13 | Fragile Code | Amend Plan | **Task 10: WebSocket routing loses `waitForRelay` with timeout.** The imperative code calls `registry.waitForRelay(slug, 10_000)` which waits up to 10 seconds for a relay to become ready. The plan shows no equivalent waiting mechanism in the Effect version. | `src/lib/effect/daemon-main.ts:1250` | Add relay wait logic: either use `Effect.timeout` with a `Deferred`-based wait on `ProjectReady` event from `DaemonEventBus`, or port the `waitForRelay` method to the Effect ProjectRegistry service. |
| 14 | Fragile Code | Amend Plan | **Task 10: WebSocket routing loses `ensureRelayStarted` call.** Before waiting for a relay, the imperative code calls `ensureRelayStarted(slug)` (line 1249) which lazily starts the relay if not already running. The plan does not mention this behavior. | `src/lib/effect/daemon-main.ts:627-655, 1249` | Add lazy relay startup to WebSocketRoutingLive: before waiting for relay, check registry state and start relay if not already started/starting. |
| 15 | Missing Wiring | Amend Plan | **Task 10: Session prefetch not accounted for.** The plan title mentions "Session Prefetch" but the implementation sketch only shows `WebSocketRoutingLive` and `ProjectDiscoveryLive`. No `SessionPrefetchLive` code or tests are provided. The imperative session prefetch (lines 1342-1375) includes instance password resolution, Basic auth header construction, and fire-and-forget fetches. | `src/lib/effect/daemon-main.ts:1341-1375` | Add `SessionPrefetchLive` Layer sketch with: instance credential resolution from `InstanceManagerTag`, auth header construction, `Effect.forkScoped` for fire-and-forget fetch loop. |
| 16 | Missing Wiring | Amend Plan | **Task 10: Instance status listener not accounted for.** Lines 1377-1387 register `instanceManager.on("status_changed", ...)` to reset error-state projects when an instance becomes healthy. This cross-service wiring is not mentioned in Task 10 or any other task. | `src/lib/effect/daemon-main.ts:1378-1387` | Add a scoped fiber in Task 10 (or a separate sub-task) that subscribes to `DaemonEventBus` for `InstanceStatusChanged` events and resets error-state projects. |
| 17 | Implicit Assumptions | Ask User | **Task 9: `createProjectRelay` is an imperative async function with complex dependencies.** The relay factory passes 15+ fields to `createProjectRelay` including `getProjects`, `addProject`, `removeProject`, `setProjectTitle`, `getInstances`, etc. -- essentially duplicating the IPC context. Converting this requires either: (a) making `createProjectRelay` accept Effect services, or (b) building imperative shims from Effect services. Which approach is intended? | `src/lib/effect/daemon-main.ts:570-623` | Design decision needed: does Task 9 also convert `createProjectRelay` to Effect, or does it build imperative callback shims that delegate to Effect services? The latter is simpler but creates a new boundary. |
| 18 | Insufficient Test Coverage | Amend Plan | **Task 8: No testing strategy for 19 handlers.** The plan says "Write tests for each handler" but provides no test structure or guidance on which handlers need integration-level testing vs unit testing. The `setKeepAwakeCommand` handler (which replaces a service) and `instanceAdd` (with ID collision avoidance) are particularly tricky. | N/A | Add testing guidance: group handlers into (1) simple state updates (unit test with mock Refs), (2) complex logic like `instanceAdd` ID generation (dedicated test cases), (3) service interaction like `setKeepAwakeCommand` (needs mock KeepAwakeTag). |
| 19 | Insufficient Test Coverage | Amend Plan | **Task 10: No auth failure path tests specified.** The plan says to create `test/unit/effect/ws-routing-layer.test.ts` but provides no test cases. Auth checking has 3 paths: no pin required, valid cookie, valid pin header. Socket destruction on auth failure needs explicit test coverage. | N/A | Add test cases: (1) upgrade succeeds when no PIN set, (2) upgrade succeeds with valid cookie, (3) upgrade rejected and socket destroyed when auth fails, (4) upgrade rejected when `shuttingDown` is true. |
| 20 | Fragile Code | Amend Plan | **Task 10: `discoverProjects` uses `dismissedPaths` from mutable state.** Line 1492 checks `dismissedPaths.has(normalizedDir)`. In the Effect version, this must come from `DaemonConfigRefTag.dismissedPaths`. The plan does not mention this. | `src/lib/effect/daemon-main.ts:1492` | Add note: `ProjectDiscoveryLive` must read `dismissedPaths` from `DaemonConfigRefTag` to filter discovered projects. |

---

**No issues found in:** (none -- all audited categories had findings)

## Key Risks

1. **Task ordering problem (Finding #10):** Task 10 needs `HttpServerRefTag` which is only introduced in Task 11. This is a hard blocker -- either introduce the Tag earlier or accept that Task 10 will use the imperative `DaemonLifecycleContext` as a bridge.

2. **Relay factory is the most complex conversion (Findings #6-9, #17):** `buildRelayFactory` closes over 15+ dependencies and passes them as callbacks to `createProjectRelay`. This is not a simple "read from Context" conversion -- it requires a design decision about whether `createProjectRelay` itself gets converted.

3. **KeepAwake reconfiguration (Finding #5):** The imperative code creates a NEW `KeepAwake` instance with different command/args. The current `KeepAwakeLive` Layer has no reconfiguration mechanism. This needs architectural resolution before implementation.
