From ffef26c1b5fced7eac801df23bd409cccf10d9a8 Mon Sep 17 00:00:00 2001 From: Raj D <25481060+radroid@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:29:25 -0400 Subject: [PATCH 01/21] docs(coil): resolve the #125 review findings and verify the plan's assumptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #125 listed twelve findings against the Loops design package, all of which had to land before Phase 2 is built from the plan. This resolves every one and re-verifies the assumptions the build depends on against the tree. Four of the fixes are latent bugs rather than contradictions. `deadlineAtMs` is now mandatory and non-nullable (the old null branch in guard 10b meant *no deference at all*, so a deadline-less loop fired on top of a healthy self-pacing thread). Stop conditions move from guard 13 to guard 4b, ahead of every non-consuming skip, because they were only evaluated on a tick that had already decided the thread was idle — so a busy self-paced run walked past its own deadline. Guard 5 is retired: upstream #8600 made settlement a server-side sweep with no provenance marker, the same correction autoResume/guards.ts already made after a timer destroyed an armed week-long resume. And `thread.pin` turns out to emit companion unsettled/unsnoozed events, so arming a snoozed thread would have cancelled the snooze. Costs are re-measured against merge-base 941acb4f9, and the result inverts the plan's intuition: Phase 5 falls from "0-3 rows [A]" to one row at risk 6, while Phase 4's settings entry rises to ~350 of the feature's ~411 total risk. The master toggle gained a data model and routes and moved from Phase 4 to Phase 2, since Phases 2 and 3 were otherwise unswitchable as ordered. build-report.mjs now asserts the prototype count in both directions. The old guard only fired at zero matches, so one drifted marker emitted a report missing a frame — invisible in the browser and in a 15k-line diff. Reproduced and fixed: a `>` in a hint now throws "inlined 7 prototypes but prototypes/ holds 8". TESTS.md goes 162 -> 183 cases, each addition marked [#125]. Co-Authored-By: Claude Opus 5 (1M context) --- docs/coil/loop/DESIGN.md | 38 +- docs/coil/loops-v2/BACKEND.md | 403 ++++++++++++++--- docs/coil/loops-v2/FINDINGS.md | 43 +- docs/coil/loops-v2/PLAN.md | 405 +++++++++++++----- docs/coil/loops-v2/TESTS.md | 119 ++++- docs/coil/loops-v2/UPSTREAM-DELTA.md | 101 +++++ docs/coil/loops-v2/build-report.mjs | 27 +- .../coil/loops-v2/prototypes/p4-settings.html | 9 +- .../prototypes/p7-console-shapes.html | 12 + docs/coil/loops-v2/prototypes/p8-mobile.html | 5 + docs/coil/loops-v2/report.html | 26 +- 11 files changed, 993 insertions(+), 195 deletions(-) diff --git a/docs/coil/loop/DESIGN.md b/docs/coil/loop/DESIGN.md index 69484e1a8ca4..93ddc6d5295f 100644 --- a/docs/coil/loop/DESIGN.md +++ b/docs/coil/loop/DESIGN.md @@ -1,20 +1,34 @@ # Loop Watch — design (radroid/t3code#38) -> **Superseded by [`docs/coil/loops-v2/`](../loops-v2/PLAN.md) (2026-08-17).** The re-checks the -> second paragraph below asks for are covered in -> [`loops-v2/UPSTREAM-DELTA.md`](../loops-v2/UPSTREAM-DELTA.md): #5219 as `ThreadBackgroundLiveness` -> in its §2, and #3638 confirmed still not on `upstream/main` in its §1. Three of this design's -> premises did not survive them. +> **Superseded by [`docs/coil/loops-v2/`](../loops-v2/PLAN.md) (2026-08-17).** > > **Status: designed, not built.** No `apps/server/src/coil/loop/` exists — every path this > document writes in the present tense is a proposal. It is archived here because the research -> underneath it is reusable, not because the feature shipped. +> underneath it is reusable, not because the feature shipped. The reactor detail is the part that +> survived best and is what `loops-v2/BACKEND.md` builds on. > -> Two things have moved since 2026-08-02 and must be re-checked before anyone builds from it. -> Upstream #5219 supersedes the subagent-tracking gap that motivated the issue, and upstream -> PR #3638 ships `schedule_task` / `delegate_task` MCP tools. The codebase line numbers cited -> throughout were measured against the 2026-08-02 merge-base and have since drifted through -> several upstream syncs — re-verify each one rather than trusting it. +> **Corrected 2026-09-02 (issue #125 §A6).** An earlier version of this banner named upstream +> **#5219** and PR **#3638** as the premises that "did not survive". That is backwards: both +> re-checks came back **confirming** — #5219 landed and is exactly the `ThreadBackgroundLiveness` +> the successor design composes with +> ([`UPSTREAM-DELTA.md`](../loops-v2/UPSTREAM-DELTA.md) §2), and #3638 was re-confirmed as **still +> not on `upstream/main`** (§1), which is why a fork-local scheduler is not a parallel path. +> +> The three premises that actually moved are in +> [`loops-v2/FINDINGS.md`](../loops-v2/FINDINGS.md) §A: +> +> - **§A1** — upstream shipped **thread pinning** two days after this design froze, so the sidebar +> affordance this document argues at length against building already exists. +> - **§A2** — `Sidebar.tsx` / `Sidebar.logic.ts` are **not** seam rows, which cuts both ways: a +> pinned thread is free, a bespoke row type is the most expensive kind of row this fork can take. +> - **§A3** — the settings mount this design targets (`BetaSettingsPanel.tsx`) **no longer exists**, +> so its "+2 lines, risk 6" costing is void. +> +> A fourth has moved since: upstream **inverted pin-vs-settle** (`f70eeeeb0`), so a pin no longer +> outranks settlement — see `UPSTREAM-DELTA.md` §9.1. +> +> The codebase line numbers cited throughout were measured against the 2026-08-02 merge-base and +> have since drifted through many upstream syncs — re-verify each one rather than trusting it. _Chosen 2026-08-02 from a 4-design / 12-judgement panel. Full alternatives in [OPTIONS.md](OPTIONS.md); codebase evidence in [RESEARCH.md](RESEARCH.md)._ @@ -26,7 +40,7 @@ _Chosen 2026-08-02 from a 4-design / 12-judgement panel. Full alternatives in [O | --------------------------- | ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | quiescence (Deadman) | 7.67 | WINNER. The only design with zero fatal flaws across all three lenses. Its trigger — `now - projection_threads.updated_at` — is the one signal every one of the twelve judgements independently named as the best idea worth stealing. I re-verified it: ProjectionPipeline.ts:794-808 groups `thread.activity-appended` with `thread.message-sent` and rewrites the row with `updatedAt: event.occurredAt`, and | | budget (RUNWAY) | 6.33 | Headline mechanism is dead; its product instinct survives. I confirmed the SRE's kill shot: `stopSessionInternal` calls `completeTurn(context, "interrupted", "Session stopped.")` at ClaudeAdapter.ts:3057, emitting a real-turnId `turn.completed`, so arming on any non-synthetic turn.completed restarts exactly the work the user just hit Stop on. I also confirmed the second hole at ClaudeAdapter.ts:19 | -| observability (Night Watch) | 6.33 | Right diagnosis, two mechanisms that do not exist. I read the user's own template at /Users/rajdholakia/.claude/skills/auto-loop-bootstrap/assets/templates/.loop/state.json: it is `{stage, iter, pr_mode, pr_size_policy, base_branch, backlog_source}` — there is no `status` key and no `remaining`/`backlog` array, and the companion CLAUDE.md:50 says outright `The loop NEVER halts on a semantic event` | +| observability (Night Watch) | 6.33 | Right diagnosis, two mechanisms that do not exist. I read the user's own template at `~/.claude/skills/auto-loop-bootstrap/assets/templates/.loop/state.json`: it is `{stage, iter, pr_mode, pr_size_policy, base_branch, backlog_source}` — there is no `status` key and no `remaining`/`backlog` array, and the companion CLAUDE.md:50 says outright `The loop NEVER halts on a semantic event` | | contract (BATON) | 6 | Loses on three independently-verified defects, all in the load-bearing mechanism. (1) The contract is read from the wrong directory on most threads: `resolveThreadWorkspaceCwd` (checkpointing/Utils.ts:21-27) returns `worktreePath` FIRST and only falls back to the project root, so on any worktree-backed thread the agent writes into the worktree while the supervisor stats the project root — three fa | # Loop Watch — `apps/server/src/coil/loop/` diff --git a/docs/coil/loops-v2/BACKEND.md b/docs/coil/loops-v2/BACKEND.md index 2c2f098beb01..655107502e8d 100644 --- a/docs/coil/loops-v2/BACKEND.md +++ b/docs/coil/loops-v2/BACKEND.md @@ -12,15 +12,19 @@ scheduler rather than replacing it**. Fork-owned, provider-agnostic. Line references were measured against merge-base `196c8ea0d` (2026-08-14 sync), not the 2026-08-02 base the archived design used; UPSTREAM-DELTA §5 lists the ones that have since moved. Churn and -file-size figures are re-measured against the current merge-base **`cebac353d`**. - -> **Corrected 2026-08-19 by review.** An earlier revision named `a4cc1367b` as the current -> merge-base and said the 2026-08-18 daily sync force-rewrote the fork onto the _same_ base. The -> base **moved**: two upstream commits (`3723722f7`, `cebac353d`) sit under the replayed fork stack, -> and `cebac353d` is an upstream ancestor, so `cebac353d` is the merge-base. The distinction is not -> cosmetic — the package's headline ledger figure is only true against `cebac353d`; measured against -> `a4cc1367b` two of upstream's own edits get counted as the fork's. `docs/coil/SEAMS.md` still -> carries the old base; that defect predates this package and is a follow-up, not part of it. +file-size figures are re-measured against the current merge-base **`941acb4f9`** (the 2026-09-02 +sync), which is also the base `docs/coil/SEAMS.md` now carries — **53 upstream-owned files, ++2609 / −981**. + +> **Re-baselined 2026-09-02 (issue #125 §D).** Two earlier bases are named in the history of this +> document and both are now superseded. The 2026-08-18 sync moved the base from `a4cc1367b` to +> `cebac353d`; the 2026-09-02 sync (182 upstream commits, issue #128) moved it again to +> `941acb4f9`. **Every churn and risk figure below is re-measured against `941acb4f9`**, and several +> moved a lot — `ClaudeAdapter.ts` 16 → **23**, `settingsSearch.ts` 14 → **24**, +> `_chat.$environmentId.$threadId.tsx` 4 → **2**. Churn is measured over the 60 days _before_ the +> merge-base, so a busier upstream window moves every number without any fork change; compare risk +> within this table, not against a previous revision. The SEAMS.md header no longer lags — that +> follow-up landed as `93c1e67b8`. **Marker discipline.** `[A]` is an assumption this design has not verified. `[V]` is verified by command against this tree. **`[V - external]`** is verified by command too, but against a shipped @@ -50,8 +54,9 @@ fires is the measure of a correct implementation, not a sign it is doing nothing Total new upstream surface for phases 1–4: **3 new seam rows, ~+16 lines** — the `hooks` spread into `ClaudeAdapter`'s existing `queryOptions` object (+1), `settingsSearch.ts` (**~+13**) and `SettingsSidebarNav.tsx` (+2) — plus one existing seam row rewritten in place at delta zero, and -~6 lines in an existing fork-owned file, which is not upstream surface at all. The budget is -PLAN §6; §11 below prices each row. +~6 lines in an existing fork-owned file, which is not upstream surface at all. **Phase 5 adds one +more row, measured 2026-09-02 and no longer an estimate: `McpHttpServer.ts` `+2/−1`, churn 2, +risk 6** (§11). The budget is PLAN §6; §11 below prices each row. Everything else is new files upstream has never seen. --- @@ -205,23 +210,48 @@ decision table testable without a server, a clock, or a provider. Every case in ## 3. Data model -One file, `coil-loop.json`, in `ServerConfig.stateDir`. +One file, `coil-loop.json`, in `ServerConfig.stateDir`. Its top level is +`{ version: 1, global: LoopGlobalSettings, threads: Record }` — the `global` +key is the master toggle's home, added by review (issue #125 §B7) because the toggle was the +headline kill switch with no data model, no route and no test. ```ts +LoopGlobalSettings = { + enabled: boolean // default FALSE. The master toggle (guard 2). + maxArmedThreads: number // default 3, enforced in the route AND re-checked per tick + defaultMaxCheckIns: number // default 6, <= 20 + defaultRunMs: number // default 8h — seeds the arm form, never a fallback deadline + defaultIdleMs: number // default 15 min + defaultBusyIdleMs: number // default 45 min +} + LoopRecord = { armed: boolean // default FALSE. Nothing is supervised implicitly. armedAtMs: number goal: string | null // what the user said they wanted, for the console header - // budget — mandatory, no unlimited mode - maxCheckIns: number // <= 20, enforced in the route with a 400 + // budget — mandatory, no unlimited mode, no null state + maxCheckIns: number // 1..20, enforced in the route with a 400 checkInsUsed: number - deadlineAtMs: number | null + deadlineAtMs: number // MANDATORY at arm time. Not nullable. See the note below. // thresholds, per-thread overridable idleMs: number // default 15 min busyIdleMs: number // default 45 min + // what the agent scheduled for itself, from the Stop / SubagentStop hook (§4) + crons: { + recordedAtMs: number + entries: Array<{ + id: string + schedule: string // a 5-field cron expression, NOT a timestamp + recurring: boolean + prompt: string // truncated to 1000 chars by the binary — a label, not the prompt + nextFireAtMs: number | null // computed fork-side; null when the expression did not parse + }> + } | null // null = never observed; { entries: [] } = observed and empty + degraded: null | "gate_off" | "wake_lost" + // liveness bookkeeping lastCheckIn: { firedAtMs: number; createdAtIso: string } | null checkIns: Array<{ // the iteration ledger, bounded by maxCheckIns (so <= 20) @@ -234,6 +264,9 @@ LoopRecord = { strikes: number rateLimitedUntilMs: number + // pin bookkeeping — see §7's "Arming pins, and what that costs" + pinnedByLoop: boolean // true only when the arm route created the pin + // terminal state — sticky, only a human re-arm clears it stopped: null | { reason: "done" | "spent" | "stalled" | "handed-back" @@ -245,6 +278,22 @@ LoopRecord = { } ``` +**`deadlineAtMs` is mandatory and is not nullable — decided by review (issue #125 §A1).** Three +files disagreed about this: an earlier revision of §4 flagged "reject arming without a deadline" as +an unresolved change to this contract, PLAN stated the deadline bound with no caveat, and guard 10b +carried a `deadlineAtMs == null` branch that meant _no deference at all_ — so a deadline-less loop +would fire on top of a healthy self-pacing thread, the exact case §0 declares impossible. The +resolution is the one that makes the wrong behaviour unrepresentable: **a null deadline is not a +state.** The route returns `400 deadline_required` (never a clamp — D9), the field is `number`, and +guard 10b's null branch is gone. + +Its **decoding default is `0`**, deliberately, and that is the only reason a record can ever be seen +without a real deadline: a file written by a build that predates the field, or one hand-edited to +drop it, decodes to epoch — which is always `<= now`, so guard 4b stops the loop as `spent` on the +first evaluation. Fail-closed. `maxCheckIns` takes the same treatment for the same reason (default +`0` ⇒ immediately spent). A decoding default that meant "unbounded" would turn a corrupted write +into an unbounded overnight spend, which is the single worst outcome this feature can produce. + The `checkIns` array is what makes the console's iteration ledger reconstructable at all: without `firedAtMs` and the activity cursor recorded _at nudge time_, the history cannot be rebuilt later. Ledger rows therefore render **derived facts** — turns, activities, files moved between two cursors @@ -255,7 +304,9 @@ absent here rather than stubbed. **Every field is `Schema.withDecodingDefaultKey`.** This is not style. A missing _required_ key fails the whole-file decode, and the boot path turns a decode failure into `EMPTY_STATE` — which would silently disarm every loop on the machine. `autoResume/state.ts` carries this warning in a -comment and it is the highest-severity footgun in the module. +comment and it is the highest-severity footgun in the module. **Choose each default so the +fail-closed reading is the one you get**: `armed: false`, `deadlineAtMs: 0`, `maxCheckIns: 0`, +`crons: null`, `pinnedByLoop: false`, `global.enabled: false`. Blockers live in a sibling map keyed by thread: @@ -286,13 +337,16 @@ threshold = busyTurn ? config.busyIdleMs : config.idleMs selfPacedWakeMs = record.crons.nextFireAtMs // from the Stop hook, persisted graceMs = wakeGrace(record.crons) // derived per entry, see "the grace" below deferrable = selfPacedWakeMs != null - && record.deadlineAtMs != null // no deadline, no cap to defer to && selfPacedWakeMs <= record.deadlineAtMs // the loop's own clock is the cap fire when idleMs >= threshold && !(deferrable && now < selfPacedWakeMs + graceMs) ``` +`deadlineAtMs` is a `number`, never null (§3), so there is no third branch here and no +"deadline-less" case to reason about. The grace boundary is **inclusive**: at exactly +`selfPacedWakeMs + graceMs` the wake counts as lost and T3 fires. + `busyTurn` is `shell.session?.status ∈ {running, starting} || shell.latestTurn?.state === "running"`. **`shell.backgroundLiveness` is deliberately not in `busyTurn`**, even though it is the exact, @@ -329,17 +383,33 @@ stands supervision down for up to 24 hours, and a one-shot pinned to a future da indefinitely, all while the run is nominally armed. The rule is now: **T3 defers only while the recorded next fire is at or before the loop's wall-clock deadline.** Past the deadline the run is over on T3's clock either way, so the deadline is the natural cap and no new config knob is needed; -beyond it T3 paces on its own clock and the ordinary staleness trigger applies. Two consequences -worth arguing with: whether the deadline is the right cap or an explicit `maxDeferMs` would be -honest — that belongs beside PLAN's Q1 — and that `deadlineAtMs` is nullable today (§3), so a loop -armed with no deadline has no cap to defer to. The proposal is to **reject arming without a -deadline** in the route rather than to defer forever; that is a change to §3's contract and is -flagged, not assumed. +beyond it T3 paces on its own clock and the ordinary staleness trigger applies. + +**Both loose ends are now closed (issue #125 §A1).** The deadline is the cap; no `maxDeferMs` knob +is added, because a second knob would have to be explained in terms of the first and every value +other than "the deadline" describes a run that is nominally armed but knowingly unsupervised. +And `deadlineAtMs` is **not nullable** (§3) — the route rejects an arm without one with +`400 deadline_required`, so "a loop with no cap to defer to" is not a reachable state rather than a +branch to handle. Deference is therefore exactly one sentence: _T3 stands down while a recorded +wake's `nextFireAtMs` is at or before the deadline and is not yet overdue by its grace. Past the +deadline there is nothing left to defer to._ `record.crons` is written by the `Stop` / `SubagentStop` hook callbacks (see §2's `crons.ts`): read `input.session_crons`, compute `nextFireAtMs` fork-side from `schedule` (one-shot = single fire time encoded in the fields; server-local tz) `[A — the parse is ours]`, persist per thread. +**The parse is fork-owned, and it must not bring a dependency.** `git grep -i cron -- '*package.json'` +and a `cron` search over `pnpm-lock.yaml` both return **zero** — there is no cron parser anywhere in +this repo `[V]`. Nor should one be added: a general parser handles a grammar the producer never +emits. The producer's grammar is documented and narrow — `CronCreateInput.cron` is +_"Standard 5-field cron expression in local time: `M H DoM Mon DoW`"_, with `*/5` steps and `1-5` +ranges in its own examples, **no seconds field and no `@daily`-style macros** `[V - external]`. So +`crons.ts` ships a ~120-line parser for exactly that grammar: five space-separated fields, each +`*`, `N`, `A-B`, `A-B/S`, `*/S` or a comma-list of those, evaluated in the **server's local +timezone** (the tool says "local time", and the binary and the server run in the same process +group). Anything it cannot parse yields `nextFireAtMs: null` for that entry, which means **no +deference from that entry** — an unparseable schedule must never stand supervision down. + **The delivery is no longer an assumption.** This was the package's single largest `[A]` — a design whose strongest trigger rode on a hook payload nobody had confirmed. Read out of the shipped binary (2.1.236) `[V - external]`: the payload `{ background_tasks, session_crons }` is spread @@ -380,9 +450,27 @@ off, and `wake_lost` when a recorded wake did not land. Both surface on the cons `gate_off` needs a source, because `session_crons` carries no gate status: a **`PostToolUse` hook on `ScheduleWakeup`**, declared in the _same_ fork-built hooks object as the `Stop` callbacks, reading the tool's own response. That is zero extra seam — the object is fork-side, so the upstream spread -does not grow — and like every other callback here it only writes to the fork store `[A — the -response shape is not verified]`. If it turns out not to be readable, the state is dropped rather -than guessed at. +does not grow — and like every other callback here it only writes to the fork store. + +**The plumbing is verified; only the response body is still an assumption.** `HookCallbackMatcher` +carries an optional `matcher` string, which for `PreToolUse` / `PostToolUse` selects by tool name, +and `PostToolUseHookInput` is `BaseHookInput & { hook_event_name: "PostToolUse"; tool_name: string; +tool_input: unknown; tool_response: unknown; tool_use_id: string; duration_ms?: number }` +`[V - external]`. So `{ PostToolUse: [{ matcher: "ScheduleWakeup", hooks: [cb] }] }` reliably +delivers the tool's own response to a fork callback. What is **not** verified is the response body: +issue #42 read `ScheduleWakeupTool.call` out of the binary as `if(!zTH())return OsH("gate_off"),…`, +so the marker string is expected to be `gate_off`, but nothing has observed it on the wire +`[A — the response body]`. Spec it as a **substring probe, not a parse**: JSON-stringify +`tool_response`, and record `degraded: "gate_off"` only when the result contains `gate_off` +case-insensitively. Anything else leaves `degraded` untouched. A probe that finds nothing is the +same as no probe, which is the correct degradation — the state is dropped rather than guessed at. + +**Where it renders.** `gate_off` and `wake_lost` are the two values of `record.degraded` and both +render in the console's loop-state section, in the same slot as the rate-limit hold (§8) and with +the same rule: they must read differently from "stalled". `gate_off` reads +_"Self-pacing unavailable — the agent's scheduler is switched off upstream. T3 is pacing this run."_ +None of the prototypes draw this row; that is a gap in the mocks, declared here rather than +hidden (issue #125 §B10). **Why `updatedAt` and nothing else.** `thread.activity-appended` is grouped with `thread.message-sent` in the projection pipeline and rewrites the row with @@ -512,30 +600,99 @@ own deadline — and because `CronCreate` is unbounded and `recurring` entries l (§1.1), it can keep walking for days. The bound would be advisory exactly where the spend is unattended. +**One semantic for the master toggle, settled by review (issue #125 §A3).** Guards 1 and 2 were +described in three incompatible ways across the package — "no fiber", "stand down at next tick", and +"existing loops stop at their next check-in". Only one of them can be true at a time, and a tick +requires a fiber. The rule is: + +- **The supervisor fiber always runs**, exactly as `autoResume`'s does. One tick loop, forked once at + layer construction. +- **`COIL_LOOP_ENABLED=0` is the only thing that stops a fiber existing.** It is read at layer + construction and never again — an env kill switch for an operator, not a product control. +- **The master toggle is a guard evaluated every tick**, and again immediately pre-dispatch so it is + never one-tick-stale. Toggle off ⇒ nothing fires, every armed loop reports `standing_down` with + reason `disabled`, **nothing is disarmed and nothing is stopped**. Toggle back on and the same + loops resume with their budgets intact. + +That is the semantic a kill switch has to have for a feature that spends money unattended: switching +it off must be reversible without asking the user to re-arm anything, and it must not quietly +manufacture terminal states nobody chose. + **Guards, in order.** Non-consuming skips are marked ○ — they keep budget and surface a reason. -| # | Guard | On fail | -| --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| 1 | `config.enabled` (env kill switch, checked at layer construction) | no fiber forks | -| 2 | `store.global.enabled` — re-read **every tick and again pre-dispatch** | ○ the settings toggle is a true kill switch, not one-tick-stale | -| 3 | `record.armed === true` | ○ | -| 4 | shell is `Some`, not archived, is a supported thread | **disarm** | -| 5 | `settledOverride !== "settled"` | ○ | -| 6 | `snoozedUntil == null \|\| <= now` | ○ the only way to honour a snooze | -| 7 | `settledOverride === "active"` ⇒ nudge **then** repair pin | — | -| 8 | `!hasPendingApprovals && !hasPendingUserInput && !hasActionableProposedPlan` | ○ | -| 9 | `autoResumeStore.getThread(threadId).pending === null` | ○ | -| 10 | `now >= record.rateLimitedUntilMs` | ○ | -| 10b | **no deferrable wake still pending** — `nextFireAtMs == null \|\| record.deadlineAtMs == null \|\| nextFireAtMs > record.deadlineAtMs \|\| now >= nextFireAtMs + wakeGrace(entry)` | ○ the deference rule — while a wake is pending _inside the loop's deadline_ the agent is pacing itself, so T3 stands by. A wake past the deadline is not deferred to: `CronCreate` is unbounded (§1.1, §4). Console: _"Self-pacing · next wake 02:35"_ | -| 11 | `now - lastCheckIn.firedAtMs >= config.idleMs` | ○ structural floor: a tight loop stays impossible even if `updatedAt` fails to bump | -| 12 | idle threshold met, on a freshly re-read shell | ○ | -| 13 | budget, deadline, strikes, sentinel | **stop** | -| 14 | armed threads `< maxArmedThreads` — enforced in the route **and** re-checked in the tick | ○ not bypassable by hand-editing the state file | +| # | Guard | On fail | +| --- | ------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `config.enabled` (env kill switch, `COIL_LOOP_ENABLED`, checked at layer construction) | no fiber forks — the **only** condition under which no fiber exists | +| 2 | `store.global.enabled` — re-read **every tick and again pre-dispatch** | ○ reason `disabled`; the loop reports `standing_down` and stays armed. Nothing is disarmed, nothing is stopped | +| 3 | `record.armed === true` | ○ | +| 4 | shell is `Some`, not archived, is a supported thread | **disarm** | +| 4b | **stop conditions, swept before any ○ guard**: `now >= deadlineAtMs`, `checkInsUsed >= maxCheckIns`, sentinel present, two strikes | **stop** — see "Why the stop sweep moved" below | +| 5 | _retired._ `settledOverride !== "settled"` — see "Why guard 5 is gone" below | — | +| 6 | `snoozedUntil == null \|\| <= now` | ○ the only way to honour a snooze | +| 7 | `settledOverride === "active"` ⇒ nudge **then** repair pin | — | +| 8 | `!hasPendingApprovals && !hasPendingUserInput && !hasActionableProposedPlan` | ○ | +| 9 | `autoResumeStore.getThread(threadId).pending === null` | ○ | +| 10 | `now >= record.rateLimitedUntilMs` | ○ | +| 10b | **no deferrable wake still pending** — `nextFireAtMs == null \|\| nextFireAtMs > record.deadlineAtMs \|\| now >= nextFireAtMs + wakeGrace(entry)` | ○ the deference rule — while a wake is pending _inside the loop's deadline_ the agent is pacing itself, so T3 stands by. A wake past the deadline is not deferred to: `CronCreate` is unbounded (§1.1, §4). The `now >=` is **inclusive**. Console: _"Self-pacing · next wake 02:35"_ | +| 11 | `now - lastCheckIn.firedAtMs >= config.idleMs` | ○ structural floor: a tight loop stays impossible even if `updatedAt` fails to bump | +| 12 | idle threshold met, on a freshly re-read shell | ○ | +| 13 | _moved to 4b._ Kept as a number so cited cases keep meaning | — | +| 14 | armed threads `< maxArmedThreads` — enforced in the route **and** re-checked in the tick | ○ not bypassable by hand-editing the state file | Guard 8's third clause is the one every design in the original panel missed. `Sidebar.logic.ts` treats plan-ready as **not** pending-input, so a thread parked on an unapproved plan otherwise passes every other blocking guard and gets pushed past the human's yes. +**Why the stop sweep moved from 13 to 4b (added by review, 2026-09-02).** Guard 13 sat _after_ the +idle guards, so a stop condition was only ever evaluated on a tick that had already decided the +thread was idle enough to nudge. Every ○ guard above it is therefore a way to walk past a deadline: +a thread that never goes idle passes 12's check never, so 13 never ran, and a self-paced run +strolled through its own deadline indefinitely — which is precisely the outcome the `stopSession` +paragraph above says must not happen. The same hole swallowed the sentinel: an agent that wrote +`.coil/loop-done` while still working was not recorded as `done` until it happened to go quiet. +Stop conditions are facts about the **run**, not about the thread's current activity, so they are +swept immediately after the shell resolves and before anything can skip. `13` is retained as a +retired number rather than reused, the same discipline TESTS.md uses for its case numbering. + +**Why guard 5 is gone (added by review, 2026-09-02).** `settledOverride === "settled"` was a ○ skip, +read as "the human is done here". It has not meant that since upstream **#8600** moved settlement +server-side: `ThreadSettlementReactor` sweeps every minute and dispatches `thread.auto-settle`, +which shares `thread.settle`'s decider case and emits the same `thread.settled` event **with no +provenance marker**. Auto-settle is on by default, so a settled flag is now overwhelmingly likely to +be a timer rather than a person, and there is no discriminator to recover the difference from. The +fork has already paid for this once: `autoResume/guards.ts` removed settledness from `threadIsGone` +for exactly this reason, and its comment records that an armed week-long resume was reliably +destroyed on day 3 by a timer. Keeping guard 5 would have reproduced the bug in a worse shape — a ○ +skip never stops the loop, so an auto-settled loop would sit armed and silently do nothing until its +deadline, then report `spent`. Settling is also not a one-way door: sending a turn to a settled +thread clears the override on upstream's own path. The user's opt-out is **disarm**, and the console +says so. Guard 7 (repair the `active` pin) is unaffected and stays. + +Snooze is **not** in the same position and guard 6 stands: there is no auto-snooze command — +`InternalOrchestrationCommand` contains `thread.auto-settle` and no snooze counterpart `[V]` — so +`snoozedUntil` still carries a human's intent. + +**Arming pins, and what that costs.** `thread.pin` is not a decoration. Its decider case emits +**companion `thread.unsettled` and `thread.unsnoozed` events**, with a comment saying so: _"Pinning +is a promotion: it clears the parked states rather than silently outranking them"_ `[V]`. Three +consequences the design has to state rather than discover: + +1. **Arming a snoozed thread would silently cancel the snooze.** The route therefore returns + `400 thread_snoozed` on an arm while `snoozedUntil > now`. Unsnooze first; that is a decision the + human makes, not one a supervisor makes for them. +2. **Arming a settled thread unsettles it, and that is correct** — the human asked for the thread to + run, which is the definition of a promotion. +3. **Disarm unpins only what the loop pinned.** `pinnedByLoop` is recorded at arm time (`true` only + when `pinnedAt` was null before the arm), and disarm dispatches `thread.unpin` only when it is + `true`. Without it, disarming a loop on a thread the user had pinned themselves would remove + their pin, and nothing would record that it had ever been theirs. + +There is **no actor check** on any of this: orchestration commands carry no actor field, `dispatch` +takes only an optional descriptive `origin`, and the decider's `thread.pin` case guards on archival +alone `[V]`. A fork reactor may dispatch `thread.pin` exactly as it dispatches `thread.turn.start`. +The corollary is that nothing upstream will stop the fork from doing the wrong thing here, which is +why the three rules above are rules rather than notes. + --- ## 8. Not fighting auto-resume, and surviving limits @@ -600,6 +757,31 @@ Two design consequences: - It is a second, independent argument for `raise_blocker`: a deferred blocker is durable fork-side state and cannot be discarded by a session stop. +### 9.1c And there is now a _second_ blocking dialog, which the loop can trigger itself + +Upstream **#8144** (`c7222ca4d`, 2026-08-25) added `onUserDialog` with +`supportedDialogKinds: ["resume_return"]` to the same `queryOptions` object phase 1 edits `[V]` — +present in the fork's tree today. It routes into the same blocking `Deferred` as `AskUserQuestion`, +and it fires **on session resume**. So the loop's own nudge, landing on a thread whose session was +torn down, can manufacture a pending user-input that guard 8 then treats as a hard skip. + +The failure is not that the loop pushes past a human — guard 8 correctly refuses — it is that the +loop can **cause** the thing that parks it, silently, and then sit at zero spend until its deadline +reports `spent`. Three rules, and no new guard: + +1. **Guard 8 does not change.** A pending input is a pending input whatever produced it; nudging + past one is worse than waiting. +2. **The fork's `user-input.requested` record (§9.1b) carries the dialog kind**, so the console can + say _"waiting on a session-resume confirmation since 01:04"_ rather than showing an unexplained + idle loop. A `resume_return` park that the loop caused must be legible as exactly that. +3. **It resolves through the deadline, and that is acceptable.** A loop parked on a dialog spends + nothing and ends `spent`, distinctly coloured and distinctly worded, with the reason on the + console. That is the correct outcome for "a human is needed and was not there". + +Whether this is common enough to want a `resume_return` auto-answer is a question for the first +dogfooding run, not for the design. It is deliberately **not** answered here: auto-answering a +dialog the human never saw is precisely the class of move that made the console worth building. + ### 9.2 The fix — a second, non-blocking channel A fork-owned MCP tool, `raise_blocker`, that **records and returns immediately**: @@ -656,32 +838,56 @@ GET /api/coil/loop?threadId=… -> record + derived state + blockers + led POST /api/coil/loop -> arm / disarm / edit bounds / re-arm after terminal POST /api/coil/loop/answer -> answer a blocker or a native pending input GET /api/coil/loops -> all loops (the workspace view) +GET /api/coil/loop/settings -> the global block: enabled + defaults + armed count +POST /api/coil/loop/settings -> write the master toggle and the defaults ``` +**The settings pair is new, added by review (issue #125 §B7), and it moves phase.** `store.global.enabled` +was the headline kill switch with no data model, no route and no test — and the sequencing +consequence was worse than the omission: phase 2 shipped "default off behind the master toggle" +while only phase 4 could flip it, so phases 2 and 3 were **unswitchable as ordered**. §3 now carries +`LoopGlobalSettings`, and **the settings routes ship in phase 2 with the reactor**. Phase 4 adds the +_UI_ over a route that already works, which is also how it should have been sequenced anyway: a +control surface built on a live endpoint is testable, one built on a stub is not. + Operate scope, not read — these mutate scheduling. Upstream's private scope-auth path currently exists as **two independent fork mirrors** — `authenticateWithOperateScope` in `autoResume/http.ts` (scope hardcoded) and `authenticateWithScope(scope)` in `webPush/http.ts` (parameterised) — and only one is in the ledger; this feature moves the general form to a shared `coil/http/auth.ts` rather than becoming a third. +**Correction to phase 0's brief: the webPush form is _not_ strictly more general.** It is +parameterised on scope and returns the session, which `autoResume`'s is not — but it calls +`failEnvironmentAuthInvalid` with **one** argument, where `autoResume`'s passes +`EnvironmentAuth.serverAuthDpopFailureReason(error)` as the second. That second argument is the +whole point of `3bdf109e2`, which landed on 2026-09-02 precisely because the stale one-argument call +compiled and drifted silently: a relay client whose DPoP proof failed got a precise reason from +every environment endpoint except this one. Promoting webPush's body verbatim would **re-introduce +the bug it just fixed, in a shared helper, for all three callers**. The promoted +`authenticateWithScope(scope)` must be the union of both: parameterised on scope, returning the +session, **and** passing the DPoP failure reason. + --- ## 11. Seam cost -| File | Delta | Note | -| --------------------------------------------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `apps/server/src/coil/index.ts` | +~6 | **fork-owned, churn 0** — store, reactor, routes | -| `apps/web/src/routes/_chat.$environmentId.$threadId.tsx` | **±0** | existing row (+10/−6, churn 4) rewritten in place — **delta 0** (the row already carries risk 64): `` → ``, which then hosts both. Genuinely delta-zero, and every future per-thread fork surface is free forever. | -| `apps/server/src/server.ts` | **0** | already has its 3-line row | -| `packages/contracts` | **0** | activity `kind` is an open `TrimmedNonEmptyString`, payload is `Schema.Unknown` | -| `Sidebar.tsx` / `Sidebar.logic.ts` | **0** | a loop is a **pinned thread** — `thread.pin` already exists | -| `apps/server/src/provider/Layers/ClaudeAdapter.ts` | **+1** | **new row.** A single spread into the existing `queryOptions` object: `...(loop ? { hooks: loop.claudeHooks(threadId) } : {})`. `options.hooks` is set nowhere in this repo today, so this is the first subscription to a 30-event surface — though the neighbourhood is no longer empty: #4466 sets `settings: { disableAllHooks: true }` on the capability probe, so upstream has begun touching hook-adjacent config. | -| `apps/web/src/components/settings/settingsSearch.ts` | **~+13** | **new row** (phase 4) — the `SettingsPath` union, a `SETTINGS_SECTION_LABELS` entry, and 2 `SETTINGS_SEARCH_ITEMS`. Measured, not estimated: applying that recipe to the real file and diffing gives **+13**, because each search item is a 5–6 line object literal (37 items span 202 lines); upstream's own +26 on this file decomposes the same way, as 1 union + 1 label + 4 items. Three items is ~+19. Append-ordered arrays, additive. | -| `apps/web/src/components/settings/SettingsSidebarNav.tsx` | **+2** | **new row** (phase 4) — the icon import and its `SETTINGS_SECTION_ICONS` entry. | +| File | Delta | Note | +| --------------------------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `apps/server/src/coil/index.ts` | +~6 | **fork-owned, churn 0** — store, reactor, routes | +| `apps/web/src/routes/_chat.$environmentId.$threadId.tsx` | **±0** | existing row (+10/−6, churn **2**, risk **32**) rewritten in place — **delta 0**: `` → ``, one JSX element swapped for one JSX element inside the fragment the fork already added. The aggregator then hosts both. Genuinely delta-zero, and every future per-thread fork surface is free forever. Re-measured 2026-09-02: churn fell 4 → 2, so the row is **cheaper** than the docs said, not dearer. | +| `apps/server/src/server.ts` | **0** | already has its 3-line row | +| `packages/contracts` | **0** | activity `kind` is an open `TrimmedNonEmptyString`, payload is `Schema.Unknown` | +| `Sidebar.tsx` / `Sidebar.logic.ts` | **0** | a loop is a **pinned thread** — `thread.pin` already exists | +| `apps/server/src/provider/Layers/ClaudeAdapter.ts` | **+1** | **new row.** A single spread into the existing `queryOptions` object: `...(loop ? { hooks: loop.claudeHooks(threadId) } : {})`. `options.hooks` is set nowhere in this repo today, so this is the first subscription to a 30-event surface — though the neighbourhood is no longer empty: #4466 sets `settings: { disableAllHooks: true }` on the capability probe, so upstream has begun touching hook-adjacent config. | +| `apps/web/src/components/settings/settingsSearch.ts` | **~+13** | **new row** (phase 4) — the `SettingsPath` union, a `SETTINGS_SECTION_LABELS` entry, and 2 `SETTINGS_SEARCH_ITEMS`. Measured, not estimated: applying that recipe to the real file and diffing gives **+13**, because each search item is a 5–6 line object literal (37 items span 202 lines); upstream's own +26 on this file decomposes the same way, as 1 union + 1 label + 4 items. Three items is ~+19. Append-ordered arrays, additive. | +| `apps/web/src/components/settings/SettingsSidebarNav.tsx` | **+2** | **new row** (phase 4) — the icon import and its `SETTINGS_SECTION_ICONS` entry. | + +| `apps/server/src/mcp/McpHttpServer.ts` | **+2/−1** | **new row** (phase 5) — one import of the fork's `LoopToolkitRegistrationLive`, and the terminal `export const layer = PreviewToolkitRegistrationLive.pipe(…)` wrapped in a `Layer.mergeAll`. Churn **2**, risk **6**. Measured, not estimated — see "Phase 5, measured" below. | **The `ClaudeAdapter` row is new since the first draft** and it is the cost of the deference rule. -It is worth arguing rather than waving through: the file is churn-16 and ~4.6k lines (4,644 at the -merge-base), which is the most expensive kind of row this fork can take. Three things keep it cheap: +It is worth arguing rather than waving through: the file is churn-**23** and ~4.8k lines (4,820 at +merge-base `941acb4f9`), which is the most expensive kind of row this fork can take. Three things +keep it cheap: 1. **It is one line, and it is additive.** A spread into an object literal alongside the existing `mcpServers` spread. Every line of logic lives fork-side in `coil/loop/crons.ts`. @@ -715,6 +921,63 @@ sleep should be findable by name. `SETTINGS_SEARCH_ITEMS`, so the fork's own rows become searchable the moment the items are added — which is part of what the `settingsSearch.ts` row buys. +**Both settings rows are mandatory, not a style choice** `[V]`. `SETTINGS_SECTION_LABELS` in +`settingsSearch.ts` and `SETTINGS_SECTION_ICONS` in `SettingsSidebarNav.tsx` are both +`Readonly>`, and `SETTINGS_NAV_ITEMS` is _derived_ by mapping the label +record's keys. So adding `"/settings/loops"` to the `SettingsPath` union without also adding a label +**and** an icon is a type error. There is no cheaper additive shape. + +Their price has risen since the estimate and should be re-read before phase 4 starts: measured at +merge-base `941acb4f9`, `settingsSearch.ts` is churn **24** (was 14) and `SettingsSidebarNav.tsx` +churn **19**. At ~+13 and +2 lines that is risk ~312 and ~38 — an order of magnitude above phase 1's +adapter row, and phase 4's own upstream neighbourhood keeps moving (`a19f01fc1`, 2026-09-02, adds +another `settingsSearch.ts` entry). D10 stands; the number is simply larger than it was. + +**The zero-row fallback, recorded because it now exists in the tree.** `/settings/diagnostics` is a +real, navigable settings route that is **not** a `SettingsPath` member — `SETTINGS_BREADCRUMB_LABELS` +patches its label in by hand `[V]`. So a fork-owned `settings.loops.tsx` reached from the console +rather than from the sidebar costs **zero upstream rows**, at the price of not being findable by +name in the nav or in settings search. That is exactly the trade D10 rejected ("a feature that can +spend money while you sleep should be findable by name") and the decision does not change — but if +the two rows are ever refused at review, this is the fallback, and it is upstream's own pattern +rather than an invention. + +**Phase 5, measured (issue #125 asked; PLAN had it as `0–3 rows [A]`).** Three paths, all measured +against `941acb4f9`: + +| Path | Upstream edits | Rows | Risk | +| ------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----- | ---- | +| **A** — a gated `"loop"` capability | `McpInvocationContext.ts` +1/−1 (churn 1) · `McpSessionRegistry.ts` +1/−1 (churn 1) · **`packages/contracts/src/previewAutomation.ts`** +1/−1 (churn 3) · `McpHttpServer.ts` +2/−1 (churn 2) | **4** | 16 | +| **B** — ungated toolkit **(the decision)** | `McpHttpServer.ts` +2/−1 (churn 2) | **1** | 6 | +| **C** — register from `coil/index.ts` | none | **0** | 0 | + +**Path B is the decision.** Path A is rejected on two counts and neither is the row count: it forces +an edit to `packages/contracts` (`PreviewAutomationUnavailableError`'s `capability` is a runtime +`Schema.Literal("preview")`, so widening the TypeScript union alone does not typecheck), which +breaks **D12**; and it makes a tool named `raise_blocker` fail with an error class named +`PreviewAutomationUnavailableError`. The capability gate buys nothing here in exchange: nothing at +registration or dispatch consults `capabilities` — `requireMcpCapability` is one voluntary line +inside one handler helper, and the credential is **already per-thread and already all-or-nothing** +`[V]`. The loop toolkit's real gate is `store.global.enabled` plus the per-thread armed record, both +of which it must check anyway. + +**Try Path C first, and fall back to B without ceremony.** `McpServer.toolkit(t)` is +`Layer.effectDiscard(registerToolkit(t)).pipe(Layer.provide(McpServer.layer))` and `layerHttp` +provides the _same_ layer value, so Effect's MemoMap should hand both the one `McpServer` instance — +the identical memoisation property `coil/index.ts` already depends on and `autoResume/sharing.test.ts` +already pins. The unresolved blocker is typed, not behavioural: a declared `McpInvocationContext` +dependency leaks into the layer's `R`, which today is discharged only inside the non-exported +`McpTransportLive`. Upstream's own `registerPreviewSnapshot` shows the way out — +`Effect.withFiber` + `Context.getUnsafe(fiber.context, McpInvocationContext)` — which keeps `R` +empty. **This has not been typechecked.** Attempt it, give it one focused session, and take Path B +the moment it fights back; the difference is one row at risk 6. + +**A coupling phase 5 inherits and cannot fix.** The MCP credential is minted only when +`enableAgentBrowserAccess` is true (`ProviderService.prepareMcpSession` revokes it otherwise) `[V]`. +So a user who turns **Settings → Integrations → Agent browser access** off loses `raise_blocker`, +`loop_status` and `loop_done` along with it, with no loop-shaped explanation. The console must +render that as a named degraded state, not as an empty blocker list. + --- ## 12. Restart safety @@ -728,6 +991,36 @@ This is the property **no event-edge design has**, because both `providerService `ThreadBackgroundLiveness` explicitly does not provide — its own module doc says _"no persistence, no migration. After a server restart the registry is empty."_ +**Upstream has started work in this area and it does not take the premise away — `5b7d72aad` +(#9167, 2026-09-02), arriving on the next sync.** It continues _active threads_ across a server +restart: before a self-update it stamps the running `activeTurnId` into +`provider_session_runtime.runtime_payload_json.continueAfterServerUpdate`, and on the next cold +start it re-establishes the binding and sends a fresh turn (`"Continue where you left off."`, or +promptless where the adapter supports it). Four reasons D1's durability argument survives intact, +each from the commit `[V]`: + +1. **It marks only threads with `session.status === "running" && activeTurnId !== null`.** A thread + waiting on a scheduled wake is neither. **A pending wake is never marked, so it is never + continued** — which is the exact gap this feature exists to cover. +2. **It is opt-in and ships off**: `continueThreadsAfterServerUpdate` decodes to `false`. +3. **It only writes the marker on the intentional self-update path.** A crash, an OOM, a `kill`, a + machine reboot — the cases a supervisor is for — write nothing and behave exactly as before, + settling the thread with _"Provider session did not survive a server restart."_ +4. **It does not continue the interrupted turn.** `activeTurnId` is nulled and a new turn is + started, so a partially-finished turn is lost either way. + +What it does introduce is a **double-fire window**, and it is worth naming precisely because it is +the kind of thing that is invisible until 3am. Reconciliation dispatches `session.status: "starting", +activeTurnId: null` synchronously at startup, but the actual `sendTurn` is parked behind server +activation — so there is a real interval in which a continued thread looks idle, with no error, and +nothing takes a lease. **This design already covers it, by two independent mechanisms**: +`busyTurn` counts `status === "starting"` (§4), so the fuse is `busyIdleMs`; and +`processStartedAtMs` floors the idle clock at process start, so no armed thread can fire for at +least `idleMs` after boot regardless. Neither was added for this, which is the reassuring part. +TESTS case 130b pins it. If a stronger guard is ever wanted, the honest signal is the marker itself — +non-null means upstream has claimed the thread — but reading `provider_session_runtime` from a fork +reactor would be a new coupling to an upstream table, and the two mechanisms above cost nothing. + --- ## 13. What this honestly does not solve diff --git a/docs/coil/loops-v2/FINDINGS.md b/docs/coil/loops-v2/FINDINGS.md index ca75bd6e1183..536454a6d1ca 100644 --- a/docs/coil/loops-v2/FINDINGS.md +++ b/docs/coil/loops-v2/FINDINGS.md @@ -34,13 +34,25 @@ da6e1a967 2026-08-04 feat(sidebar-v2): thread pinning for sidebar v2 (#5312) > A pin overrides the settled/snoozed lifecycle: while `pinnedAt` is set the thread renders > in the pinned block and never classifies into a shelf. -That comment overstates its own rule, and this file repeated the overstatement. What a pin -actually overrides is **auto-settle**. **Snooze still outranks a pin**, which the sidebar's -partition loop says outright (`Sidebar.tsx`, the `supportsSnooze` branch): "Snooze outranks -everything, including a pin: 'hide until Tuesday' temporarily suspends 'keep on top'. The pin -survives underneath". Read every "a pin overrides the lifecycle" below as _overrides -auto-settle_ — a snoozed Loop leaves the pinned block until it wakes, and returns to its exact -slot. +That comment overstated its own rule, and this file repeated the overstatement. It has since been +**rewritten upstream, and in the other direction**. + +> **Corrected 2026-09-02 (issue #125 §D).** `f70eeeeb0` (#7969, 2026-08-23) inverted pin-vs-settle. +> The contract comment now reads _"Settled and snoozed threads remain in their respective shelves +> even when pinned"_, and the sidebar partition is a single `if / else if` chain — **snoozed, then +> settled, then pinned, then active** `[V]` — mirrored verbatim in +> `apps/mobile/src/features/threads/threadListV2.ts`. So the earlier claim here, that a pin +> overrides auto-settle, is **false**: a settled Loop leaves the pinned block, not just a snoozed +> one. What a pin still buys is a pin marker in whichever shelf the thread lands in, plus +> user-arranged order within the pinned block. +> +> **This does not sink D3, and the reason matters.** D3's load-bearing claim was never "pinning +> keeps a Loop visible forever" — it was **"Direction A costs zero rows in `Sidebar.tsx`"**, and +> that is unchanged. What did change is more useful than what was lost: `thread.pin`'s decider case +> emits companion `thread.unsettled` and `thread.unsnoozed` events `[V]` — _"Pinning is a promotion: +> it clears the parked states rather than silently outranking them"_ — so arming a Loop actively +> un-parks it rather than relying on an ordering rule. The cost of that promotion (arming a snoozed +> thread would cancel the snooze; disarming could remove a user's own pin) is priced in BACKEND §7. Also real today: `thread.pinned` / `thread.unpinned` **events** (`:1070-1071`; the commands that produce them are `thread.pin` / `thread.unpin`, `:739,749`), a fractional @@ -271,6 +283,23 @@ console's primary content is a queue of human-answerable items the loop has accu questions, decisions, blockers — each answerable in place, without reading back through the night's output. +> **The plan diverges from the first half of this, deliberately (issue #125 §A2).** PLAN §3 decides +> that **the transcript stays the default view and the console is an overlay on the same route**. +> This paragraph and prototype P7's Shape A both read as though the opposite had been decided, and +> the reversal was never declared — that is the contradiction #125 caught, and this note is the +> declaration. +> +> The reason is #38's, restated: an overlay has _"nothing to toggle back from"_. A second default +> view means a sticky per-thread toggle that has to survive reload, agree across two windows, agree +> on mobile, and be discoverable when it is wrong — and landing on a different view means owning the +> thread route's render decision, which is `ChatView.tsx` territory (an existing seam row at churn 83) rather than the delta-zero overlay row. +> +> **The content half of this requirement is met in full.** Everything below — the three sources, the +> degradation property, answering without steering into a live turn — is unchanged. What is given up +> is one interaction: the console is a click away rather than zero, so a human opening the thread +> still sees the night's tail first. If dogfooding shows that tail is what sends people back to +> typing "are you still working on it?", Shape A is the fallback and P7 prices it. + Design consequences to work through in the prototypes: - **Where do the items come from?** Three candidate sources, in descending order of how diff --git a/docs/coil/loops-v2/PLAN.md b/docs/coil/loops-v2/PLAN.md index eac72216d59c..9ebeabda1d61 100644 --- a/docs/coil/loops-v2/PLAN.md +++ b/docs/coil/loops-v2/PLAN.md @@ -1,22 +1,24 @@ # Loops — implementation plan -**Status:** proposed — nothing built. -**Baseline:** upstream merge-base `cebac353d`, with the fork **zero commits behind upstream at -verification (2026-08-17)**. The merge-base is the anchor rather than a fork `main` SHA, because -every sync rewrites `main`. Verification originally quoted `a4cc1367b`; the 2026-08-18 daily sync -**moved the merge-base** to `cebac353d`, and the seam numbers below are only correct against -`cebac353d` — measured against `a4cc1367b` the same recipe mis-attributes two of upstream's own -edits to the fork. Every claim below was verified against that tree — see -[UPSTREAM-DELTA.md](UPSTREAM-DELTA.md) §7. - -| Companion doc | What it holds | -| -------------------------------------- | --------------------------------------------------- | -| [report.html](report.html) | the design report, 8 clickable prototypes embedded | -| [report.src.html](report.src.html) | the source the report is built from — edit this one | -| [BACKEND.md](BACKEND.md) | full backend design + rejected architectures | -| [TESTS.md](TESTS.md) | 162 test cases | -| [FINDINGS.md](FINDINGS.md) | raw research notes | -| [UPSTREAM-DELTA.md](UPSTREAM-DELTA.md) | the 2026-08-17 re-verification | +**Status:** proposed — nothing built. **Reviewed and re-baselined 2026-09-02** against the +findings in issue #125; §12 lists what changed. +**Baseline:** upstream merge-base **`941acb4f9`** (the 2026-09-02 sync). The merge-base is the +anchor rather than a fork `main` SHA, because every sync rewrites `main`. The package has now moved +base three times — `a4cc1367b` → `cebac353d` → `941acb4f9` — and **every churn and risk figure below +is re-measured against `941acb4f9`**. Several moved materially: `ClaudeAdapter.ts` 16 → **23**, +`settingsSearch.ts` 14 → **24**, `_chat.$environmentId.$threadId.tsx` 4 → **2**. Churn is counted +over the 60 days _before_ the merge-base, so a busier upstream window moves every number without any +fork change. Structural claims were re-verified against that tree — see +[UPSTREAM-DELTA.md](UPSTREAM-DELTA.md) §7 and §9. + +| Companion doc | What it holds | +| -------------------------------------- | ---------------------------------------------------- | +| [report.html](report.html) | the design report, 8 clickable prototypes embedded | +| [report.src.html](report.src.html) | the source the report is built from — edit this one | +| [BACKEND.md](BACKEND.md) | full backend design + rejected architectures | +| [TESTS.md](TESTS.md) | 183 test cases | +| [FINDINGS.md](FINDINGS.md) | raw research notes | +| [UPSTREAM-DELTA.md](UPSTREAM-DELTA.md) | the 2026-08-17 re-verification, plus §9 (2026-09-02) | `report.html` is generated. Edit `report.src.html` (or a prototype under `prototypes/`), then run both steps — `node docs/coil/loops-v2/build-report.mjs` and @@ -72,7 +74,7 @@ Three properties define it: **T3 Coil is a fork of `pingdotgg/t3code`** that rebases onto upstream continuously — 116 upstream commits landed in the three days before this plan was written, and the sync carrying them landed on `main` the same day `[V]`. The fork maintains a **seam ledger** (`docs/coil/SEAMS.md`) listing every -upstream-owned file it edits: currently **53 files, +2590/−1042 lines** `[V]`. Each edited file is a +upstream-owned file it edits: currently **53 files, +2609/−981 lines** `[V]`. Each edited file is a permanent, recurring merge cost. This produces three rules that shape everything below and would look strange otherwise: @@ -98,20 +100,41 @@ has already survived review. Each with the reasoning, so a reviewer can attack the reasoning rather than guess at it. -| # | Decision | Why | Confidence | -| --- | --------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | -| D1 | **Durable T3-native reactor, backstopping Claude's scheduler** | Claude's scheduler works ≤30min but is in-process and Claude-only `[V]`. Only `ScheduleWakeup` is clamped, to [60, 3600]s; `CronCreate` takes an unbounded cron expression and both write `session_crons` `[V - external]` | High — evidence in BACKEND §1.1 | -| D2 | **Trigger on `updatedAt` staleness + recorded `session_crons`, never `session.status`** | `ProviderSessionReaper` skips any binding whose thread has `session.activeTurnId != null` `[V]`, so a turn whose completion never arrives pins `status = running` with nothing automated to clear it — gating on it deadlocks the exact threads this is for | High | -| D3 | **A loop is a pinned thread** (Direction A) | Upstream shipped pinning 2026-08-04; `pinnedAt` overrides auto-settle — snooze still outranks a pin `[V]`; costs zero sidebar edits `[V]` | High | -| D4 | **The Loops workspace (Direction C) is a later phase (Phase 6), not phase 1** | User agreed. Lives in fork-owned routes, so cost is low, but it is only worth it once several loops exist | High — user-confirmed | -| D5 | **Never Direction B** (a bespoke Loops section in the sidebar) | Would open a row in `Sidebar.tsx` (3911 lines, 7 commits in 3 days `[V]`) and `Sidebar.logic.ts`; also has no mobile equivalent | High | -| D6 | **Two question channels: blocking (native) + deferred (`raise_blocker`)** | `AskUserQuestion` blocks on a `Deferred` `[V]`; a loop that waits loses the night, and one that nudges past a pending decision is worse | High | -| D7 | **Console reads three sources, two needing no model cooperation** | Degradation test: a model that never calls `raise_blocker` must still produce a useful console | High | -| D8 | **Budget is check-ins + wall-clock, not dollars** | The adapter stamps `total_cost_usd` onto `turn.completed.totalCostUsd` and nothing downstream aggregates it; its per-turn vs session-accumulated semantics are unmeasured `[A]`, so summing per turn could inflate quadratically | Medium — revisit if metered | -| D9 | **Mandatory budget, no unlimited option; route returns 400 rather than clamping** | A silent clamp hides a mistake in a feature that spends money unattended | Medium | -| D10 | **Own settings section** (`/settings/loops`) | Now priced at 2 small additive seam rows, with an upstream precedent that landed the same day (2026-08-17) `[V]` | High | -| D11 | **Fork-owned durable JSON, not a DB migration** | The migration registry is upstream-owned; `autoResume` set this precedent and it has held | High | -| D12 | **Zero `packages/contracts` edits** | Activity `kind` is an open string with an `Unknown` payload, so breadcrumbs are free; and upstream has pre-announced this file as its own automations landing zone | High | +| # | Decision | Why | Confidence | +| --- | --------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | +| D1 | **Durable T3-native reactor, backstopping Claude's scheduler** | Claude's scheduler works ≤30min but is in-process and Claude-only `[V]`. Only `ScheduleWakeup` is clamped, to [60, 3600]s; `CronCreate` takes an unbounded cron expression and both write `session_crons` `[V - external]` | High — evidence in BACKEND §1.1 | +| D2 | **Trigger on `updatedAt` staleness + recorded `session_crons`, never `session.status`** | `ProviderSessionReaper` skips any binding whose thread has `session.activeTurnId != null` `[V]`, so a turn whose completion never arrives pins `status = running` with nothing automated to clear it — gating on it deadlocks the exact threads this is for. **"Never" is scoped, not absolute**: `status ∈ {running, starting}` is read as `busyTurn`, which selects `busyIdleMs` (45 min) over `idleMs` (15 min). It **lengthens the fuse and never vetoes a fire** — that distinction is the long-tool-call case, and a reader who takes "never `session.status`" literally drops it (BACKEND §4) | High | +| D3 | **A loop is a pinned thread** (Direction A) | Upstream shipped pinning 2026-08-04 and it still costs **zero** sidebar edits `[V]`. **What a pin buys has shrunk twice and the decision survives both** — see the note below the table | Medium-High — see the note | +| D4 | **The Loops workspace (Direction C) is a later phase (Phase 6), not phase 1** | User agreed. Lives in fork-owned routes, so cost is low, but it is only worth it once several loops exist | High — user-confirmed | +| D5 | **Never Direction B** (a bespoke Loops section in the sidebar) | Would open a row in `Sidebar.tsx` (3911 lines, 7 commits in 3 days `[V]`) and `Sidebar.logic.ts`; also has no mobile equivalent | High | +| D6 | **Two question channels: blocking (native) + deferred (`raise_blocker`)** | `AskUserQuestion` blocks on a `Deferred` `[V]`; a loop that waits loses the night, and one that nudges past a pending decision is worse | High | +| D7 | **Console reads three sources, two needing no model cooperation** | Degradation test: a model that never calls `raise_blocker` must still produce a useful console | High | +| D8 | **Budget is check-ins + wall-clock, not dollars** | The adapter stamps `total_cost_usd` onto `turn.completed.totalCostUsd` and nothing downstream aggregates it; its per-turn vs session-accumulated semantics are unmeasured `[A]`, so summing per turn could inflate quadratically | Medium — revisit if metered | +| D9 | **Mandatory budget, no unlimited option; route returns 400 rather than clamping** | A silent clamp hides a mistake in a feature that spends money unattended | Medium | +| D10 | **Own settings section** (`/settings/loops`) | Now priced at 2 small additive seam rows, with an upstream precedent that landed the same day (2026-08-17) `[V]` | High | +| D11 | **Fork-owned durable JSON, not a DB migration** | The migration registry is upstream-owned; `autoResume` set this precedent and it has held | High | +| D12 | **Zero `packages/contracts` edits** | Activity `kind` is an open string with an `Unknown` payload, so breadcrumbs are free; and upstream has pre-announced this file as its own automations landing zone | High | + +**Correction on D3, added by the #125 review.** Two facts moved under it and neither kills it. + +_First, upstream inverted pin-vs-settle._ `f70eeeeb0` (#7969, 2026-08-23) rewrote the contract +comment to _"Settled and snoozed threads remain in their respective shelves even when pinned"_, and +the sidebar's partition is now a single `if/else if` chain — snoozed, then settled, then pinned, +then active `[V]`, mirrored verbatim in `apps/mobile/src/features/threads/threadListV2.ts`. So the +old claim that "`pinnedAt` overrides auto-settle" is **false**: a settled loop leaves the pinned +block. What survives is that a pinned thread still shows its pin marker in whichever shelf it lands +in, and that the whole arrangement costs zero fork lines. + +_Second, and more usefully, `thread.pin` turns out to be a promotion rather than a decoration._ Its +decider case emits companion `thread.unsettled` and `thread.unsnoozed` events, with a comment +saying so `[V]`. That mostly makes the inversion moot for a loop — arming pins, and pinning +unsettles — but it also means arming a **snoozed** thread would silently cancel the snooze, and +disarming would remove a pin the user set themselves. BACKEND §7 now carries the three rules that +follow (`400 thread_snoozed` on arm, unsettle-on-arm is correct, `pinnedByLoop` gates the unpin). + +_The decision stands_ because its actual load-bearing claim was never "pinning keeps a loop +visible forever" — it was **"Direction A costs zero rows in `Sidebar.tsx`"**, and that is unchanged. +Confidence drops from High to Medium-High because the visibility a pin buys is now conditional. **Retraction on D2, added by review.** The justification previously read "a background subagent's message auto-opens a synthetic turn that pins `status = running` and nothing closes it", marked @@ -133,12 +156,14 @@ The `updatedAt` half of the trigger is independently verified and unchanged. - A per-thread loop: arm, bound, supervise, stop, re-arm. - Deference to the agent's own scheduler, via recorded `session_crons` — **bounded by the loop's own - wall-clock deadline** (see Q1). + wall-clock deadline**, which is mandatory at arm time (Q1, resolved). - The console: blocking items, deferred blockers, loop state, iteration ledger. - `raise_blocker` / `loop_status` / `loop_done` as a fork MCP toolkit. - Settings: master toggle, defaults, armed roster. - Web + desktop (same app). Mobile read-only surfacing — **deferred until after Phase 3** and priced when it is built; this plan takes no mobile seam row. +- The master toggle's **data model and routes** (`LoopGlobalSettings`, `GET`/`POST +/api/coil/loop/settings`) ship in **Phase 2**, with the reactor. Phase 4 adds the UI over them. ### Out (explicitly, with reasons in report §12) @@ -151,6 +176,12 @@ The `updatedAt` half of the trigger is independently verified and unchanged. - **Loops answering their own low-stakes questions** — destroys the console's completeness, which is the only reason to trust it. - **Maintainer bots (#44)** — the same reactor with a different work source; sequenced after. +- **Push notifications and mobile surfacing.** Designed in the report (§6) and named as a + requirement in prototype P8, but they have **no phase, no seam line, no acceptance criteria and no + tests**, and this revision does not invent them (issue #125 §B8, §B9). Two specific things move + from "designed" to "not built": the three push reasons, and the budget on the mobile thread-list + row. The latter is not free the way P8 implies — chrome on the mobile row is a **mobile seam row**, + in a file the fork does not touch today. Price both when mobile is built. ### Divergences from #42 and #38 (deliberate, and open to challenge) @@ -192,7 +223,9 @@ The `updatedAt` half of the trigger is independently verified and unchanged. console covers the parts that need no model cooperation: the per-check-in rollup (the iteration ledger), loop state and bounds, blocking items, deferred blockers. It deliberately does **not** take over the thread route — the transcript stays the default view and the console is an overlay - on the same route, so there is nothing to toggle back from. And it deliberately carries no + on the same route, so there is nothing to toggle back from. **This is a declared divergence from a + requirement the user stated, not an oversight** — see the note closing this section. And it + deliberately carries no model-authored summary of the run: the ledger renders derived facts, never the model's account of its own night, which is the thing that was untrustworthy to begin with. The "right now" block is the one part worth reconsidering, and it depends on the same in-memory roster that guard #15 does. @@ -201,6 +234,29 @@ The `updatedAt` half of the trigger is independently verified and unchanged. the agent inside the loop, and nothing else. Cross-thread reads are a Phase 6 concern (the cross-loop inbox); until then the answer to "how is it going" is the console, opened by a human. +**Declared divergence — the console does not become the default view (issue #125 §A2).** The user's +own words were _"at any time I open the chat there should be a page where I have a questionnaire +ready to be answered"_ (FINDINGS §F1), and prototype P7's Shape A — the recommended shape — draws +exactly that: opening a loop lands on the console, transcript one click away. **This plan decides +the opposite, and the reversal is deliberate.** Three reasons, in order of weight: + +1. **A second route to toggle back from is a one-way door with a hinge on it.** The fork's own rule + is that if you add a way in you add the way out and the way to see it. A sticky per-thread view + toggle is a small piece of state with a large surface: it has to survive reload, agree across + two windows, agree on mobile, and be discoverable when it is wrong. The overlay has none of that + — it is additive chrome over a view that already works everywhere. +2. **The seam is genuinely zero, and Shape A's is not.** The overlay rewrites a row the fork already + owns at delta zero (§6). Landing on a different default view means owning the thread route's + render decision, which is `ChatView.tsx` territory — an existing row at churn 83. +3. **The requirement is about content, not placement.** What was asked for is a standing answer to + "what do you need from me?". An overlay that opens on top of the transcript, on the same route, + with the same content, answers it. Nothing in the ask requires the transcript to go away. + +**What is given up, stated plainly:** the console is one interaction away rather than zero, so a +human who opens the thread still sees the night's tail first. If dogfooding shows that tail is what +sends people back to typing "are you still working on it?", Shape A is the fallback and P7 prices +it. FINDINGS §F1 and P7's Shape A now both carry this note; they previously read as the decision. + --- ## 4. Architecture @@ -247,13 +303,26 @@ one focused agent-assisted session per unit. - Promote the HTTP scope-auth helper into a shared `apps/server/src/coil/http/auth.ts`. Today there are **two independent implementations of the same mirror of upstream's private auth path** `[V]`: - `autoResume/http.ts:45` has `authenticateWithOperateScope` (scope hardcoded) and - `webPush/http.ts:49` has `authenticateWithScope(scope)` (parameterised). The webPush form is - strictly more general — promote it, re-point `autoResume`, and let the loop routes be the third - caller rather than the third paste. + `autoResume/http.ts` has `authenticateWithOperateScope` (scope hardcoded) and `webPush/http.ts` + has `authenticateWithScope(scope)` (parameterised). Let the loop routes be the third caller rather + than the third paste. + **The webPush form is not simply the better one — promote the union of both.** It is + parameterised and returns the session, which `autoResume`'s is not; but it calls + `failEnvironmentAuthInvalid` with one argument where `autoResume`'s passes + `EnvironmentAuth.serverAuthDpopFailureReason(error)` as the second. That second argument is + `3bdf109e2` (2026-09-02), which exists because the stale one-argument call compiled and drifted + silently. Promoting webPush's body verbatim would re-introduce that bug in a shared helper, for + all three callers. The promoted helper is: parameterised on scope, returns the session, **and** + passes the DPoP failure reason. - Widen `isClaudeThread` from `(thread: OrchestrationThread)` to - `Pick` so an `OrchestrationThreadShell` is assignable `[A — the -archived design verified this; re-check]`. + `Pick` so an `OrchestrationThreadShell` is assignable `[V]`. The + two structs declare the field identically — `session: Schema.NullOr(OrchestrationSession)` in + both, so both resolve to `OrchestrationSession | null`. Reproduce with + `git grep -n "session: Schema.NullOr(OrchestrationSession)" packages/contracts/src/orchestration.ts` + (two hits, one per struct). `Pick<…, "session">` is the minimal shape; nothing else in the + predicate is read. Note the shells are **not** interchangeable in general — + `OrchestrationThreadShell` has no `deletedAt`, so guard 4 on a shell tests `archivedAt` plus the + shell simply being absent. **Files:** `coil/http/auth.ts` (new), `coil/autoResume/http.ts`, `coil/webPush/http.ts`, `coil/autoResume/guards.ts` — all fork-owned. @@ -277,8 +346,18 @@ reads its state. `prompt` is **truncated to 1000 characters by the binary** `[V - external]`, so nothing here may assume it holds the agent's full prompt), compute `nextFireAtMs` fork-side from `schedule` (one-shot = single fire time encoded in the fields; server-local tz) `[A — the parse is ours]`, - persist per thread. -- `coil/loop/config.ts` — env-overridable defaults. + persist per thread. **The parse brings no dependency**: there is no cron parser anywhere in this + repo (`git grep -i cron -- '*package.json'` and a `cron` search over `pnpm-lock.yaml` are both + empty `[V]`), and none is added. `CronCreateInput.cron` is documented as _"Standard 5-field cron + expression in local time"_ with `*/5` steps and `1-5` ranges — no seconds field, no macros + `[V - external]` — so `crons.ts` parses exactly that grammar in ~120 lines. An entry that does not + parse yields `nextFireAtMs: null`, which means **no deference from that entry**. +- `coil/loop/config.ts` — env-overridable defaults (`COIL_LOOP_*`). +- A `PostToolUse` hook matched on `ScheduleWakeup`, in the **same** fork-built hooks object, to + source the `gate_off` degraded state. Zero extra seam. The plumbing is verified — + `HookCallbackMatcher.matcher` selects by tool name and `PostToolUseHookInput` carries + `tool_name` + `tool_response` `[V - external]` — the response body is not, so it is a substring + probe rather than a parse (BACKEND §4). - **The one upstream edit:** a single spread into `ClaudeAdapter`'s existing `queryOptions` object, beside the `mcpServers` spread: ```ts @@ -293,8 +372,12 @@ reads its state. - Arming is impossible (no arm route yet); nothing dispatches. - On a Claude thread that self-paces, `GET` shows the pending wake with a plausible `nextFireAtMs`. -- On a non-Claude thread, the record exists and `crons` is empty — no errors. -- A hook callback that throws does not break the turn `[A — to be proven by the §4b hook-failure case]`. +- On a non-Claude thread, the record exists and `crons` is `null` — no errors, and no hooks object + is built at all. +- A hook callback that throws does not break the turn `[A — to be proven by the §4b hook-failure +case]`. The callback returns `{ continue: true }` on every path and never rethrows; a `Stop` hook + **can** halt a turn by returning `{ decision: "block" }` or `{ continue: false }` `[V - external]`, + so "observability only" is a property the code has to hold, not one the surface gives for free. - Killing and restarting the server preserves the record. **Why first:** it is the only phase that can be validated purely by observation, and it de-risks the @@ -318,8 +401,17 @@ here and nothing has been wasted.** expression, so an unbounded rule would let a recorded `0 9 * * *` stand T3 down for a day and a one-shot pinned to a future date stand it down indefinitely. Past the deadline there is nothing left to defer _to_, so the deadline is the natural cap and no new knob is needed. See Q1. -- Arm / disarm / re-arm via `POST /api/coil/loop`, with the 400s from D9. -- Arming also dispatches `thread.pin`; disarming unpins `[A — verify pin/unpin from a fork reactor]`. +- Arm / disarm / re-arm via `POST /api/coil/loop`, with the 400s from D9 — including + `400 deadline_required`, because `deadlineAtMs` is mandatory and is not nullable (BACKEND §3). +- **The master toggle's data model and its routes** — `LoopGlobalSettings` in the store, plus + `GET`/`POST /api/coil/loop/settings`. Moved here from phase 4 by the #125 review: shipping + "default off behind the master toggle" in phase 2 while only phase 4 could flip it left phases 2 + and 3 **unswitchable as ordered**. +- Arming also dispatches `thread.pin`; disarming unpins **only when the loop created the pin** + (`pinnedByLoop`). There is no actor check on `thread.pin` — commands carry no actor and the + decider guards on archival alone `[V]` — so the rules are the fork's to keep. `thread.pin` also + emits companion `thread.unsettled` / `thread.unsnoozed` events `[V]`, so the arm route returns + `400 thread_snoozed` rather than silently cancelling a snooze. BACKEND §7. - Rate-limit tap fiber; `rateLimitedUntilMs` persisted. - Terminal states and disarm stop the session when recorded crons are still pending (`providerService.stopSession` — BACKEND §7), so a bound can actually stop a self-paced run. @@ -334,6 +426,10 @@ here and nothing has been wasted.** - A stall: exactly one fire at the threshold, none during background activity. - Budget exhaustion reports `spent`, never `done`. - Human takeover disarms without resetting budget. +- The master toggle is flippable over HTTP in this phase, with no UI — and flipping it off leaves + every loop **armed** and reporting `standing_down`, disarming nothing. +- A deadline that has passed stops the loop **even while the thread is busy** (guard 4b), which is + what stops a self-paced run walking through its own deadline. **Size:** L. This is the bulk of the work. @@ -345,8 +441,14 @@ here and nothing has been wasted.** - `apps/web/src/coil/ThreadCoilOverlay.tsx` (new) — a fork-owned aggregator that mounts `AutoResumeOverlay` and the console; then **rewrite the existing overlay row in place** to mount - it, so the seam delta is zero `[V — the row is +10/−6, churn 4; delta 0 (row already carries risk -64)]`. + it, so the seam delta is zero `[V — re-measured 2026-09-02: the row is +10/−6, churn **2**, risk +**32**; the edit swaps one JSX element for one JSX element inside the fragment the fork already +added, so the delta is unchanged]`. The fork's web client pattern to reuse is + `AutoResumeOverlay.tsx`: `ManagedRuntime.make(primaryEnvironmentHttpLayer)` + + `resolvePrimaryEnvironmentHttpUrl`, 30s poll plus a focus listener, every failure swallowed to + `null` so the overlay disappears rather than degrading chat. Auth is ambient (the environment HTTP + layer attaches the credential); **there is no client-side scope check and none should be added** — + the 403 is the server's to state. - Console UI: blocking / deferred / loop-state sections, iteration ledger, empty state. - `POST /api/coil/loop/answer`, routing native pending-inputs to the existing resolve path and blockers to the fork store. @@ -367,20 +469,37 @@ here and nothing has been wasted.** **Goal:** the on/off switch and the bounds. - `apps/web/src/routes/settings.loops.tsx` (new, fork-owned) + a fork-owned panel component. -- `settingsSearch.ts`: `SettingsPath` union + label + 2 search items — **~+13 lines**, because each - `SETTINGS_SEARCH_ITEMS` entry is a 5–6 line object literal in the file's existing style (37 items - span 202 lines). A third search item takes it to ~+19. For scale, upstream's own `integrations` - addition to this file measured +26. -- `SettingsSidebarNav.tsx`: icon import + record entry — +2. -- **Not** `SettingsPanels.tsx` (churn 36, risk 2088), **not** `contracts/settings.ts` (churn 26, persisted). - -**Seam cost:** **2 new rows**, ~+15 lines total, both additive. +- `settingsSearch.ts`: `SettingsPath` union + `SETTINGS_SECTION_LABELS` entry + 2 + `SETTINGS_SEARCH_ITEMS` — **~+13 lines**, because each search item is a 5–6 line object literal in + the file's existing style. A third search item takes it to ~+19. +- `SettingsSidebarNav.tsx`: icon import + `SETTINGS_SECTION_ICONS` entry — +2. +- **Both rows are mandatory, not stylistic** `[V]`: `SETTINGS_SECTION_LABELS` and + `SETTINGS_SECTION_ICONS` are both `Readonly>` and `SETTINGS_NAV_ITEMS` is + derived from the label record's keys, so adding a `SettingsPath` member without both entries is a + type error. +- **Not** `SettingsPanels.tsx` (churn **43**, risk ~2900), **not** `contracts/settings.ts` + (churn **38**, persisted). Loop state stays in `coil-loop.json`; the panel reads and writes + `/api/coil/loop/settings`, which phase 2 already shipped. + +**Seam cost:** **2 new rows**, ~+15 lines total, both additive. **Re-measured 2026-09-02 and the +price has risen**: `settingsSearch.ts` is now churn **24** (was 14) and `SettingsSidebarNav.tsx` +churn **19**, so risk is ~312 and ~38. D10 stands, but this is now the most expensive phase in the +plan by risk, and it buys discoverability rather than function. **The zero-row fallback, if the rows +are ever refused:** `/settings/diagnostics` is a real settings route that is **not** a `SettingsPath` +member — its label is patched in by `SETTINGS_BREADCRUMB_LABELS` `[V]` — so a fork route reached +from the console costs zero rows and loses only nav and search discoverability. **Sequencing:** no longer a constraint. This originally had to wait for the sync carrying upstream #7082, or the `settingsSearch.ts` entry would have conflicted with `integrations` on the way in. That sync has landed — `routes/settings.integrations.tsx` is in the tree and `settingsSearch.ts` carries 7 `integrations` references `[V]` — so phase 4 now adds its entry beside a row that is already there, which was the cheap ordering all along. -**Acceptance:** master toggle off ⇒ no fiber, nothing armed, existing loops stand down at next tick. +**Acceptance:** the toggle is a **guard, not a lifecycle** (issue #125 §A3). The supervisor fiber +always runs — one tick loop, like auto-resume — and the toggle is re-read every tick and again +pre-dispatch. Toggle off ⇒ nothing fires, every armed loop reports `standing_down` with reason +`disabled`, **nothing is disarmed and nothing is stopped**; toggle back on and the same loops resume +with budgets intact. The only thing that stops a fiber existing is `COIL_LOOP_ENABLED=0`, read once +at layer construction. An earlier draft of this line said "no fiber" **and** "stand down at next +tick", which cannot both be true — a tick requires a fiber. **Size:** S–M. --- @@ -392,15 +511,26 @@ already there, which was the cheap ordering all along. - `apps/server/src/mcp/toolkits/loop/` — `raise_blocker`, `loop_status`, `loop_done`. - Console renders deferred blockers; answers are delivered on the next check-in prompt. -**Seam cost:** 0–3 rows `[A]` — the toolkit itself is new, but the capability gate may require -edits to `McpInvocationContext.ts` / `McpSessionRegistry.ts` / `McpHttpServer.ts`. **Measure before -committing to this phase** — the archived design estimated three files and that has not been -re-verified. +**Seam cost: 1 new row — `McpHttpServer.ts` `+2/−1`, churn 2, risk 6.** Measured 2026-09-02, no +longer `[A]`. The measurement changed the design: the archived estimate of three files assumed a +gated `"loop"` capability, and that path is now **rejected** — `McpCapability` is backed by a runtime +`Schema.Literal("preview")` in `packages/contracts/src/previewAutomation.ts`, so widening it is a +**contracts edit** and breaks D12, and it would make `raise_blocker` fail with an error class named +`PreviewAutomationUnavailableError`. It buys nothing in exchange: nothing at registration or +dispatch consults `capabilities`, and the MCP credential is already per-thread and already +all-or-nothing `[V]`. The toolkit's real gate is `store.global.enabled` plus the per-thread armed +record. A **zero-row** path exists (register the toolkit from `coil/index.ts`, using upstream's own +`Effect.withFiber` + `Context.getUnsafe` shape to keep the layer's `R` empty); it is worth one +focused attempt and is **not typechecked**, so budget the one row and treat zero as upside. +**Inherited coupling:** the MCP credential is minted only when `enableAgentBrowserAccess` is true +`[V]`, so turning off **Settings → Integrations → Agent browser access** silently removes all three +tools. The console must name that state rather than render an empty blocker list. **Acceptance:** - `raise_blocker` returns immediately (assert elapsed time, not just the value). - Attribution comes from `McpInvocationContext`, never from an argument. - The answer reaches the agent on the next check-in. +- With `enableAgentBrowserAccess` off, the console says so by name. **Size:** M. --- @@ -414,29 +544,40 @@ cross-loop inbox. **Seam cost:** ~1 row wherever the switch mounts. **Size:** M ## 6. Seam budget -The total upstream cost of the whole feature, which is the number that matters for a fork: +The total upstream cost of the whole feature, which is the number that matters for a fork. +All churn and risk re-measured against merge-base `941acb4f9`, 2026-09-02. + +| Phase | File | Delta | Churn | Risk | Kind | +| ----- | ------------------------------------------- | ----- | ----- | ---- | ---------------------------------------- | +| 1 | `provider/Layers/ClaudeAdapter.ts` | +1 | 23 | 23 | **new row** — additive, read-only | +| 3 | `routes/_chat.$environmentId.$threadId.tsx` | ±0 | 2 | 32 | existing row rewritten in place | +| 4 | `settings/settingsSearch.ts` | ~+13 | 24 | ~312 | **new row** — additive | +| 4 | `settings/SettingsSidebarNav.tsx` | +2 | 19 | ~38 | **new row** — additive | +| 5 | `mcp/McpHttpServer.ts` | +2/−1 | 2 | 6 | **new row** — measured, not `[A]` | +| 6 | mode-switch mount | ~+2 | — | — | **new row** — additive, deferred | +| — | `packages/contracts` | 0 | — | — | D12 — and it is why phase 5 takes path B | +| — | `Sidebar.tsx` / `Sidebar.logic.ts` | 0 | — | — | a loop is a pinned thread | +| — | `server.ts` | 0 | — | — | already has its row | + +**Total: 3 new rows for phases 1–4** (~16 lines), **4 for phases 1–5** (~18 lines), all additive, +plus one deferred. For comparison, the ledger carries 53 rows at +2609/−981. -| Phase | File | Delta | Kind | -| ----- | ------------------------------------------- | ----- | --------------------------------- | -| 1 | `provider/Layers/ClaudeAdapter.ts` | +1 | **new row** — additive, read-only | -| 3 | `routes/_chat.$environmentId.$threadId.tsx` | ±0 | existing row rewritten in place | -| 4 | `settings/settingsSearch.ts` | ~+13 | **new row** — additive | -| 4 | `settings/SettingsSidebarNav.tsx` | +2 | **new row** — additive | -| 6 | mode-switch mount | ~+2 | **new row** — additive, deferred | -| — | `packages/contracts` | 0 | — | -| — | `Sidebar.tsx` / `Sidebar.logic.ts` | 0 | a loop is a pinned thread | -| — | `server.ts` | 0 | already has its row | +Two things the re-measurement changed, both worth stating because they invert the intuition the +earlier drafts built: -**Total: 3 new rows for phases 1–4** (~16 lines), all additive, plus one deferred. For comparison, -the ledger currently carries 53 rows. The row count is what recurs at every sync; the line count is -a one-time write, and review corrected it upward from ~7 after measuring `settingsSearch.ts` -against the real file. +- **Phase 5 got much cheaper.** It was `0–3 rows [A]`, priced as the scary one. Measured, it is one + row at risk **6** — the three `mcp/` files are churn 1–2, the quietest neighbourhood in this plan. +- **Phase 4 got much dearer.** `settingsSearch.ts` went churn 14 → 24, so the two settings rows now + carry ~350 of the plan's ~411 total risk. The most expensive thing in this feature is the + navigation entry, not the reactor, not the adapter hook, and not the agent-facing tools. + +The row count is what recurs at every sync; the line count is a one-time write. --- ## 7. Test strategy -162 cases in TESTS.md, plus the three coverage gates in its §11. Structure: +183 cases in TESTS.md, plus the three coverage gates in its §11. Structure: - **Pure and fast** — `decide.ts` and `guards.ts` are pure so the entire decision table tests without a server or clock. Target **100% branch coverage** on both. @@ -452,15 +593,24 @@ The four to write first: 3. The empty console. _(the degradation property)_ 4. `spent` is never reported as `done`. +Two more that the #125 review added to the front of the list, because each pins a hole that was +open in the docs rather than a behaviour that might regress: + +5. A deadline that has passed stops the loop **while the thread is busy** (guard 4b). Without it, + every ○ guard above the old guard 13 was a way to walk past a deadline. +6. An **auto**-settle does not stand the loop down (guard 5 retired). The fork has already lost an + armed auto-resume to this exact server sweep. + --- ## 8. Rollout - **Default off** at every level: env kill switch, master setting, per-thread arm. - **Dogfood** on this repo's own overnight runs before anything else. -- **Kill path:** `COIL_LOOP_ENABLED=0` stops the fiber forking at layer construction. The master - toggle is re-read every tick _and_ immediately pre-dispatch, so it is a true kill switch rather - than one-tick-stale. +- **Kill path:** `COIL_LOOP_ENABLED=0` stops the fiber forking at layer construction — the only + condition under which no fiber exists. The master toggle is a **guard**: re-read every tick _and_ + immediately pre-dispatch, so it is a true kill switch rather than one-tick-stale, and it stands + loops down without disarming or stopping any of them. - **Blast radius if wrong:** bounded by the mandatory budget (≤20 check-ins/loop) and the armed-loop ceiling (default 3). Worst case is `3 × 20 = 60` unwanted turns, and the reserve-before-dispatch discipline means a broken provider burns budget rather than tight-looping. @@ -469,16 +619,17 @@ The four to write first: ## 9. Risk register -| Risk | Likelihood | Impact | Mitigation | -| --------------------------------------------------------------------------------------------------------------------- | ------------------------------- | ---------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `session_crons` arrives as a cron _expression_, not a fire time, so the `nextFireAtMs` parse is ours and can be wrong | Medium `[A]` | Deference misfires | Narrowed by review: **delivery of the field is settled** `[V - external]`, only the parse is still `[A]`. **Phase 1 is designed to find this out cheaply** — it logs the parse beside the raw `schedule`. Fallback is pure staleness with a longer threshold — worse, but zero upstream cost | -| Upstream ships its own automations feature | Medium | Duplicated work | Zero contracts edits means the fork becomes a _caller_, not a migration. Re-check each sync | -| The `ClaudeAdapter` row conflicts on a sync | Low-Medium | Recurring cost | One additive line beside an existing spread; fails to a type error, not silent drift | -| A loop pushes past a human decision | Low | Trust | Three separate guards (approvals, pending input, plan-ready), each non-consuming | -| Token burn from a runaway loop | Low | Cost | Mandatory budget, deadline, armed ceiling, reserve-before-dispatch, strike detector | -| Model never calls `raise_blocker` | **High** | Console thinner | By design: two of three console sources need no model cooperation. This is the acceptance test | -| `spent` misread as success | Medium | The original problem returns | Distinct colour, distinct word, distinct push copy; asserted in tests | -| `settingsSearch.ts` conflicts | Medium `[V — 3 commits/3 days]` | Small recurring | Append-ordered array, same add/add shape as #29. The #7082 ordering is already satisfied; the residual risk is ordinary and priced | +| Risk | Likelihood | Impact | Mitigation | +| --------------------------------------------------------------------------------------------------------------------- | ------------------------- | ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `session_crons` arrives as a cron _expression_, not a fire time, so the `nextFireAtMs` parse is ours and can be wrong | Medium `[A]` | Deference misfires | Narrowed by review: **delivery of the field is settled** `[V - external]`, only the parse is still `[A]`. **Phase 1 is designed to find this out cheaply** — it logs the parse beside the raw `schedule`. Fallback is pure staleness with a longer threshold — worse, but zero upstream cost | +| Upstream ships its own automations feature | Medium | Duplicated work | Zero contracts edits means the fork becomes a _caller_, not a migration. Re-check each sync | +| The `ClaudeAdapter` row conflicts on a sync | Low-Medium | Recurring cost | One additive line beside an existing spread; fails to a type error, not silent drift | +| A loop pushes past a human decision | Low | Trust | Three separate guards (approvals, pending input, plan-ready), each non-consuming | +| Token burn from a runaway loop | Low | Cost | Mandatory budget, deadline, armed ceiling, reserve-before-dispatch, strike detector | +| Model never calls `raise_blocker` | **High** | Console thinner | By design: two of three console sources need no model cooperation. This is the acceptance test | +| `spent` misread as success | Medium | The original problem returns | Distinct colour, distinct word; asserted in tests (cases 17, 137). **Push copy is designed in the report and not built** — it has no phase and no tests, so it is listed under §3 Out rather than counted as mitigation (issue #125 §B8) | +| `settingsSearch.ts` conflicts | **High** `[V — churn 24]` | Small recurring | Append-ordered array, same add/add shape as #29. The #7082 ordering is already satisfied. Re-measured 2026-09-02 the churn is 24, not 14, and `a19f01fc1` added another entry the same day — so expect this row to conflict most syncs. §6 records the zero-row fallback if it stops being worth it | +| An **auto**-settle or **auto**-anything reads as human intent | **High** `[V]` | A loop silently retires | Guard 5 retired (BACKEND §7). The general rule the fork keeps re-learning: upstream automates a user-only signal, and every guard reading _intent_ from it inverts. Audit each guard against "could a server timer write this?" before adding it | --- @@ -486,7 +637,17 @@ The four to write first: The four places an outside opinion is most valuable. -**Q1 — Is the deference rule right, or too clever?** +**Q1 — Is the deference rule right, or too clever? — RESOLVED 2026-09-02.** +The rule stands, and both of its loose ends are closed. **The deadline is the cap**; no `maxDeferMs` +knob is added, because a second knob would have to be explained in terms of the first and every +value other than "the deadline" describes a run that is nominally armed but knowingly unsupervised. +And **`deadlineAtMs` is mandatory at arm time and is not nullable** — the route returns +`400 deadline_required` and never clamps (D9). So "a loop with no cap to defer to" is not a +reachable state rather than a branch anyone has to handle, and the whole rule is one sentence: _T3 +stands down while a recorded wake's `nextFireAtMs` is at or before the deadline and is not yet +overdue by its grace._ The grace boundary is **inclusive**. The original text follows, for the +reasoning. + T3 stands down while a recorded wake is still pending — any legal delay, not merely one inside the threshold window — **up to the loop's own wall-clock deadline**, and covers the wake once it is overdue by a grace derived from the recorded entry: `max(90s, min(10% of the period, 15min))` for a @@ -499,8 +660,7 @@ would fire the design's strongest trigger against a perfectly healthy thread. **The deadline cap was added by review**, and it replaces a retracted argument that the exposure was bounded without a second rule because `ScheduleWakeup` clamps a delay to an hour. It is not: `CronCreate` writes the same `session_crons` field with an unbounded cron expression, so a recorded -`0 9 * * *` would stand T3 down for a day. Whether the deadline is the right cap, or whether this -wants an explicit `maxDeferMs`, is open along with the rest of Q1. +`0 9 * * *` would stand T3 down for a day. The alternative to all of it is that T3 always paces on its own clock and simply tolerates occasional double-firing. Deference is more correct and more complex. _Is the complexity worth it, @@ -511,7 +671,11 @@ Prototype P7 shape C makes it a cross-thread "needs you" surface. It is barely m useful with loops switched off, and it might be the more valuable feature. _Is the loop the right container for this at all?_ -**Q3 — Is `raise_blocker` worth the MCP toolkit, or should it be a file?** +**Q3 — Is `raise_blocker` worth the MCP toolkit, or should it be a file? — the price is now +measured.** The toolkit costs **one** seam row at risk 6, not the three the archived design +estimated (§6, BACKEND §11). That does not settle the question — a file is still cheaper and still +worse — but it removes the seam argument from the file's side of it. The original follows. + A blocker could be a line the agent appends to `.coil/blockers.jsonl`, read by the same read-only supervisor that stats the done-file. That is provider-agnostic with **zero** MCP work, at the cost of no structured options and no immediate confirmation to the agent. _Cheaper and worse, or cheaper @@ -528,14 +692,15 @@ design?_ Stated so a reviewer can aim at evidence rather than opinion. -| If this turned out to be true | Then | -| ----------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | -| Our parse of `schedule` disagrees with what actually fires | Deference degrades to pure staleness with a longer threshold; the `ClaudeAdapter` row still earns its keep on restart coverage alone | -| `cron_durable` flips to **true** upstream | Claude's crons survive restarts; the durability argument weakens sharply and this feature shrinks to bounds + console | -| Upstream ships a supervision/automations feature | Re-cut against it. The console and the question channel probably survive; the reactor probably does not | -| `AskUserQuestion` becomes non-blocking | D6 collapses — one channel suffices, and `raise_blocker` is unnecessary | -| The user's real loops are almost never blocked on questions | The console is over-built; ship the reactor and the status pill only | -| Pinning is removed or reworked upstream | D3 collapses back to the Direction A/B/C comparison, and C becomes the answer | +| If this turned out to be true | Then | +| ----------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Our parse of `schedule` disagrees with what actually fires | Deference degrades to pure staleness with a longer threshold; the `ClaudeAdapter` row still earns its keep on restart coverage alone | +| `cron_durable` flips to **true** upstream | Claude's crons survive restarts; the durability argument weakens sharply and this feature shrinks to bounds + console | +| Upstream ships a supervision/automations feature | Re-cut against it. The console and the question channel probably survive; the reactor probably does not | +| `AskUserQuestion` becomes non-blocking | D6 collapses — one channel suffices, and `raise_blocker` is unnecessary | +| The user's real loops are almost never blocked on questions | The console is over-built; ship the reactor and the status pill only | +| Pinning is removed or reworked upstream | D3 collapses back to the Direction A/B/C comparison, and C becomes the answer. **Partly realised already**: `f70eeeeb0` reworked pin-vs-settle so a settled loop leaves the pinned block. D3 survived because its load-bearing claim was the zero row count, not the visibility — see the D3 note in §2 | +| Upstream continues threads across restarts by default | D1's durability argument narrows. **Checked**: `5b7d72aad` (#9167) does this, but only for threads with a live `activeTurnId`, only on the intentional self-update path, and only behind a setting that ships **off** — a pending wake is never marked. BACKEND §12 | **One falsifier has been struck.** The table's top entry used to read "`session_crons` is empty/absent in practice on real Claude sessions". It is **refuted**: the binary spreads @@ -548,8 +713,10 @@ wrong — takes its place, and Phase 1 still answers it. ## 12. Immediate next actions -1. **Review this plan** (that is what it is for). -2. Decide Q1–Q4. +1. ~~Review this plan~~ — done, twice: four reviews before #120 merged, then issue **#125**, whose + findings this revision resolves. What changed is listed below. +2. Decide Q1–Q4. **Q1 is resolved** (§10). Q2, Q3 and Q4 remain open; Q3's seam argument is now + measured rather than estimated. 3. Open a consolidated issue; close #42 pointing at it; note the residue of #38 is now covered. 4. Start **Phase 0** — it is pure debt paydown, zero seam, and safe to do before any decision lands. 5. Start **Phase 1** — it answers the biggest remaining `[A]` in the plan, the `schedule` parse, at @@ -557,3 +724,33 @@ wrong — takes its place, and Phase 1 still answers it. 6. When the design is accepted, register its vocabulary (loop, check-in, blocker, deference, held / spent / stalled, iteration ledger) in `docs/coil/CONTEXT.md` — created lazily then, per `docs/coil/agents/domain.md`, not before. + +### What the #125 review changed + +Grouped by whether it changes behaviour, cost, or only the words. + +**Behaviour — four fixes, two of which were latent bugs rather than contradictions:** + +- `deadlineAtMs` is **mandatory and non-nullable**; the route 400s. The null branch in guard 10b — + which meant _no deference at all_, so a deadline-less loop fired on top of a healthy self-pacing + thread — is gone (§A1). +- **Stop conditions moved from guard 13 to guard 4b**, ahead of every non-consuming skip. They were + evaluated only on a tick that had already decided the thread was idle, so a busy self-paced run + walked past its own deadline and a `done` sentinel went unnoticed until the thread went quiet. +- **Guard 5 (`settledOverride !== "settled"`) is retired.** Upstream #8600 made settlement a + server-side sweep with no provenance marker, so the flag no longer carries human intent — the same + correction `autoResume/guards.ts` already made after a timer destroyed an armed week-long resume. +- **`thread.pin` is a promotion, not a decoration** — it emits companion `thread.unsettled` / + `thread.unsnoozed`. Arming a snoozed thread now 400s; disarm unpins only what the loop pinned. + +**Cost — re-measured against the 2026-09-02 merge-base `941acb4f9`:** phase 5 fell from `0–3 rows +[A]` to **1 row at risk 6**; phase 4 rose to ~350 risk and is now the most expensive part of the +feature; the phase 3 row is cheaper than recorded. `store.global.enabled` gained a data model and +routes and **moved from phase 4 to phase 2**, because phases 2 and 3 were otherwise unswitchable. + +**Words — contradictions closed:** the master toggle has one semantic (§A3, a guard, never "no +fiber"); the grace boundary is inclusive everywhere (§A4); D2's "never `session.status`" is scoped +so a PLAN-only reader keeps the long-tool-call case (§A5); the console-precedence reversal is +declared with its reason (§A2); push, mobile row chrome and the run digest move from "designed" to +**Out** (§B8, §B9); `gate_off` gets a verified source and a named render slot (§B10); the archived +design's superseded-by banner names the premises that actually moved (§A6). diff --git a/docs/coil/loops-v2/TESTS.md b/docs/coil/loops-v2/TESTS.md index 6a6bbad17259..a5fe1f0c3dc1 100644 --- a/docs/coil/loops-v2/TESTS.md +++ b/docs/coil/loops-v2/TESTS.md @@ -16,9 +16,11 @@ Written against the conventions already in the tree, not invented: Counts below are cases, not files. **★** marks a case that encodes a bug this design exists to prevent — if you cut scope, do not cut these. -**The total is 162.** Cases inserted into an existing sequence carry a letter suffix rather than -renumbering everything after them (`11b`–`11k`, `70b`–`70i`, `118b`–`118f`, `136b`–`136c`), so a -case number cited elsewhere keeps meaning the same case. +**The total is 183.** Cases inserted into an existing sequence carry a letter suffix rather than +renumbering everything after them (`11b`–`11l`, `45b`–`45d`, `70b`–`70k`, `86b`–`86e`, +`118b`–`118h`, `136b`–`136c`), so a case number cited elsewhere keeps meaning the same case. The +2026-09-02 review (issue #125) added 21 and rewrote four; every one of them is marked +**`[#125]`** so the delta is auditable. --- @@ -57,8 +59,11 @@ decide whether the two schedulers cooperate or fight. inside the run. 11d. `nextFireAtMs` in the past **with** `updatedAt` movement after it → the wake landed; clear it and treat the thread as normally active. Detected immediately, without waiting out `graceMs`. -11e. `nextFireAtMs` overdue by more than `graceMs` **without** `updatedAt` movement → `fire`, -reason `wake_lost`. ★ Inside `graceMs` T3 still stands down. `graceMs` is **derived from the +11e. `nextFireAtMs` overdue by `graceMs` **or more**, without `updatedAt` movement → `fire`, +reason `wake_lost`. ★ **The boundary is inclusive** — guard 10b is `now >= nextFireAtMs + graceMs`, +the same way case 2's threshold is inclusive, and case 11k says so at the other end. `[#125]` an +earlier draft of this case said "more than", which disagreed with both. Strictly inside `graceMs` +T3 still stands down. `graceMs` is **derived from the recorded entry**, not a constant: `max(90s, min(0.10 × period, 15min))` when `recurring: true`, 90s for a one-shot. This is the strongest trigger in the design: an unmet commitment, not an inference — so the grace has to be wide enough that jitter alone never trips it (11k). @@ -75,7 +80,8 @@ that the exposure was bounded without a second rule because the binary clamps a `ScheduleWakeup` takes `delaySeconds`, clamped; `CronCreate` takes a 5-field expression with no hour bound. Unbounded deference therefore stands T3 down for up to 24h on the first, and indefinitely on the second. The deadline is the cap because past it there is nothing left to -defer to; whether that is the right cap, or an explicit `maxDeferMs` is, is open beside Q1. +defer to. `[#125]` **Resolved**: the deadline is the cap and no `maxDeferMs` knob is added +(PLAN Q1). 11k. A `recurring: true` 30-minute wake that lands **2 minutes late** → **no** `wake_lost` and no fire. ★ From the binary's own scheduler text: "recurring tasks fire up to 10% of their period late (max 15 min); one-shot tasks landing on :00 or :30 fire up to 90 s early." The 90s is the @@ -86,15 +92,32 @@ derivation too: a 30-minute period tolerates 3 min, a 20-minute period 2 min, an period 15 min and not more (the cap binds). The floor is inclusive the same way case 2 is: at exactly `graceMs` the wake counts as lost. +11l. `[#125]` A record whose `deadlineAtMs` is the fail-closed decoding default `0` **never +defers** and never fires: guard 4b stops it as `spent` before 10b is reached. ★ Assert the stop +reason, and assert that no `nextFireAtMs` — however near — can produce a `skip` from such a +record. This is the case the old `deadlineAtMs == null` branch got backwards: it read a missing +deadline as "defer to nothing", i.e. fire freely on a healthy self-pacing thread. + ### 1.2 Budget and deadline 12. `checkInsUsed < maxCheckIns` → allowed. 13. `checkInsUsed === maxCheckIns` → `stop("spent")`. 14. `now >= deadlineAtMs` → `stop("spent")` **even when budget remains**. ★ -15. Deadline null → never stops on time. +15. `[#125]` **Rewritten.** `deadlineAtMs` is a `number` and is never null — a null deadline is + not a state (BACKEND §3). What replaces the old "deadline null → never stops on time" case is + its inverse: a record decoded with the fail-closed default `0` → `stop("spent")` on the first + evaluation. ★ A deadline that did not survive a write must mean "over", never "unbounded"; + the opposite default turns one corrupted byte into an unbounded overnight spend. 16. Deadline in the past at arm time is rejected by the route, not silently accepted (see §5). 17. `spent` is returned as `spent`, never as `done`. ★ (assert the literal, not truthiness) +15b. `[#125]` The deadline stops the loop **while the thread is busy** — `busyTurn` true, idle +below `busyIdleMs`, `now >= deadlineAtMs` → `stop("spent")`. ★ Guard 4b is swept before every +○ guard for exactly this reason: with the old ordering a thread that never went idle never +reached the stop check, and a self-paced run strolled through its own deadline indefinitely. +15c. `[#125]` The sentinel is honoured while the thread is busy, for the same reason → +`stop("done")`, not "noticed once it goes quiet". ★ + ### 1.3 Strikes 18. Movement ≥ `productiveMs` after a check-in resets strikes to 0. @@ -129,7 +152,16 @@ Each guard gets: passes-when-satisfied, blocks-when-not, and **the right kind of 31. `armed === false` → skip. 32. Shell `None` → **disarm** (thread deleted). 33. `archivedAt !== null` → disarm. -34. `settledOverride === "settled"` → skip, budget intact. ★ +34. `[#125]` **Inverted.** `settledOverride === "settled"` → the loop **proceeds**; settledness + never blocks a check-in. ★ Guard 5 is retired. Upstream #8600 moved settlement server-side — + `ThreadSettlementReactor` sweeps every minute and dispatches `thread.auto-settle`, which shares + `thread.settle`'s decider case and emits the same event **with no provenance marker** — so the + flag no longer distinguishes "a human is done here" from "a timer fired". `autoResume/guards.ts` + already made this correction after a timer destroyed an armed week-long resume on day 3. + 34b. `[#125]` The failure the retirement prevents, as a scenario: arm a loop, let a simulated + auto-settle write `settledOverride: "settled"`, and assert the loop still checks in. ★ With the + old guard it would have sat armed and silently done nothing until its deadline, then reported + `spent` — a ○ skip never stops, so the failure is invisible rather than loud. 35. `snoozedUntil` in the future → skip, budget intact. ★ 36. `snoozedUntil` in the past → passes. 37. `hasPendingApprovals` → skip. ★ @@ -141,6 +173,15 @@ Each guard gets: passes-when-satisfied, blocks-when-not, and **the right kind of 43. `now - lastCheckIn.firedAtMs < idleMs` → skip, **even if the idle threshold appears met**. ★ (structural anti-tight-loop floor; must hold even when `updatedAt` never bumps) 44. `armedCount >= maxArmedThreads` → skip. + 45b. `[#125]` **Guard 4b is swept before every ○ guard.** A record that is both past its deadline + _and_ rate-limited reports `stop("spent")`, not `skip("rate-limited")`. ★ Assert the returned + decision, because "the loop is held" and "the loop is over" are different words on the console + and the wrong one hides a finished run behind a hold. + 45c. `[#125]` Guard 4b runs after guard 4, not before it: a deleted thread **disarms**, it does not + report `spent`. Order matters in both directions. + 45d. `[#125]` Guard 2 (the master toggle) still precedes 4b: with the toggle off, a loop past its + deadline reports `standing_down` / `disabled` and is **not** stopped. ★ The toggle stands loops + down; it never manufactures terminal states nobody chose. 45. **Guard order is asserted explicitly**: a record that trips several guards reports the _first_ one, because that string is what the console renders. ★ 46. A skip never increments `checkInsUsed` — asserted across every ○ guard in one table-driven case. ★ @@ -174,6 +215,11 @@ Each guard gets: passes-when-satisfied, blocks-when-not, and **the right kind of One case per field — this is the highest-severity footgun in the module, because a whole-file decode failure becomes `EMPTY_STATE` and silently disarms every loop. 62. A corrupt/truncated file → `EMPTY_STATE` and an error log, never a throw at boot. + 61b. `[#125]` Every decoding default is asserted to be the **fail-closed** value, not merely + present: `armed: false`, `deadlineAtMs: 0`, `maxCheckIns: 0`, `crons: null`, + `pinnedByLoop: false`, `global.enabled: false`. ★ Case 61 proves a missing field still decodes; + this proves it decodes to the reading that spends nothing. A default meaning "unbounded" would + turn one truncated write into an unbounded overnight spend. 63. Unknown extra keys are tolerated (forward compatibility with a newer build). 64. Concurrent mutations from two fibers serialize through the `SynchronizedRef` with no lost update. ★ @@ -209,6 +255,14 @@ today's. A fork observability bug must not be able to break a turn. 70g. `SubagentStop` is handled identically to `Stop`. 70h. The record survives a store round-trip (it is the only durable copy of the wake). +70j. `[#125]` A `PostToolUse` callback matched on `ScheduleWakeup` whose `tool_response` +stringifies to something containing `gate_off` writes `degraded: "gate_off"`. ★ The plumbing is +verified — `HookCallbackMatcher.matcher` selects by tool name, and `PostToolUseHookInput` carries +`tool_name` and `tool_response` `[V - external]` — but the response **body** is not, so this is a +substring probe, not a parse. +70k. `[#125]` A `tool_response` that does **not** contain the marker leaves `degraded` untouched — +it is never inferred, never guessed, and a successful call never clears an unrelated degraded +state by accident. ★ A probe that finds nothing must behave exactly like no probe. 70i. The entry's `prompt` arrives **truncated to 1000 characters by the binary**. ★ Assert a longer prompt round-trips as the truncation, and that no fork path treats it as the agent's full prompt — the trigger reads `schedule` and `recurring`; `prompt` is console display text and @@ -227,8 +281,20 @@ receive. non-bypassable; a silent clamp hides a mistake) 76. `POST` arm with `maxCheckIns < 1` → 400. 77. `POST` arm with a deadline in the past → 400. ★ + 77b. `[#125]` `POST` arm with **no** deadline → `400 deadline_required`. ★ Not a clamp and not a + default: a null deadline is not a state (D9, BACKEND §3). Assert the error code, because the + console distinguishes it from the past-deadline 400. 78. `POST` arm when already at `maxArmedThreads` → 400, and **the tick re-checks it too**, so a hand-edited state file cannot exceed the ceiling. ★ + 78b. `[#125]` `POST` arm on a **snoozed** thread → `400 thread_snoozed`, and **no** `thread.pin` is + dispatched. ★ `thread.pin`'s decider case emits companion `thread.unsettled` / + `thread.unsnoozed` events `[V]`, so arming would otherwise silently cancel a snooze the human + set. Assert the absence of the dispatch, not just the status code. + 78c. `[#125]` `POST` arm on a **settled** thread succeeds and pins; the resulting unsettle is + correct, because arming is the human asking for the thread to run. + 78d. `[#125]` `pinnedByLoop` gates the unpin: arming a thread that was **already pinned** records + `pinnedByLoop: false`, and disarming it dispatches **no** `thread.unpin`. ★ Otherwise disarming + a loop removes a pin the user set themselves, and nothing records that it was ever theirs. 79. `POST` disarm on a running loop → disarmed, terminal reason `handed-back`. 80. `POST` re-arm after `spent` → clears terminal, fresh budget. 81. `POST answer` on a blocker → recorded, `deliveredToAgent` false. @@ -238,6 +304,17 @@ receive. 84. Malformed JSON body → 400, no state mutation. 85. Every response shape decodes against its schema (guards against drift with the client). 86. `GET /api/coil/loops` returns every armed loop across projects, ordered deterministically. + 86b. `[#125]` `GET /api/coil/loop/settings` on a fresh install returns the global block with + `enabled: false` and every default populated from config. The route exists in **phase 2**, with + the reactor — shipping "default off behind the master toggle" while only phase 4 could flip it + left phases 2 and 3 unswitchable. + 86c. `[#125]` `POST /api/coil/loop/settings` writes the master toggle durably, and the next tick + observes it. ★ The toggle is re-read every tick _and_ pre-dispatch, so assert both. + 86d. `[#125]` Toggling off leaves every armed loop **armed**: `armed` stays true, `stopped` stays + null, `checkInsUsed` is unchanged, and each reports `standing_down` / `disabled`. ★ Toggling + back on resumes the same loops with the same budgets. The toggle is a guard, not a lifecycle. + 86e. `[#125]` `POST` settings with `maxArmedThreads` below the current armed count is accepted and + the excess loops stand down at the next tick rather than being disarmed — same rule as 86d. --- @@ -295,6 +372,10 @@ receive. 116. `loop_done` writes the terminal state and is equivalent to the sentinel file. 117. `loop_done` from a thread with no loop is a no-op, not a crash. 118. All three tools are unavailable when the global toggle is off. ★ + 118g. `[#125]` With `enableAgentBrowserAccess` off, the MCP credential is never minted `[V]`, so all + three tools vanish. Assert the console renders a **named** degraded state, not an empty blocker + list. ★ A missing question channel that looks like "no questions" is the exact failure mode + `raise_blocker` exists to prevent. ### 7b. Voided questions (upstream #5127) @@ -306,6 +387,14 @@ The agent gets `{}` from upstream; the console must not report that as a human d 118e. The console renders a voided question as still needing attention, with the reason. ★ 118f. A voided question does **not** count as a blocking guard hit on the next tick — the block is gone, so the loop may proceed, but the console still shows it. ★ +118h. `[#125]` **The loop can manufacture its own blocker.** Upstream #8144 added `onUserDialog` +with `supportedDialogKinds: ["resume_return"]` to the same `queryOptions` object phase 1 edits +`[V]`, routing into the same blocking `Deferred` as `AskUserQuestion` and firing on **session +resume** — so a check-in landing on a torn-down session can park the loop on a dialog the loop +itself caused. Assert three things: guard 8 still skips (nudging past a pending input is worse); +the fork's `user-input.requested` record carries the **dialog kind**, so the console can say +"waiting on a session-resume confirmation since 01:04" rather than showing an unexplained idle +loop; and the loop spends nothing while parked and ends `spent`, not `stalled`. ★ --- @@ -338,6 +427,15 @@ Heavier tests; a handful, each replaying a real failure. deadline, strikes and `armedAtMs` all survive and the loop continues from check-in 3. ★ 130. **Reboot storm.** Three armed threads, all long-idle, server restarts. Assert none fire on the first tick and they do not all fire simultaneously afterwards. ★ + 130b. `[#125]` **Upstream's restart continuation.** `5b7d72aad` (#9167) re-establishes a binding + after a self-update and dispatches `session.status: "starting", activeTurnId: null` + synchronously at startup, while the actual `sendTurn` waits on server activation — a real + window in which a continued thread looks idle with no error. Assert the loop does not fire in + it and spends nothing. ★ Two independent mechanisms already cover it and **neither was added + for this**, which is the point of the case: `busyTurn` counts `"starting"`, so the fuse is + `busyIdleMs`; and `processStartedAtMs` floors the idle clock at process start. Also assert the + premise the feature rests on: a thread waiting on a scheduled wake has no `activeTurnId`, so + upstream never marks it for continuation and the durability gap is untouched. 131. **Loop vs auto-resume.** A rate limit arrives while a loop is armed with auto-resume off. Assert the loop does not nudge into the live limit and spends no budget. ★ 132. **Loop vs auto-resume, the other direction.** A pending auto-resume exists; assert the loop @@ -390,3 +488,8 @@ Heavier tests; a handful, each replaying a real failure. without that property fails the suite. - A test that fails if a new field is added to `LoopRecord` without a decoding default. ★ (schema-reflective; this is the failure that silently disarms every loop on the machine) +- `[#125]` **A guard-provenance review, once per guard, written down rather than tested.** Before + any guard is added, answer: _could a server timer write this value?_ Guard 5 died because + `settledOverride` changed answer from "no" to "yes" when upstream #8600 landed, and nothing in the + suite could notice — the old test passed, asserting the wrong behaviour. No test catches a + predicate whose _meaning_ moved, so this is a checklist item, not a case. diff --git a/docs/coil/loops-v2/UPSTREAM-DELTA.md b/docs/coil/loops-v2/UPSTREAM-DELTA.md index cde744091e86..1f6fe72e65d2 100644 --- a/docs/coil/loops-v2/UPSTREAM-DELTA.md +++ b/docs/coil/loops-v2/UPSTREAM-DELTA.md @@ -328,3 +328,104 @@ Undocumented anywhere in this package until now: the binary truncates each entry the agent's full prompt text is wrong — it receives a prefix. Harmless for the `id`, `schedule` and `recurring` fields the deference rule reads; not harmless for anything that wants to match on prompt content, hash it, or round-trip it back into a turn. + +--- + +## 9. Second re-verification — 2026-09-02 (issue #125 §D) + +Two syncs have landed since §7. The merge-base moved `cebac353d` → **`941acb4f9`** (the 2026-09-02 +sync, 182 upstream commits, issue #128), and the seam ledger re-baselined to **53 files, ++2609 / −981** — the same 53 files, so no row was added or retired by the sync itself. + +### 9.1 Both facts #125 flagged as stale are confirmed, and both are now in the fork's tree + +| Change | Effect on this package | +| ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **`f70eeeeb0` (#7969, 2026-08-23) inverted pin-vs-settle.** The contract now reads _"Settled and snoozed threads remain in their respective shelves even when pinned"_, and the sidebar partition is a single chain: **snoozed → settled → pinned → active** `[V]`, mirrored in `apps/mobile/src/features/threads/threadListV2.ts` | **D3 survives, with lower confidence.** Loop-as-pinned-thread no longer keeps a _settled_ loop visible. But D3's load-bearing claim was the **zero row count** in `Sidebar.tsx`, not the visibility, and that is untouched. FINDINGS §A1 and PLAN D3 corrected | +| **`c7222ca4d` (#8144, 2026-08-25) added a second blocking dialog.** `onUserDialog` with `supportedDialogKinds: ["resume_return"]` sits in the _same_ `queryOptions` object phase 1 edits, routes into the same blocking `Deferred` as `AskUserQuestion`, and fires on **session resume** `[V]` | **A real new failure mode**: the loop's own nudge, landing on a torn-down session, can manufacture a pending user-input that guard 8 then treats as a hard skip — the loop causes the thing that parks it. No new guard; the fork's `user-input.requested` record carries the dialog kind so the console can name it, and the deadline ends it. BACKEND §9.1c, TESTS 118h | + +### 9.2 Churn has moved a lot, and it moved the plan's conclusions + +Re-measured with the SEAMS.md recipe against `941acb4f9`. #125 was right that the figures only +reproduced against the older base; they now reproduce against the stated one. + +| File | Churn then | Churn now | Consequence | +| ------------------------------------------- | ---------- | --------- | -------------------------------------------------------------------- | +| `provider/Layers/ClaudeAdapter.ts` | 16 | **23** | Phase 1's row is dearer but still one additive line — risk 23 | +| `settings/settingsSearch.ts` | 14 | **24** | Phase 4 is now **the most expensive phase in the plan** at ~312 risk | +| `settings/SettingsSidebarNav.tsx` | — | **19** | ~38 risk; both settings rows are type-forced, not optional | +| `routes/_chat.$environmentId.$threadId.tsx` | 4 | **2** | Phase 3's row is _cheaper_ than recorded — risk 32, delta still zero | +| `mcp/McpHttpServer.ts` | 6 | **2** | Phase 5 is one row at risk **6** | +| `mcp/McpSessionRegistry.ts` | 7 | **1** | not taken — see §9.3 | +| `mcp/McpInvocationContext.ts` | 3 | **1** | not taken — see §9.3 | +| `settings/SettingsPanels.tsx` | 36 | **43** | still avoided; ~2900 risk if it were not | +| `contracts/settings.ts` | 26 | **38** | still avoided; D12 | + +**The inversion is the finding.** The plan was written expecting phase 5 (agent-facing MCP tools) +to be the expensive, scary one and phase 4 (a settings entry) to be routine. Measured, it is the +other way round: the `mcp/` neighbourhood is the quietest in the plan, and the **navigation entry +carries ~350 of the feature's ~411 total risk**. + +### 9.3 Phase 5's seam cost, measured rather than estimated + +PLAN carried `0–3 rows [A]`; the archived design guessed three files. Three paths exist: + +- **Path A — a gated `"loop"` capability: 4 rows, risk 16. Rejected.** `McpCapability` is backed by + a runtime `Schema.Literal("preview")` in `packages/contracts/src/previewAutomation.ts`, so + widening the union is a **contracts edit** (breaking D12), and it would make `raise_blocker` fail + with an error class named `PreviewAutomationUnavailableError`. It buys nothing: nothing at + registration or dispatch consults `capabilities` — `requireMcpCapability` is one voluntary line + inside one handler helper — and the MCP credential is already per-thread and already + all-or-nothing `[V]`. +- **Path B — an ungated toolkit: 1 row (`McpHttpServer.ts` `+2/−1`, churn 2, risk 6). The decision.** +- **Path C — register from `coil/index.ts`: 0 rows. Worth one attempt, not typechecked.** + `McpServer.toolkit(t)` and `layerHttp` provide the same `McpServer` layer value, so Effect's + MemoMap should hand both the one instance — the property `coil/index.ts` already relies on. The + blocker is typed, not behavioural: a declared `McpInvocationContext` dependency leaks into the + layer's `R`. Upstream's own `registerPreviewSnapshot` shows the way out (`Effect.withFiber` + + `Context.getUnsafe`). Take Path B the moment it fights back; the difference is one row at risk 6. + +**A coupling phase 5 inherits and cannot fix:** the MCP credential is minted only when +`enableAgentBrowserAccess` is true `[V]`, so turning **Settings → Integrations → Agent browser +access** off silently removes `raise_blocker`, `loop_status` and `loop_done`. TESTS 118g. + +### 9.4 `5b7d72aad` (#9167) — upstream continues active threads across a restart + +Arriving on the next sync, and the closest upstream has come to this feature's territory. It does +**not** take D1's premise away, for four reasons, each from the commit `[V]`: + +1. It marks only threads with `session.status === "running" && activeTurnId !== null`. **A thread + waiting on a scheduled wake is neither, so a pending wake is never marked and never continued** — + which is exactly the gap this feature covers. +2. It is opt-in and ships **off** (`continueThreadsAfterServerUpdate` decodes to `false`). +3. It writes its marker only on the **intentional self-update** path. A crash, an OOM, a `kill` or a + machine reboot — the cases a supervisor is for — behave exactly as before. +4. It does not continue the interrupted turn; `activeTurnId` is nulled and a fresh turn is started. + +It does introduce a **double-fire window**: reconciliation dispatches `status: "starting", +activeTurnId: null` synchronously at startup while the `sendTurn` waits on server activation, so a +continued thread looks idle for a real interval with no error and no lease. This design already +covers it twice over — `busyTurn` counts `"starting"` (so the fuse is `busyIdleMs`) and +`processStartedAtMs` floors the idle clock at process start — and **neither mechanism was added for +this**. BACKEND §12; TESTS 130b. + +### 9.5 Everything else re-checked, and still true + +| Claim | Result at `941acb4f9` | +| ----------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | +| `session_crons` / `ScheduleWakeup` / `CronCreate` in server + contracts | **0 files** | +| `options.hooks` set in `ClaudeAdapter` | **0** — the beachhead is still unclaimed | +| `mcp/toolkits/` contents | **`preview` only** | +| the `mcpServers` spread anchor in `queryOptions` | **present**, and `onUserDialog` now sits beside it | +| `pinnedAt` / `thread.pin` / `thread.unpin` in contracts | **present** | +| an actor check on `thread.pin` | **none** — commands carry no actor; the decider guards on archival alone | +| a cron parser dependency anywhere in the repo | **none** — `git grep -i cron -- '*package.json'` and a `cron` search over `pnpm-lock.yaml` are both empty | +| two independent scope-auth mirrors (phase 0's debt) | **still two** — and they are no longer equivalent, see below | + +**One correction to phase 0's brief.** The webPush helper is _not_ strictly more general than the +autoResume one. It is parameterised on scope and returns the session, which autoResume's is not — +but it calls `failEnvironmentAuthInvalid` with **one** argument where autoResume's passes +`EnvironmentAuth.serverAuthDpopFailureReason(error)` as the second. That argument is `3bdf109e2` +(2026-09-02), which landed precisely because the stale one-argument call compiled and drifted +silently. Promoting webPush's body verbatim would re-introduce that bug in a shared helper, for all +three callers. Promote the **union**: parameterised, session-returning, and DPoP-reporting. diff --git a/docs/coil/loops-v2/build-report.mjs b/docs/coil/loops-v2/build-report.mjs index d10ebfca5dc3..c78c5eea9d55 100755 --- a/docs/coil/loops-v2/build-report.mjs +++ b/docs/coil/loops-v2/build-report.mjs @@ -74,7 +74,32 @@ const out = src.replace( }, ); -if (embedded === 0) throw new Error("no EMBED markers matched — the marker syntax has drifted"); +/** + * Assert the COUNT, not merely "at least one". + * + * The old guard only fired when *zero* markers matched, so a single drifted marker — the hint + * regex terminates on `>` and splits on `|`, both of which are easy to type into a caption — + * silently emitted a report missing that frame. Invisible in the browser and invisible in a + * 15k-line diff. Reproduced with 7 of 8 inlined and no error (issue #125 §C11). + * + * Two directions, because they catch different mistakes: a marker that stopped matching, and a + * prototype nobody referenced. + */ +const prototypeFiles = NodeFS.readdirSync(protoDir) + .filter((name) => name.endsWith(".html") && !name.startsWith("_")) + .sort(); + +if (embedded !== prototypeFiles.length) { + throw new Error( + `inlined ${embedded} prototypes but prototypes/ holds ${prototypeFiles.length} ` + + `(${prototypeFiles.join(", ")}) — a marker has drifted, or a prototype was added without one`, + ); +} + +const unreferenced = prototypeFiles.filter((name) => !src.includes(`