[AI:Finding] F20260806-63E2D28A: Daemon's top-level cycle error boundary discards the error object entirely — a crashed cycle is invisible

.operator/data/findings/F20260806-63E2D28A.md

@@ -2,12 +2,12 @@
 id: F20260806-63E2D28A
 kind: finding
 title: Daemon's top-level cycle error boundary discards the error object entirely — a crashed cycle is invisible
-status: pending
+status: in-progress
 priority: 2
 source: code-quality#FINDING-001
 created_at: '2026-08-06T20:45:49Z'
+started_at: "2026-08-06T23:03:44Z"
 ---
-
 **Severity**: high
 **Priority**: 2
 **Files Affected**: 2

.operator/data/tasks/T20260806-3A261A44.md

@@ -0,0 +1,86 @@
+---
+id: T20260806-3A261A44
+kind: task
+title: Finalize the cycle execution row as failed when Engine.runOnce throws
+status: pending
+priority: 2
+created_at: '2026-08-06T23:03:44Z'
+parent_id: F20260806-63E2D28A
+---
+
+# Finalize the cycle execution row as failed when `Engine.runOnce` throws
+
+## Problem
+
+`Engine.runOnce` (`engine/engine/engine.ts:118-197`) opens a cycle execution
+row with `status: "running"` at `engine.ts:139` and finalizes it at
+`engine.ts:185`, but has no try/catch between them. Any throw from
+`cycleHistory.start` (KV/SQLite write), `enumerateRepos` (`engine.ts:154`),
+`guard.acquire` (`engine.ts:221`, SQLite write), or an event-bus handler
+propagates out and skips the finalize entirely.
+
+Result: the `executions/cycle-*` row stays `running` forever with no error
+text, so the App UI shows a cycle stuck in-flight. Because the caller's
+boundary is also lossy, there is currently no second source of truth for what
+failed.
+
+This is inconsistent with the sibling path in the same file: workspace-prep
+failures at `engine.ts:284-325` are caught, logged at ERROR, and written as a
+synthetic `executions/workspace-prep-*` row finalized `status: "failed"` with
+the error text. The cycle row deserves the same treatment — and the errors
+that escape `runOnce` are precisely the *unexpected* ones, since the expected
+ones are already handled.
+
+Violates the `intelligence/rules/migration.md` §Quality Gates invariant
+"Every stage execution produces an `executions/{id}` KV entry" in spirit: a
+row that never reaches a terminal status is not an observable record.
+
+## Solution
+
+Wrap the body of `runOnce` (from after `cycleHistory.start` through the
+existing finalize) so that a throw:
+
+1. Logs ERROR via `this.deps.log?.error` with the message, the `.cause`
+   chain, `traceId`, and the elapsed `durationMs` — matching the shape at
+   `engine.ts:287-289`.
+2. Calls `cycleHistory.finalize({ finishedAt, durationMs, status: "failed",
+   error: message, successScore: 0, summary: ... }, ctx)` — the
+   `ExecutionFinalizeFields` contract at
+   `engine/pipeline/primitives/execution-history.ts:101-122` already carries
+   `error` and `successScore`.
+3. Wraps that finalize in its own best-effort try/catch so an observability
+   write failure never masks the original error — mirror the existing
+   `} catch { /* best-effort */ }` at `engine.ts:321-324`.
+4. Rethrows the original error unchanged, so the daemon boundary and the
+   `--once` exit code behaviour are preserved.
+
+Do not change the success path: the `status: failed === 0 ? "completed" : "failed"`
+computation at `engine.ts:188` stays as-is.
+
+## Regression test
+
+In `engine/engine/engine.test.ts`, add a test named for the bug scenario,
+e.g. `"finalizes the cycle execution row as failed when the cycle throws"`:
+build an `Engine` with the existing `FakeKV` (`engine.test.ts:59`) and an
+`enumerateRepos` (or `guard.acquire`) that rejects; assert `runOnce` rejects
+AND that the `executions/cycle-*` row in `kv.rows` has `status: "failed"`
+with non-empty `error`. The lookup pattern at `engine.test.ts:427`
+(`[...kv.rows.entries()].find(([key]) => key.startsWith("executions/..."))`)
+is the model. The test must fail on pre-fix code (the row is `running`).
+
+## Affected Files
+
+- `engine/engine/engine.ts:118-197` — wrap the cycle body; on throw, ERROR-log and finalize the cycle row as `failed` with the error text, then rethrow
+- `engine/engine/engine.test.ts` — regression test asserting the cycle row reaches `failed`, using the existing `FakeKV`
+
+## Acceptance Criteria
+
+- A cycle that throws leaves its `executions/{id}` row in `status: "failed"` with the error text populated, not `running`
+- The original error still propagates out of `runOnce` unchanged — the daemon boundary and the `--once` exit-code path are unaffected
+- A failure inside the error-path `finalize` does not mask the original error
+- The regression test fails against the current no-try/catch `runOnce`
+- An ERROR log line is added for the cycle failure carrying message, `.cause` chain, `traceId`, and `durationMs`; the existing INFO lines at `engine.ts:128`, `156`, `192` are preserved
+- Coverage on `engine/engine/engine.ts` stays >=90%
+- `engine.ts` stays within the 300-line cap on code lines (logging, comments, and JSDoc are excluded from the count)
+- `npm run typecheck && npm run lint && npm test` passes from the repo root
+

.operator/data/tasks/T20260806-A9673078.md

@@ -0,0 +1,126 @@
+---
+id: T20260806-A9673078
+kind: task
+title: Log daemon cycle failures and scheduler job rejections instead of discarding them
+status: pending
+priority: 2
+created_at: '2026-08-06T23:03:44Z'
+parent_id: F20260806-63E2D28A
+---
+
+# Log daemon cycle failures and scheduler job rejections instead of discarding them
+
+## Problem
+
+The daemon's outermost error boundary destroys every failure it catches.
+
+1. `engine/daemon/daemon.ts:196` is a bare `} catch {` — the thrown error is
+   never bound, so message, stack, and `.cause` are lost. The only surviving
+   side effects are `this.health.recordCycle(false)` and a status-line flag,
+   which reduce an arbitrary failure to the single word `failure`.
+2. The class could not log it even if the error were bound: the logger
+   dependency is typed `{ info: (msg: string) => void }` at `daemon.ts:51`,
+   so `.warn` / `.error` are structurally unreachable — even though the
+   composition root already passes the full `Logger` at `engine/entry.ts:541`.
+   Every neighbouring branch (`daemon.ts:117`, `122`, `125`, `168`, `210`)
+   logs via `this.log?.info`; the failure path is the sole unlogged one.
+3. `engine/daemon/scheduler.ts:40` discards the job callback's rejection the
+   same way, and `IntervalScheduler` holds no logger at all, so it cannot
+   back-stop the boundary above it. Its skipped-tick branch
+   (`scheduler.ts:36`) is likewise an unlogged decision, unlike the
+   equivalent guard at `daemon.ts:167-168` whose comment states
+   "a dropped tick is a DECISION, never silent."
+
+In daemon mode the process keeps ticking every `cycleIntervalMs` and failing
+invisibly for as long as it is left running. In `--once` mode
+(`engine/entry.ts:621-628`) it exits 1 printing only `Cycle complete: failure`,
+giving CI/cron callers a red exit with zero diagnostics.
+
+Violates `intelligence/rules/typescript.md` §REQUIRED (Observability) —
+"Every catch-and-continue branch MUST warn with the reason" and "ERROR —
+every failure, with full error including `.cause`" — and the §Quality Gates
+observability BLOCKER in `intelligence/rules/migration.md`.
+
+## Solution
+
+### 1. Widen the `Daemon` logger dependency
+
+Change `daemon.ts:51` from `{ info: (msg: string) => void }` to the full
+`Logger` from `../logging/logger.js` (the layer graph allows
+`daemon/ → logging/`; `entry.ts:541` already passes exactly that). Keep it
+optional (`log?: Logger`) so existing tests that omit it still compile.
+
+### 2. Bind and log the cycle error
+
+Replace the bare catch at `daemon.ts:196` with `catch (err)` and emit an
+ERROR line before `this.health.recordCycle(false)`, carrying:
+- the error message,
+- the full `.cause` chain (walk `err.cause` and join, do not stop at depth 1),
+- `cycle: this.cycleCount`, `traceId: ctx.traceId`, and the cycle duration
+  (capture `Date.now()` at the top of the `try`).
+
+Follow the shape already used at `engine/engine/engine.ts:287-289`
+(`log?.error(msg, { traceId, repoId, errorCode, durationMs })`).
+
+### 3. Give `IntervalScheduler` a logger
+
+Add an optional constructor parameter `log?: Logger` to `IntervalScheduler`.
+
+**Note**: `this.scheduler = new IntervalScheduler()` at `daemon.ts:37` is a
+class-field initializer and runs *before* the constructor parameter
+properties are assigned, so `this.log` is `undefined` there. Move the
+instantiation into the constructor body next to the existing
+`this.health = new HealthMonitor(...)` at `daemon.ts:54` and pass `this.log`.
+`IntervalScheduler` is same-layer (`daemon/`), so this does not conflict with
+the "composition root is `entry.ts` only" rule — the `new` is already there today.
+
+Then in `schedule()`:
+- `scheduler.ts:36` — log WARN naming `job.id` before returning from a
+  skipped tick (unexpected-but-non-fatal; mirrors `daemon.ts:167-168`).
+- `scheduler.ts:40` — bind the error and log ERROR naming `job.id`, the
+  message, and the `.cause` chain. Keep the swallow (the daemon must not
+  crash from a job error) but replace the justifying comment with one that
+  states the error is now reported.
+
+### 4. Regression tests
+
+In `engine/daemon/daemon.test.ts`, add a test named for the bug scenario,
+e.g. `"logs an ERROR with the cause chain when the engine cycle throws"`:
+construct a `Daemon` with a fake logger capturing `error` calls and an
+`engine.runOnce` that rejects with
+`new Error("outer", { cause: new Error("inner") })`; assert the ERROR line
+contains both `"outer"` and `"inner"`. The existing fake at
+`daemon.test.ts:16-20` (`makeEngine`) and the log fakes used at
+`daemon.test.ts:198`, `221`, `246`, `276` are the patterns to follow — extend
+the log fake to capture `warn`/`error`, not just `info`.
+
+In `engine/daemon/scheduler.test.ts`, add two tests:
+- `"logs an ERROR naming the job id when a scheduled callback rejects"` —
+  tighten the existing `"catches callback errors without crashing"`
+  (`scheduler.test.ts:90-104`) into a separate assertion, or add alongside it.
+- `"logs a WARN when a tick is skipped because the previous run is still in flight"`
+  — reuse the fake-timer + slow-callback setup already at
+  `scheduler.test.ts:~60-88`.
+
+Both must fail against the pre-fix code (they will: no logger exists to assert on).
+
+## Affected Files
+
+- `engine/daemon/daemon.ts:51` — widen `log?` from `{ info }` to `Logger`
+- `engine/daemon/daemon.ts:37,54` — move `new IntervalScheduler()` into the constructor body, pass `this.log`
+- `engine/daemon/daemon.ts:182-199` — capture cycle start time; replace `} catch {` with `catch (err)` + ERROR log carrying message, `.cause` chain, cycle number, traceId, duration
+- `engine/daemon/scheduler.ts:21-53` — add optional `log?: Logger` constructor param; WARN on the skipped-tick branch (line 36); bind + ERROR-log the job rejection (line 40)
+- `engine/daemon/daemon.test.ts` — regression test for the cause-chain ERROR line; extend the log fake to capture `warn`/`error`
+- `engine/daemon/scheduler.test.ts` — regression tests for the job-rejection ERROR and the skipped-tick WARN
+- `engine/entry.ts:541` — verify only; already passes the full `Logger`, no change expected
+
+## Acceptance Criteria
+
+- A `Daemon` whose `engine.runOnce` rejects with an `Error` carrying a `.cause` produces an ERROR log line containing both the message and the cause; the test fails against the current bare `} catch {`
+- An `IntervalScheduler` job whose callback rejects produces an ERROR log naming the job id, and a tick skipped by the `running` guard produces a WARN; both tests fail against pre-fix code
+- `Daemon`'s injected logger type exposes `error`; no catch block in `engine/daemon/**` discards its error binding
+- The daemon still does not crash on a cycle failure — `recordCycle(false)`, the status-line flag, and the `finally` block behaviour are unchanged
+- Coverage on `engine/daemon/daemon.ts` and `engine/daemon/scheduler.ts` stays >=90%
+- `daemon.ts` and `scheduler.ts` stay within the 300-line cap (logging and comments are excluded from the count — do not strip comments to fit)
+- `npm run typecheck && npm run lint && npm test` passes from the repo root
+