diff --git a/docs/plan-durability.md b/docs/plan-durability.md index 98617a53d..58499c606 100644 --- a/docs/plan-durability.md +++ b/docs/plan-durability.md @@ -434,16 +434,82 @@ task's PREPARED record. If the crash happens after close ledger events append but before plan file writes complete, the next `loadPlan()` call detects the hash mismatch and rebuilds plan files from the authoritative ledger. +### Restart reconciliation and authority boundaries (issue #2668) + +Restart reconciliation must keep durable workflow policy separate from +process-local execution authority. The following distinction is intentional: + +| Durable across restart | Process-local and never resurrected as authority | +| --- | --- | +| `.swarm/plan-ledger.jsonl` and its replayed plan identity | The session `/swarm auto-proceed` override | +| The plan execution profile and persisted QA-gate profile for that plan identity | In-memory session context, child handles, timers, and retry/circuit state | +| Ratchet-tighter session QA-gate overrides stored in `.swarm/swarm.db` | | +| Evidence/WAL records, reservation/lease records, and their owner-visible recovery classifications | Live ownership/lease authority from a prior process; it must be re-proven after restart | + +The plan ledger remains authoritative. `plan.json` and `plan.md` are derived +projections, while ratchet-tighter session QA-gate overrides are durable policy +stored in the project database and restored on restart. The session +`/swarm auto-proceed` override remains process-local. Neither a restored QA +override nor a durable lease record grants execution authority: ownership and +live authority must be re-proven, and old child handles are never revived. + +The restart sequence is deliberately ordered: + +1. Snapshot coordination rehydrates the session with a generation/scope fence. + Transient authority is reset; a late result from an older generation cannot + clear or settle newer work. +2. After the plugin manifest is available, the post-resolution coordinator + calls authoritative `loadPlan()` before building the rehydration cache or + exposing a projection-dependent status. This keeps repair and replay off the + bounded plugin-manifest path. +3. The coordinator exposes readiness (`running`, `succeeded`, `superseded`, + `failed`, or `timed_out`) through the status/inspect surfaces. A superseded + attempt is settled but not successful: a fresh current-generation attempt + must perform recovery instead of reusing it. A failed or timed-out attempt + is retried only after the previous attempt has settled; an unsettled attempt + remains unknown rather than being guessed healthy. +4. Durable evidence, WAL settlement, and reservation records are classified + for the owner. Recovery may release an expired reservation only with + corroborated owner absence; expiration by itself is not proof. + +#### Missing or corrupt projections + +Never repair `plan.json` or `plan.md` by hand. A missing, stale, or malformed +projection is a derived-state problem: + +- `loadPlan()` replays `.swarm/plan-ledger.jsonl`, compares the projection + identity/hash, and rewrites the derived projection when the ledger is valid. +- A corrupt ledger suffix is quarantined and replay resumes from the last + valid event. Restart-owned replay rechecks its exact hydration authority + after the integrity-read await and immediately before publishing the unique + quarantine side file; a superseded restart leaves no stale quarantine + artifact. If the remaining history is not sufficient to prove the plan + identity, the plan stays unknown and operator recovery is required. +- The persisted QA profile is read by exact plan identity. A missing or + mismatched profile is not silently replaced by a session override. +- Inspect with `/swarm status`, `/swarm diagnose`, `get_approved_plan`, and + `get_qa_gate_profile`; use `/swarm recover --coordination` for coordination + readiness and `/swarm recover ` for an owner-visible recovery + classification. See the [recovery runbook](troubleshooting/recovery-runbook.md) + for the decision table. + +Replay and accepted recovery are idempotent: repeating the same restart or +recovery observation must not mint a new identity, append a duplicate ledger +decision, revive a prior process's authority, or erase an uncertain external +effect. Unknown, ambiguous, and corrupt states remain visible until evidence +supports a bounded transition. + ## Corruption Handling If a ledger entry fails validation: -1. The bad suffix is **quarantined** to `.swarm/plan-ledger.quarantine` +1. The bad suffix is **quarantined** to a unique + `.swarm/plan-ledger.quarantine..` side file 2. Replay continues from the last valid event ``` .swarm/plan-ledger.jsonl ← continues with clean events -.swarm/plan-ledger.quarantine ← bad entries isolated (never replayed) +.swarm/plan-ledger.quarantine.* ← bad entries isolated (never replayed) ``` ## Migration from v6.41.x @@ -678,16 +744,23 @@ Three layers, with distinct authority: - **Process-local** — the four live maps themselves and `pendingRehydrations` (bounded by their pre-existing lifecycle mechanisms: the 2-hour idle-TTL sweep for sessions, `resetSwarmState` for the rest), plus the per-project - registries in `src/session/hydration-ownership.ts` (hydration generation - counters, per-project rehydration caches, per-project hydrated-aggregate - key sets — FIFO-capped at 32 entries each, with the directory→key memo - FIFO-capped at 64). All are cleared by `resetSwarmState`. + registries in `src/session/hydration-ownership.ts` (hydration authority + records, per-project rehydration caches, per-project hydrated-aggregate key + sets — FIFO-capped at 32 entries each, with the directory→key memo + FIFO-capped at 64). `resetSwarmState` clears the bounded registries, but not + the process-monotonic authority epoch: reusing an epoch after reset could + revive a stale pre-reset token. - **Project-local** — ownership stamps on each session: `owningProjectKey` (the canonical project root that created or restored the session; never serialized — the hydrating directory defines it, snapshot - bytes never do) and `hydrationStamp` (the per-project hydration generation - the session was created/restored at). Sessions created without a directory - are unowned and survive every hydration (fail-open toward preservation). + bytes never do), `hydrationStamp` (the per-project hydration generation), and + its paired `hydrationAuthorityEpoch` (the process-local authority + incarnation). The generation and epoch together identify the authority that + created/restored the session; an older epoch is stale even when its numeric + generation is larger after bounded-record eviction or reset. All three + fields are process-local and never serialized. Sessions created without a + directory are unowned and survive every hydration (fail-open toward + preservation). - **Authoritative** — the durable ledger and SQLite snapshot store. Hydration only READS them and never writes them (invariant 5); a re-hydration replaces the project's own snapshot-derived sessions from the durable read, nothing @@ -696,16 +769,21 @@ Three layers, with distinct authority: Rules a hydration for project K follows (`rehydrateState`, `src/session/snapshot-reader.ts`): -1. **Fence (generation):** each initiation (`loadSnapshot` entry, +1. **Fence (authority):** each initiation (`loadSnapshot` entry, `startSnapshotCoordinationInitialization`, retry) captures a scope - `{projectKey, generation}` from a monotonic per-project counter. An apply - whose generation is older than the project's current counter is refused - with zero mutation — a timed-out initializer settling late cannot publish - over the state of any newer hydration. -2. **Stamp (recency):** an accepted apply at generation `g` evicts only - sessions with `owningProjectKey === K` AND `hydrationStamp <= g`. A live - session created after `g` began carries stamp `g+1` and survives its own - project's in-flight hydration. + `{projectKey, generation, authorityEpoch}`. The per-project generation + orders retained records; the process-monotonic epoch makes the scope + non-reusable after FIFO eviction, reset, and reinsertion. An apply whose + exact authority is no longer current is refused with zero mutation — a + timed-out initializer settling late cannot publish over the state of any + newer hydration, even when its numeric generation is reused. +2. **Stamp (recency):** an accepted apply at authority + `{ generation: g, authorityEpoch: e }` evicts sessions with + `owningProjectKey === K` when their epoch is not `e`, or when their paired + `hydrationStamp <= g`. A live session created after `g` began carries the + current epoch and stamp `g+1`, so it survives its own project's in-flight + hydration. Comparing the epoch before the numeric stamp closes the ABA + window where FIFO eviction or reset reintroduces K at generation 1. 3. **Scope of mutation:** another project's sessions, unowned sessions, and `toolAggregates` keys the project never published are untouched. `toolAggregates` replacement is limited to the keys K's previous hydration diff --git a/docs/releases/pending/2668-restart-policy-reconciliation.md b/docs/releases/pending/2668-restart-policy-reconciliation.md new file mode 100644 index 000000000..91dd31c27 --- /dev/null +++ b/docs/releases/pending/2668-restart-policy-reconciliation.md @@ -0,0 +1,49 @@ +# Restart policy reconciliation + +## What changed + +- Restart hydration now has an explicit authority boundary: the plan ledger, + durable execution profile, persisted QA-gate profile, ratchet-tighter + session QA-gate overrides stored in `.swarm/swarm.db`, evidence, recovery + WALs, and lease records are durable inputs. The session `/swarm auto-proceed` + override, active ownership, live lease authority, child handles, timers, and + retry/circuit state remain process-local and are not revived as execution + authority. +- The post-resolution restart coordinator uses authoritative `loadPlan()` before + projection/cache inspection. Missing or stale `plan.json`/`plan.md` files are + regenerated from `.swarm/plan-ledger.jsonl`; invalid ledger suffixes remain + quarantined instead of being silently discarded. An initializer that loses + its exact hydration authority reports `superseded`, never reusable success, + so a current-generation retry still performs recovery. Authority includes a + process-monotonic epoch, preventing FIFO eviction or reset from reviving an + old callback when a numeric per-project generation is reused. +- Session recency, workflow-cache entries, and hydrated aggregate ownership + pair their numeric state with that process-local authority epoch, so an older + session/cache/aggregate owner cannot survive a hydration merely because its + numeric stamp or generation is larger after a generation is reused (ABA). +- Corrupt-ledger replay carries the same authority fence through every recovery + replay call site (including schema-invalid/missing projections, `savePlan`, + and `rebuildPlan`) and its integrity/quarantine path. Spec-staleness output + and save/rebuild marker publications use the same post-await fence, so an + obsolete restart cannot publish misleading recovery artifacts after + supersession. +- Interrupted, cancelled, stale, ambiguous, corrupt, live-wedge, and + old-generation results remain owner-visible. Recovery releases or repairs + local state only when the durable evidence proves that transition; uncertain + provider or worktree effects stay uncertain. +- The recovery runbook documents the operator restart/inspect/recover sequence; + the registered host journey covers policy/identity and task inspection, with + settlement categories exercised directly by its deterministic classifier + cases. + +## Operator guidance + +After a restart, inspect `/swarm status`, `/swarm diagnose`, +`get_approved_plan`, and `get_qa_gate_profile`. Use `/swarm recover +--coordination` only after a failed or timed-out coordination attempt has +settled, and use `/swarm recover ` for receipt-backed stale or +live-wedge repair. Do not hand-edit derived plan projections or force a live +foreign owner. See [`docs/troubleshooting/recovery-runbook.md`](../../troubleshooting/recovery-runbook.md). + +No new plan identity or ledger migration is required. Repeated replay and +accepted recovery are idempotent. diff --git a/docs/releases/pending/issue-2667-hydration-project-owned-generation-fenced.md b/docs/releases/pending/issue-2667-hydration-project-owned-generation-fenced.md index a18341a7c..8da73ea94 100644 --- a/docs/releases/pending/issue-2667-hydration-project-owned-generation-fenced.md +++ b/docs/releases/pending/issue-2667-hydration-project-owned-generation-fenced.md @@ -1,4 +1,4 @@ -# Hydration is project-owned and generation-fenced +# Hydration is project-owned and authority-fenced ## What @@ -14,12 +14,13 @@ no longer publish over newer state: its own snapshot state. Calling it without a directory keeps the legacy clear-all (direct-test path only). - Each hydration initiation (`loadSnapshot` entry, SQLite snapshot - coordination initialization and retries) captures a generation scope from a - monotonic per-project counter (`src/session/hydration-ownership.ts`). An - apply whose generation has been superseded is refused with zero mutation — - a timed-out initializer that settles late can no longer wipe state written - by a newer generation. Live sessions created while a hydration is in flight - carry a stamp above it and survive that hydration's own apply. + coordination initialization and retries) captures an exact authority scope + from `src/session/hydration-ownership.ts`: a per-project generation plus a + process-monotonic epoch that cannot be reused after bounded-registry eviction + or reset. An apply whose authority has been superseded is refused with zero + mutation — a timed-out initializer that settles late can no longer wipe + state written by a newer generation. Live sessions created while a hydration + is in flight carry a stamp above it and survive that hydration's own apply. - The plan/evidence rehydration cache is per-project instead of a process-global singleton: whichever project built the cache last no longer feeds foreign workflow states to another project's new sessions. @@ -45,8 +46,10 @@ another project's sessions (issue #2667). - New session fields `owningProjectKey` and `hydrationStamp` are never serialized; the snapshot field-parity guard covers them. - The new per-project registries are bounded (FIFO, 32 projects) with an - explicit `resetSwarmState` reset path; canonical project keys resolve - through a bounded memo (one realpath per directory spelling per process — - no new init-path filesystem cost). + explicit `resetSwarmState` reset path. The small authority-epoch scalar is + deliberately process-monotonic across resets so an old token can never + become current again. Canonical project keys resolve through a bounded memo + (one realpath per directory spelling per process — no new init-path + filesystem cost). - Ownership and generation rules are documented in `docs/plan-durability.md` § Live-State Ownership and Hydration Fencing. diff --git a/docs/testing/execute-journey.md b/docs/testing/execute-journey.md index b3a22a614..492952814 100644 --- a/docs/testing/execute-journey.md +++ b/docs/testing/execute-journey.md @@ -49,11 +49,57 @@ A plan whose identity (swarm/title) was mutated after approval is refused by - Deterministic transport: a constructor-injected `ScriptedHostClient` (real SDK response shapes) records every host call; native-task child outputs are scripted at the hook boundary. No live model, no network. -- Tests: `tests/unit/execute-journey/j01…j07` (per-file CI on all three - OSes). j04's restart uses `resetSwarmStatePreservingSingletons()` between - in-process boots — a real second OS process would run the same hydration - path; this in-process limitation is the disclosed boundary of the restart - claim. +- Tests: `tests/unit/execute-journey/j01…j08` (per-file CI on all three + OSes). j04's restart and j08's policy-reconciliation restart use + `resetSwarmStatePreservingSingletons()` between in-process boots — a real + second OS process would run the same hydration path; this in-process + limitation is the disclosed boundary of the restart claim. + +## Restart policy reconciliation journey (#2668) + +The registered host fixture +`tests/unit/execute-journey/j08-restart-policy-reconciliation.test.ts` +qualifies the boundary between durable policy and ephemeral execution +authority. It uses the same `execute-journey-driver.ts`, real +`OpenCodeSwarmPlugin.server()`/`bootSwarmPluginHost`, disposable git project, +XDG-hermetic environment, and constructor-injected `ScriptedHostClient` as the +other journey fixtures. It does not use a live model or network. + +The deterministic sequence is: + +1. Boot the plugin, create and approve a plan, persist its execution profile, + and persist the QA-gate profile for the exact plan identity. +2. Set a session-only QA or auto-proceed override, then start a scoped coder + dispatch and leave its durable evidence as the restart input. +3. Restart the host. The post-resolution coordinator must replay the + authoritative plan ledger before projection/cache inspection, while the + snapshot path applies its generation fence and clears ephemeral authority. +4. Inspect `get_approved_plan`, `get_qa_gate_profile`, and the registered task + inspection path. The durable execution/QA policy and plan identity must + remain; the prior session override, ownership, live lease authority, child + handle, and timer must not be treated as permission to execute. Any durable + lease record is recovery evidence only. +5. Exercise the settlement classifier directly with deterministic owner states: + provably dead work is `stale`, a live or foreign owner is `ambiguous`, and + unreadable evidence is `corrupt`. The registered journey separately proves + that an old-generation late result is refused without clearing newer work. + Expired leases are released only when owner absence is corroborated. + +Separate frozen acceptance cases remove or corrupt a derived projection and +run post-resolution coordination again. A valid ledger must rebuild the +projection without a new plan identity or duplicate recovery decision; +insufficient or corrupt authoritative history remains visibly unknown. + +Run the registered fixture in isolation with: + +```sh +bun test tests/unit/execute-journey/j08-restart-policy-reconciliation.test.ts +``` + +The report must retain evidence for both boots, the exact plan binding, the +persisted QA profile, registered task-inspection output, and the direct typed +interrupted/cancelled/uncertain classifier cases. A passing fixture does not +convert an uncertain provider or worktree effect into a local success claim. ## Executed host/runtime cells diff --git a/docs/troubleshooting/recovery-runbook.md b/docs/troubleshooting/recovery-runbook.md index 41084645d..3978dc4cd 100644 --- a/docs/troubleshooting/recovery-runbook.md +++ b/docs/troubleshooting/recovery-runbook.md @@ -1,6 +1,10 @@ # Task recovery runbook -A compact operator runbook for `/swarm diagnose` and `/swarm recover` (issue #2665): one shell-correct invocation per supported shell, the meaning of each recovery status category, and the boundary between deterministic repair and an external side effect that needs human reconciliation. +A compact operator runbook for restart policy reconciliation and task recovery +(issues #2668 and #2665): one shell-correct invocation per supported shell, the +meaning of each owner-visible status category, and the boundary between +deterministic repair and an external side effect that needs human +reconciliation. ## Shell-correct invocations @@ -13,10 +17,69 @@ Recovery runs through the host command path (the OpenCode TUI/GUI command line). | Git Bash / MSYS (headless) | `opencode run --dir "//swarm recover "` | The leading slash is **doubled**: MSYS path conversion rewrites a single leading-slash argument to a path under the Git install root (`C:/Program Files/Git/swarm recover …`) before OpenCode ever sees it. | | Shell-neutral CLI | `bunx opencode-swarm run recover [--force]` | Identical in every shell; `--force` is an operator assertion that no dispatch is genuinely in flight. | +For coordination readiness, use the same forms with `recover --coordination` +(for example, `bunx opencode-swarm run recover --coordination`). This retries a +failed or timed-out post-resolution coordination attempt; it must not be used +while the previous attempt is still unsettled. + Known MSYS argument friction: do **not** set `MSYS_NO_PATHCONV=1` for the whole invocation. It silences the `/swarm` rewrite but also stops converting `--dir /c/...` style arguments, so every project-path resolution breaks. Doubling only the message argument's leading slash is the shell-correct form. There is deliberately no runtime shell detection — the forms above are documented, not sniffed. `/swarm diagnose` follows the same rules (`'/swarm diagnose'` in PowerShell, `"//swarm diagnose"` in Git Bash, `bunx opencode-swarm run diagnose` on the CLI). +## Restart, inspect, recover + +Use this order after a crash, host restart, or a report that policy and +execution state disagree: + +1. Restart or reopen the host and allow snapshot coordination to report a + readiness state. A `running` or otherwise unsettled attempt is still + unknown; do not start a second coordination retry. +2. Inspect `/swarm status` for coordination and background-work state, then + run `/swarm diagnose` for read-only task classifications. Use + `get_approved_plan` to inspect the plan replay and + `get_qa_gate_profile` to inspect the persisted QA profile for the exact plan + identity. The latter deliberately excludes session-only QA overrides. +3. If the projection is missing, stale, or malformed, do not edit + `.swarm/plan.json` or `.swarm/plan.md`. `loadPlan()` replays + `.swarm/plan-ledger.jsonl`, quarantines an invalid ledger suffix, and + regenerates valid derived projections. If replay cannot prove identity, + leave the plan unknown and escalate for manual reconciliation. +4. If coordination is `superseded`, `failed`, or `timed_out` and the prior + attempt has settled, run `/swarm recover --coordination` and inspect status + again. A superseded attempt deliberately remains non-successful so its stale + recovery cannot suppress a current-generation retry. +5. For a task classified as `stale` or `live_wedge`, run + `/swarm recover `. This consumes only the receipt-backed local + repair and is idempotent. For `ambiguous`, `corrupt`, or any uncertain + external effect, stop and reconcile with the owning process, provider, or + worktree before asserting success. + +Restart restores durable plan and QA policy, not execution authority. Session +overrides, active ownership, live lease authority, child handles, timers, and +retry/circuit state from the prior process are not evidence that a new process +may execute. Durable lease records and expiry tombstones remain recovery +evidence, not permission to reuse the lease. +An expired lease is not proof of owner absence; maintenance releases it only +when the owner-absence evidence is sufficient. A late result from an older +workflow generation is rejected without clearing newer work. + +### Owner-visible restart states + +| State | Meaning | Operator action | +|---|---|---| +| `stale` | The recorded owner is gone and local repair is deterministic. | Run `/swarm recover `; re-check status. | +| `ambiguous` | A live foreign or current-process owner may still be executing, so the external effect is uncertain. | Do not force a foreign owner; inspect that process/provider. | +| `corrupt` | A receipt or projection cannot be trusted. | Preserve the evidence, replay from the ledger where possible, and reconcile manually if identity remains unproven. | +| `live_wedge` | Settlement succeeded but the Stage A receipt is missing despite green proof. | Run task recovery; it records the justified repair and never reruns the coder. | +| expired lease | The lease deadline passed, but expiry alone does not establish owner absence. | Wait for corroboration; do not treat the lease as freely reusable. | +| old-generation late result | A result belongs to a prior restart generation. | Reject it; preserve the newer generation's state. | +| `running` / `superseded` / `timed_out` coordination | Readiness is not yet authoritative, the attempt lost its generation, or the bounded attempt timed out. | Do not overlap retries; use `--coordination` only after the prior attempt settles. | + +The distinction between `ambiguous`, `corrupt`, and `stale` is intentional: +unknown facts remain visible rather than being converted into a successful +terminal state. See [plan durability](../plan-durability.md) for the ledger and +projection contract. + ## Status categories `/swarm diagnose` reports each task's recovery facts under the **Coder Settlements** row using these categories (with the task, its owning transition id, and its workflow generation): diff --git a/scripts/registry-citation-baseline.json b/scripts/registry-citation-baseline.json index 63539b779..9f78c27eb 100644 --- a/scripts/registry-citation-baseline.json +++ b/scripts/registry-citation-baseline.json @@ -288,20 +288,6 @@ "kind": "out-of-range", "note": "Pre-existing debt, not approved drift: \"closePlanTerminalState\" exists in src/plan/manager.ts but not inside :2090 (plan-projections.writerCitations[0])." }, - { - "rowId": "plan-projections", - "file": "src/plan/manager.ts", - "identifier": "loadPlan", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"loadPlan\" exists in src/plan/manager.ts but not inside src/plan/manager.ts:658 (plan-projections.readerCitations[0])." - }, - { - "rowId": "plan-projections", - "file": "src/plan/manager.ts", - "identifier": "loadPlanJsonOnly", - "kind": "out-of-range", - "note": "Pre-existing debt, not approved drift: \"loadPlanJsonOnly\" exists in src/plan/manager.ts but not inside :366 (plan-projections.readerCitations[0])." - }, { "rowId": "plan-projections", "file": "src/plan/manager.ts", diff --git a/scripts/retention-registry.data.ts b/scripts/retention-registry.data.ts index d1fa8de25..f30c2a7c9 100644 --- a/scripts/retention-registry.data.ts +++ b/scripts/retention-registry.data.ts @@ -2004,7 +2004,7 @@ export const RETENTION_REGISTRY: readonly RetentionRow[] = [ 'src/commands/rollback.ts — lifecycle-locked checkpoint projection publication with prior-byte compensation after authoritative re-root', 'src/commands/reset.ts — lifecycle-locked critical projection deletion with prior-byte compensation when authority cleanup aborts', ], - readerCitations: ['src/plan/manager.ts:658 loadPlan — full-file with auto-heal + ledger-replay fallback, async; :366 loadPlanJsonOnly'], + readerCitations: ['src/plan/manager.ts:715 loadPlan — full-file with auto-heal + ledger-replay fallback, async; :398 loadPlanJsonOnly'], schemaVersion: 'plan schema (projections of the ledger)', stateClass: 'derived-rebuildable', privacyClass: 'content', @@ -3483,7 +3483,7 @@ export const RETENTION_REGISTRY: readonly RetentionRow[] = [ canonicalRoot: 'project-swarm', writerModules: ['src/session/snapshot-writer.ts', 'src/session/snapshot-store.ts', 'src/session/session-start-store.ts', 'src/services/context-budget-service.ts'], writerCitations: [ - 'src/session/snapshot-writer.ts:519 writeSnapshot — per-key SQLite snapshot authority via snapshot-store with serialized post-commit projection', + 'src/session/snapshot-writer.ts:534 writeSnapshot — per-key SQLite snapshot authority via snapshot-store with serialized post-commit projection', 'src/session/snapshot-store.ts writeSnapshotRows — FULL transaction with per-session tombstones and cross-process-safe disjoint updates', 'src/session/session-start-store.ts:6 recordSessionStart — append flag a, fail-open', 'src/services/context-budget-service.ts:196 writeBudgetState — bunWrite + cache invalidation', diff --git a/src/db/qa-gate-session-override.ts b/src/db/qa-gate-session-override.ts index b053e1dd7..dd2e27f35 100644 --- a/src/db/qa-gate-session-override.ts +++ b/src/db/qa-gate-session-override.ts @@ -24,24 +24,6 @@ import { import { getProjectDb, projectDbExists } from './project-db.js'; import { DEFAULT_QA_GATES, type QaGates } from './qa-gate-profile.js'; -/** - * Test-only dependency-injection seam — see `gitignore-warning.ts:_internals` - * for the rationale (`mock.module` from `bun:test` leaks across files in - * Bun's shared test-runner process). Mutating this local object is - * file-scoped and trivially restorable via `afterEach`. - */ -export const _internals: { - getOverrideForSession: typeof getOverrideForSession; - setOverrideForSession: typeof setOverrideForSession; - clearOverrideForSession: typeof clearOverrideForSession; - clearAllSessionOverrides: typeof clearAllSessionOverrides; -} = { - getOverrideForSession, - setOverrideForSession, - clearOverrideForSession, - clearAllSessionOverrides, -}; - interface QaGateSessionOverrideRow { session_id: string; gates: string; @@ -194,6 +176,31 @@ export function clearAllSessionOverrides(directory: string): number { ); } +/** Conservative SQLite bind budget for orphan-row deletion statements. */ +const ORPHAN_DELETE_BATCH_SIZE = 500; + +function deleteOrphanOverrideBatch( + db: ReturnType, + sessionIds: string[], +): number { + if (sessionIds.length === 0) return 0; + const placeholders = sessionIds.map(() => '?').join(', '); + return db.run( + `DELETE FROM qa_gate_session_override WHERE session_id IN (${placeholders})`, + sessionIds, + ).changes; +} + +/** + * Dependency-injection seam used by sweepOrphanOverrides and its bounded-batch + * regression test. The test mutates and restores this seam in afterEach rather + * than using mock.module, which can leak across Bun test files; see + * gitignore-warning.ts:_internals for the pattern and rationale. + */ +export const _internals: { + deleteOrphanOverrideBatch: typeof deleteOrphanOverrideBatch; +} = { deleteOrphanOverrideBatch }; + /** * Delete override rows whose session is no longer live in this project * (#2668 orphan-row reaper). `sweepStaleSessions` can evict a stale session @@ -224,12 +231,15 @@ export function sweepOrphanOverrides( .map((row) => row.session_id) .filter((sessionId) => !keepSessionIds.has(sessionId)); let removed = 0; - for (const sessionId of orphans) { - const result = db.run( - 'DELETE FROM qa_gate_session_override WHERE session_id = ?', - [sessionId], + for ( + let offset = 0; + offset < orphans.length; + offset += ORPHAN_DELETE_BATCH_SIZE + ) { + removed += _internals.deleteOrphanOverrideBatch( + db, + orphans.slice(offset, offset + ORPHAN_DELETE_BATCH_SIZE), ); - removed += result.changes; } return removed; }, diff --git a/src/observability/catalog.ts b/src/observability/catalog.ts index b2478bc17..f914751d1 100644 --- a/src/observability/catalog.ts +++ b/src/observability/catalog.ts @@ -969,7 +969,7 @@ const CATALOG_SOURCE: readonly (readonly [string, CatalogEntryInput])[] = [ severity: 'notice', // Only an attempt counter, a hash PREFIX and a delay. No identifiers. privacyClass: 'operational', - producer: 'src/plan/manager.ts:359', + producer: 'src/plan/manager.ts:376', consumers: NO_CONSUMERS, futureOwnerIssue: ISSUE_SINK, retentionOwnerIssue: ISSUE_PLAN_EVIDENCE_RETENTION, @@ -983,7 +983,7 @@ const CATALOG_SOURCE: readonly (readonly [string, CatalogEntryInput])[] = [ severity: 'warning', // Carries `directory` and a free-text filesystem error. privacyClass: 'sensitive', - producer: 'src/plan/manager.ts:2065', + producer: 'src/plan/manager.ts:2195', consumers: NO_CONSUMERS, futureOwnerIssue: ISSUE_SINK, retentionOwnerIssue: ISSUE_PLAN_EVIDENCE_RETENTION, @@ -997,7 +997,7 @@ const CATALOG_SOURCE: readonly (readonly [string, CatalogEntryInput])[] = [ severity: 'error', // Carries a free-text filesystem error message. privacyClass: 'sensitive', - producer: 'src/plan/ledger.ts:1609', + producer: 'src/plan/ledger.ts:1620', consumers: NO_CONSUMERS, futureOwnerIssue: ISSUE_SINK, retentionOwnerIssue: ISSUE_PLAN_EVIDENCE_RETENTION, diff --git a/src/plan/ledger.ts b/src/plan/ledger.ts index 9f2a1a011..0996c2ca7 100644 --- a/src/plan/ledger.ts +++ b/src/plan/ledger.ts @@ -1032,6 +1032,7 @@ export async function initLedger( planId: string, initialPlanHash?: string, initialPlan?: Plan, + options?: { preCommitCheck?: () => void }, ): Promise { assertProjectRoot(directory); const ledgerPath = getLedgerPath(directory); @@ -1099,6 +1100,7 @@ export async function initLedger( const tempPath = `${ledgerPath}.tmp.${Date.now()}.${Math.floor(Math.random() * 1e9)}`; const line = `${JSON.stringify(event)}\n`; + options?.preCommitCheck?.(); writeFileFsyncedThenRename(tempPath, ledgerPath, line); // New projects also spend the carrying release in file-shadow mode. Keeping @@ -1108,6 +1110,9 @@ export async function initLedger( if (hasSqliteLedger(directory)) { const priorMode = getPlanLedgerState(directory)?.authorityMode ?? 'file_shadow'; + // The predicate belongs outside the optional SQLite-shadow catch so a + // superseded recovery cannot be downgraded to an availability warning. + options?.preCommitCheck?.(); try { replaceSqliteLedger(directory, { canonicalEvents: initialized.lines, @@ -1347,6 +1352,8 @@ export async function appendLedgerEvent( */ expectedLedgerHash?: string; planHashAfter?: string; + /** Synchronous authority fence inside the evidence lock. */ + preCommitCheck?: () => void; }, ): Promise { assertProjectRoot(directory); @@ -1432,6 +1439,7 @@ export async function appendLedgerEvent( const authorityMode = state?.authorityMode ?? 'file_shadow'; if (authorityMode === 'sqlite') { + options?.preCommitCheck?.(); try { const existing = sqliteEventsAsLedger(directory); appendSqliteLedger(directory, { @@ -1465,6 +1473,7 @@ export async function appendLedgerEvent( throw new Error('Ledger not initialized. Call initLedger() first.'); } const existingContent = fs.readFileSync(ledgerPath); + options?.preCommitCheck?.(); writeFileFsyncedThenRename( tempPath, ledgerPath, @@ -1540,6 +1549,7 @@ export async function appendLedgerEventWithRetry( maxRetries?: number; backoffMs?: number; verifyValid?: () => Promise | boolean; + preCommitCheck?: () => void; }, ): Promise { const maxRetries = options.maxRetries ?? 3; @@ -1552,6 +1562,7 @@ export async function appendLedgerEventWithRetry( return await appendLedgerEvent(directory, eventInput, { expectedHash: currentExpected, planHashAfter: options.planHashAfter, + preCommitCheck: options.preCommitCheck, }); } catch (error) { if (!(error instanceof LedgerStaleWriterError) || attempt >= maxRetries) { @@ -1657,6 +1668,8 @@ export async function takeSnapshotEvent( expectedSeq?: number; /** Previous durable hash used to preserve the ledger hash chain. */ expectedLedgerHash?: string; + /** Synchronous authority fence immediately before ledger append. */ + preCommitCheck?: () => void; }, ): Promise { const payloadHash = @@ -1674,6 +1687,7 @@ export async function takeSnapshotEvent( snapshotPayload.approval = options.approvalMetadata; } const planId = derivePlanId(plan); + options?.preCommitCheck?.(); return appendLedgerEvent( directory, { @@ -1686,6 +1700,7 @@ export async function takeSnapshotEvent( planHashAfter: options?.planHashAfter, expectedSeq: options?.expectedSeq, expectedLedgerHash: options?.expectedLedgerHash, + preCommitCheck: options?.preCommitCheck, }, ); } @@ -1883,6 +1898,7 @@ export async function replacePlanLedgerWithRoot( directory: string, plan: Plan, source: string, + options?: { preCommitCheck?: () => void }, ): Promise { assertProjectRoot(directory); const validated = PlanSchema.parse(plan); @@ -1894,6 +1910,7 @@ export async function replacePlanLedgerWithRoot( async () => { const ledgerPath = getLedgerPath(directory); if (fs.existsSync(ledgerPath)) { + options?.preCommitCheck?.(); archiveLegacyLedger(directory, fs.readFileSync(ledgerPath)); } const planHash = computePlanLedgerHash(validated); @@ -1916,7 +1933,12 @@ export async function replacePlanLedgerWithRoot( const priorMode = getPlanLedgerState(directory)?.authorityMode ?? 'file_shadow'; if (priorMode === 'file_shadow') { + options?.preCommitCheck?.(); writePortableLedger(directory, [line]); + // SQLite shadow publication is optional in file-shadow mode, but an + // authority predicate must still propagate rather than be swallowed by + // the availability catch below. + options?.preCommitCheck?.(); try { replaceSqliteLedger(directory, { canonicalEvents: [line], @@ -1935,6 +1957,7 @@ export async function replacePlanLedgerWithRoot( } return; } + options?.preCommitCheck?.(); replaceSqliteLedger(directory, { canonicalEvents: [line], state: stateForEvents(directory, [root], priorMode), @@ -1942,6 +1965,7 @@ export async function replacePlanLedgerWithRoot( mode: priorMode, version: packageJson.version, }); + options?.preCommitCheck?.(); try { writePortableLedger(directory, [line]); } catch (error) { @@ -1959,6 +1983,8 @@ export async function replacePlanLedgerWithRoot( interface ReplayOptions { /** If true, use the latest snapshot to speed up replay */ useSnapshot?: boolean; + /** Optional authority fence for recovery-side durable publications. */ + preCommitCheck?: () => void; } /** @@ -2069,15 +2095,19 @@ export async function peekPlanFromLedger( * `replayWithIntegrity`'s bug — it hid genuine replay failures). * * @param directory - The working directory - * @param _options - Optional replay options (reserved) + * @param _options - Optional replay options, including an authority fence for + * recovery-side quarantine publications * @returns {@link ReplayStatusResult} with plan, truncated flag, and bad suffix */ export async function replayFromLedgerWithStatus( directory: string, _options?: ReplayOptions, ): Promise { - const { events, truncated, badSuffix } = - await readLedgerEventsWithIntegrity(directory); + const integrity = await readLedgerEventsWithIntegrity(directory); + // Integrity reading is asynchronous from the caller's perspective. Recheck + // authority before quarantining any suffix discovered by that read. + _options?.preCommitCheck?.(); + const { events, truncated, badSuffix } = integrity; // If no events, nothing to replay if (events.length === 0) { @@ -2089,7 +2119,9 @@ export async function replayFromLedgerWithStatus( // `truncated` flag is threaded back so the caller can refuse to overwrite // plan.json with the prefix-only projection. if (truncated && badSuffix !== null) { - await quarantineLedgerSuffix(directory, badSuffix); + await quarantineLedgerSuffix(directory, badSuffix, { + preCommitCheck: _options?.preCommitCheck, + }); } const plan = reconstructPlanFromEvents(directory, events); @@ -2479,12 +2511,15 @@ export interface QuarantineResult { * * @param directory - The working directory * @param badSuffix - The corrupted content to quarantine + * @param options - Optional authority fence checked immediately before writing + * the quarantine side file * @returns {@link QuarantineResult} with the written path (or null) and the * number of parseable lines salvaged from the suffix */ export async function quarantineLedgerSuffix( directory: string, badSuffix: string, + options?: { preCommitCheck?: () => void }, ): Promise { // Salvage: count individually-parseable lines in the suffix so the size of // the sacrificed tail is observable rather than silently discarded. @@ -2499,6 +2534,7 @@ export async function quarantineLedgerSuffix( } } + let quarantinePath: string; try { assertProjectRoot(directory); // Unique, non-overwriting side path: timestamp for ordering + content hash @@ -2537,10 +2573,20 @@ export async function quarantineLedgerSuffix( // attempt a fresh write below. } - const quarantinePath = path.join( + quarantinePath = path.join( swarmDir, `plan-ledger.quarantine.${Date.now()}.${hash}`, ); + } catch { + // Silently fail if the quarantine path cannot be prepared. + return { path: null, salvagedCount }; + } + + // Keep the authority fence outside the broad preparation/write catch. A + // caller's typed supersession error must propagate rather than being + // misreported as an ordinary quarantine I/O failure. + options?.preCommitCheck?.(); + try { fs.writeFileSync(quarantinePath, badSuffix, 'utf8'); log( `[ledger] Corrupted suffix quarantined to ${path.relative(directory, quarantinePath)} (salvageable events: ${salvagedCount})`, diff --git a/src/plan/manager.ts b/src/plan/manager.ts index 3c154780b..b134ceb56 100644 --- a/src/plan/manager.ts +++ b/src/plan/manager.ts @@ -20,6 +20,19 @@ export class PlanConcurrentModificationError extends Error { } } +/** + * Internal control-flow error used to stop every recovery rung when the + * coordinator's captured hydration generation is no longer authoritative. + * Broad availability catches must rethrow this instead of trying a fallback + * that could publish stale durable state. + */ +export class PlanRecoverySupersededError extends Error { + constructor(message = 'plan recovery superseded by a newer generation') { + super(message); + this.name = 'PlanRecoverySupersededError'; + } +} + /** * Thrown when savePlan detects that the incoming plan would silently drop one * or more tasks from the prior plan without the caller acknowledging the @@ -175,6 +188,7 @@ export const _internals: { readPlanJsonUtf8: typeof readPlanJsonUtf8; readPlanFileUtf8: typeof readPlanFileUtf8; verifyWrittenPlanJson: typeof verifyWrittenPlanJson; + writeRebuildPlanMarkdown: typeof writeRebuildPlanMarkdown; ledgerExists: typeof ledgerExists; replayFromLedger: typeof replayFromLedger; loadLastApprovedPlan: typeof loadLastApprovedPlan; @@ -191,6 +205,7 @@ export const _internals: { readPlanJsonUtf8, readPlanFileUtf8, verifyWrittenPlanJson, + writeRebuildPlanMarkdown, ledgerExists, replayFromLedger, loadLastApprovedPlan, @@ -331,6 +346,7 @@ export async function retryCasWithBackoff( planHashAfter?: string; verifyValid?: () => Promise | boolean; maxRetries?: number; + preCommitCheck?: () => void; }, ): Promise { const maxRetries = options.maxRetries ?? CAS_MAX_RETRIES; @@ -342,6 +358,7 @@ export async function retryCasWithBackoff( return await appendLedgerEvent(directory, eventInput, { expectedHash: currentExpected, planHashAfter: options.planHashAfter, + preCommitCheck: options.preCommitCheck, }); } catch (error) { if (!(error instanceof LedgerStaleWriterError) || attempt >= maxRetries) { @@ -453,6 +470,7 @@ async function getLatestLedgerHash(directory: string): Promise { async function surfaceLedgerStaleIfPersisted( directory: string, plan: RuntimePlan, + options?: { preCommitCheck?: () => void }, ): Promise { const resolvedWorkspace = canonicalRootKeyFresh(directory); if (!ledgerStaleWorkspaces.has(resolvedWorkspace)) { @@ -464,10 +482,12 @@ async function surfaceLedgerStaleIfPersisted( const ledgerHash = await getLatestLedgerHash(directory); if (ledgerHash !== '' && planHash === ledgerHash) { // Reconverged → the workspace recovered. Auto-clear and return clean. + options?.preCommitCheck?.(); ledgerStaleWorkspaces.delete(resolvedWorkspace); return plan; } - } catch { + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; // If the recheck itself fails (e.g. transient ledger read error), fall // through and surface staleness conservatively. Better a visible refusal // the architect can clear than a silent stale-read of plan.json. @@ -649,6 +669,7 @@ export async function isPlanMdInSync( export async function regeneratePlanMarkdown( directory: string, plan: Plan, + options?: { preCommitCheck?: () => void }, ): Promise { assertProjectRoot(directory); const swarmDir = path.resolve(directory, '.swarm'); @@ -663,6 +684,10 @@ export async function regeneratePlanMarkdown( ); try { await bunWrite(mdTempPath, markdownWithHash); + // The rename is the publication boundary. Keep the guard immediately + // adjacent to the atomic operation so a caller that lost authority while + // the temp file was being written cannot publish stale derived state. + options?.preCommitCheck?.(); renameSync(mdTempPath, mdPath); } finally { try { @@ -690,11 +715,19 @@ export async function regeneratePlanMarkdown( export async function loadPlan( directory: string, cache?: Map>, + options?: { preCommitCheck?: () => void }, ): Promise { + // A startup coordinator may be superseded while one of the recovery reads + // below is pending. Fail before any plan recovery/persistence boundary. + options?.preCommitCheck?.(); // Step 1: Try to load and validate plan.json. Decode bytes strictly so a // malformed UTF-8 sequence cannot be silently converted to U+FFFD. A literal // U+FFFD encoded in valid UTF-8 remains ordinary plan data. let planJsonContent: string | null = null; + // When this invocation wins the one-shot startup ledger check, release that + // claim if its authority predicate later rejects a commit. Otherwise a stale + // coordinator could suppress the current generation's required recovery. + let claimedStartupWorkspace: string | null = null; try { planJsonContent = await _internals.readPlanJsonUtf8(directory); } catch (error) { @@ -717,8 +750,12 @@ export async function loadPlan( const inSync = await isPlanMdInSync(directory, validated, cache); if (!inSync) { try { - await _internals.regeneratePlanMarkdown(directory, validated); + await _internals.regeneratePlanMarkdown(directory, validated, { + preCommitCheck: options?.preCommitCheck, + }); } catch (regenError) { + if (regenError instanceof PlanRecoverySupersededError) + throw regenError; // Log warning but don't fail - plan.json is valid warn( `Failed to regenerate plan.md: ${regenError instanceof Error ? regenError.message : String(regenError)}. Proceeding with plan.json only.`, @@ -737,7 +774,9 @@ export async function loadPlan( const ledgerHash = await getLatestLedgerHash(directory); const resolvedWorkspace = canonicalRootKeyFresh(directory); if (!startupLedgerCheckedWorkspaces.has(resolvedWorkspace)) { + options?.preCommitCheck?.(); startupLedgerCheckedWorkspaces.add(resolvedWorkspace); + claimedStartupWorkspace = resolvedWorkspace; if (ledgerHash !== '' && planHash !== ledgerHash) { const currentPlanId = derivePlanId(validated); const ledgerEvents = await readLedgerEvents(directory); @@ -757,7 +796,9 @@ export async function loadPlan( ); try { const { plan: rebuilt, truncated } = - await replayFromLedgerWithStatus(directory); + await replayFromLedgerWithStatus(directory, { + preCommitCheck: options?.preCommitCheck, + }); if (truncated) { // M1 silent-rollback fix: the ledger contained a poison // line, so integrity-checked replay could only reconstruct @@ -782,6 +823,7 @@ export async function loadPlan( // Persist the verdict so it re-surfaces on every later // loadPlan (the startup replay runs at most once per // workspace per process) via the return chokepoint below. + options?.preCommitCheck?.(); ledgerStaleWorkspaces.add(resolvedWorkspace); criticalWarn( '[loadPlan] Ledger truncated (poison line detected) — preserving plan.json instead of rolling back to the prefix-only ledger projection. Durable post-poison events remain in plan.json. Corrupted suffix quarantined to .swarm/plan-ledger.quarantine.*. Run /swarm reset-session after verifying state if this persists.', @@ -791,6 +833,7 @@ export async function loadPlan( if (rebuilt) { await rebuildPlan(directory, rebuilt, { reason: 'ledger_hash_mismatch_recovery', + preCommitCheck: options?.preCommitCheck, }); warn( '[loadPlan] Rebuilt plan from ledger. Checkpoint available at .swarm/plan-export/SWARM_PLAN.md if it exists.', @@ -798,6 +841,8 @@ export async function loadPlan( return rebuilt; } } catch (replayError) { + if (replayError instanceof PlanRecoverySupersededError) + throw replayError; // Ledger replay failed — try the critic-approved immutable // snapshot as a last-resort fallback before returning stale state. // @@ -812,6 +857,7 @@ export async function loadPlan( if (approved) { await rebuildPlan(directory, approved.plan, { reason: 'approved_snapshot_fallback', + preCommitCheck: options?.preCommitCheck, }); // Heal the ledger tail so subsequent loadPlan calls don't // loop back into this recovery path. The recovered plan is @@ -823,8 +869,11 @@ export async function loadPlan( await takeSnapshotEvent(directory, approved.plan, { source: 'recovery_from_approved_snapshot', approvalMetadata: approved.approval, + preCommitCheck: options?.preCommitCheck, }); } catch (healError) { + if (healError instanceof PlanRecoverySupersededError) + throw healError; warn( `[loadPlan] Recovery-heal snapshot append failed: ${healError instanceof Error ? healError.message : String(healError)}. Next loadPlan may re-enter recovery path.`, ); @@ -840,7 +889,9 @@ export async function loadPlan( ); return approved.plan; } - } catch { + } catch (recoveryError) { + if (recoveryError instanceof PlanRecoverySupersededError) + throw recoveryError; // Fall through to the stale-plan warning below } // #1269 finding 2: we are about to return the STALE @@ -863,6 +914,7 @@ export async function loadPlan( // replay above runs at most once per workspace per process). // The chokepoint near `return validated` re-attaches the // flag and self-heals when plan↔ledger reconverge. + options?.preCommitCheck?.(); ledgerStaleWorkspaces.add(resolvedWorkspace); } warn( @@ -923,7 +975,7 @@ export async function loadPlan( '.swarm', 'spec-staleness.json', ); - await fsPromises.writeFile( + await commitAsyncPreparedFile( specStalenessPath, JSON.stringify( { @@ -940,11 +992,13 @@ export async function loadPlan( null, 2, ), - 'utf-8', + options?.preCommitCheck, + 'spec-staleness', ); // #1619 F1 — see the rationale above the enclosing try. invalidateCachedArtifact(specStalenessPath); - } catch { + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; // Non-fatal: spec-staleness.json write failure does not block plan loading } @@ -960,8 +1014,10 @@ export async function loadPlan( reason: staleResult.reason ?? 'unknown', planTitle: validated.title, }; + options?.preCommitCheck?.(); appendCoreEventSync(directory, { ...event }); - } catch { + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; // Non-fatal: event write failure does not block plan loading } } @@ -975,9 +1031,16 @@ export async function loadPlan( return await surfaceLedgerStaleIfPersisted( directory, validated as RuntimePlan, + options, ); } } catch (error) { + if (error instanceof PlanRecoverySupersededError) { + if (claimedStartupWorkspace !== null) { + startupLedgerCheckedWorkspaces.delete(claimedStartupWorkspace); + } + throw error; + } // Step 2: Validation failed, log warning and fall through to legacy warn( `[loadPlan] plan.json validation failed: ${error instanceof Error ? error.message : String(error)}. Attempting rebuild from ledger. If rebuild fails, check .swarm/plan-export/SWARM_PLAN.md for a checkpoint.`, @@ -988,6 +1051,7 @@ export async function loadPlan( // skip the replay to prevent a post-migration ledger from overwriting the // (schema-invalid) migrated plan.json. let rawPlanId: string | null = null; + let rawPlanJsonParseFailed = false; try { const rawParsed = JSON.parse(planJsonContent); if ( @@ -999,7 +1063,10 @@ export async function loadPlan( ); } } catch { - // JSON itself is malformed — rawPlanId stays null (conservative: skip ledger) + // A syntactically malformed projection has no identity to compare. + // A verified, complete ledger still supplies the authority; remember + // this distinct case so parseable foreign projections remain fenced. + rawPlanJsonParseFailed = true; } // Try replay from ledger before legacy migration. The // recovery rungs route through _internals (#2531) so a @@ -1025,22 +1092,30 @@ export async function loadPlan( const catchFirstEvent = ledgerEventsForCatch.length > 0 ? ledgerEventsForCatch[0] : null; const identityMatch = - rawPlanId === null || // Can't determine identity — skip rebuild (conservative) + rawPlanId === null || // No comparable identity; replay eligibility is gated below catchFirstEvent === null || // Empty verified prefix — no identity to compare catchFirstEvent.plan_id === rawPlanId; // Same identity — safe to rebuild if (!identityMatch) { warn( `[loadPlan] Ledger identity mismatch in validation-failure path (ledger: ${catchFirstEvent?.plan_id}, plan: ${rawPlanId}) — skipping ledger rebuild (migration detected).`, ); - } else if (catchFirstEvent !== null && rawPlanId !== null) { + } else if ( + catchFirstEvent !== null && + (rawPlanId !== null || + (rawPlanJsonParseFailed && !catchIntegrity.truncated)) + ) { // Identities match — attempt ledger rebuild. A replay error // must not escape loadPlan (#2531): it falls through to the // approved-snapshot rung below, mirroring the // missing-projection path's ladder. let rebuilt: Plan | null = null; try { - rebuilt = await _internals.replayFromLedger(directory); + rebuilt = await _internals.replayFromLedger(directory, { + preCommitCheck: options?.preCommitCheck, + }); } catch (replayError) { + if (replayError instanceof PlanRecoverySupersededError) + throw replayError; warn( `[loadPlan] Ledger replay threw in validation-failure path: ${replayError instanceof Error ? replayError.message : String(replayError)}. Falling back to critic-approved snapshot before legacy migration.`, ); @@ -1048,6 +1123,7 @@ export async function loadPlan( if (rebuilt) { await rebuildPlan(directory, rebuilt, { reason: 'validation_failure_recovery', + preCommitCheck: options?.preCommitCheck, }); warn( '[loadPlan] Rebuilt plan from ledger after validation failure. Projection was stale.', @@ -1071,6 +1147,7 @@ export async function loadPlan( approved.plan, 'load_plan_recovery_from_approved_snapshot', 'restore from critic-approved snapshot', + { preCommitCheck: options?.preCommitCheck }, ); if (removedCount > 0) { (approved.plan as RuntimePlan)._midLoadRemovals = { @@ -1085,8 +1162,11 @@ export async function loadPlan( await takeSnapshotEvent(directory, approved.plan, { source: 'recovery_from_approved_snapshot', approvalMetadata: approved.approval, + preCommitCheck: options?.preCommitCheck, }); } catch (healError) { + if (healError instanceof PlanRecoverySupersededError) + throw healError; warn( `[loadPlan] Recovery-heal snapshot append failed: ${healError instanceof Error ? healError.message : String(healError)}. Next loadPlan may re-enter recovery path.`, ); @@ -1097,6 +1177,8 @@ export async function loadPlan( return approved.plan; } } catch (approvedError) { + if (approvedError instanceof PlanRecoverySupersededError) + throw approvedError; warn( `[loadPlan] Approved-snapshot recovery failed in validation-failure path: ${approvedError instanceof Error ? approvedError.message : String(approvedError)}`, ); @@ -1119,6 +1201,7 @@ export async function loadPlan( migrated, 'load_plan_migration_from_md', 'migrate legacy plan.md to plan.json', + { preCommitCheck: options?.preCommitCheck }, ); if (removedCount > 0) { (migrated as RuntimePlan)._midLoadRemovals = { @@ -1128,7 +1211,11 @@ export async function loadPlan( } // #2531 (AC4): durable provenance — the ledger must record // that this plan came from a lossy markdown migration. - await appendMigrationProvenanceEvent(directory, migrated); + await appendMigrationProvenanceEvent( + directory, + migrated, + options?.preCommitCheck, + ); return migrated; } // If plan.md doesn't exist either, fall through to step 3 @@ -1162,8 +1249,12 @@ export async function loadPlan( // the critic-approved-snapshot rung below, then markdown. let rebuilt: Plan | null = null; try { - rebuilt = await _internals.replayFromLedger(directory); + rebuilt = await _internals.replayFromLedger(directory, { + preCommitCheck: options?.preCommitCheck, + }); } catch (replayError) { + if (replayError instanceof PlanRecoverySupersededError) + throw replayError; warn( `[loadPlan] Ledger replay threw in missing-projection path: ${replayError instanceof Error ? replayError.message : String(replayError)}. Falling back to critic-approved snapshot before legacy migration.`, ); @@ -1174,6 +1265,7 @@ export async function loadPlan( rebuilt, 'load_plan_rebuild_from_ledger', 'rebuild plan from ledger replay', + { preCommitCheck: options?.preCommitCheck }, ); if (removedCount > 0) { (rebuilt as RuntimePlan)._midLoadRemovals = { @@ -1241,6 +1333,7 @@ export async function loadPlan( approved.plan, 'load_plan_recovery_from_approved_snapshot', 'restore from critic-approved snapshot', + { preCommitCheck: options?.preCommitCheck }, ); if (snapshotRemovedCount > 0) { (approved.plan as RuntimePlan)._midLoadRemovals = { @@ -1257,8 +1350,11 @@ export async function loadPlan( await takeSnapshotEvent(directory, approved.plan, { source: 'recovery_from_approved_snapshot', approvalMetadata: approved.approval, + preCommitCheck: options?.preCommitCheck, }); } catch (healError) { + if (healError instanceof PlanRecoverySupersededError) + throw healError; warn( `[loadPlan] Recovery-heal snapshot append failed: ${healError instanceof Error ? healError.message : String(healError)}. Next loadPlan may re-enter recovery path.`, ); @@ -1266,6 +1362,8 @@ export async function loadPlan( return approved.plan; } } catch (recoveryError) { + if (recoveryError instanceof PlanRecoverySupersededError) + throw recoveryError; warn( `[loadPlan] Approved-snapshot recovery failed: ${recoveryError instanceof Error ? recoveryError.message : String(recoveryError)}`, ); @@ -1289,6 +1387,7 @@ export async function loadPlan( migrated, 'load_plan_migration_from_md', 'migrate legacy plan.md to plan.json', + { preCommitCheck: options?.preCommitCheck }, ); if (removedCount > 0) { (migrated as RuntimePlan)._midLoadRemovals = { @@ -1296,7 +1395,11 @@ export async function loadPlan( source: 'load_plan_migration_from_md', }; } - await appendMigrationProvenanceEvent(directory, migrated); + await appendMigrationProvenanceEvent( + directory, + migrated, + options?.preCommitCheck, + ); return migrated; } @@ -1325,6 +1428,7 @@ export async function savePlanWithAutoAcknowledgedRemovals( options?: { preserveCompletedStatuses?: boolean; planLockAlreadyHeld?: boolean; + preCommitCheck?: () => void; }, ): Promise<{ removedCount: number }> { const existing = await _internals.loadPlanJsonOnly(directory); @@ -1502,22 +1606,29 @@ async function verifyWrittenPlanJson( async function appendMigrationProvenanceEvent( directory: string, plan: Plan, + preCommitCheck?: () => void, ): Promise { try { - await appendLedgerEvent(directory, { - event_type: 'plan_rebuilt', - source: 'load_plan_migration_from_md', - plan_id: derivePlanId(plan), - payload: { - reason: 'load_plan_migration_from_md', - phases_count: plan.phases.length, - tasks_count: plan.phases.reduce( - (sum, phase) => sum + phase.tasks.length, - 0, - ), + preCommitCheck?.(); + await appendLedgerEvent( + directory, + { + event_type: 'plan_rebuilt', + source: 'load_plan_migration_from_md', + plan_id: derivePlanId(plan), + payload: { + reason: 'load_plan_migration_from_md', + phases_count: plan.phases.length, + tasks_count: plan.phases.reduce( + (sum, phase) => sum + phase.tasks.length, + 0, + ), + }, }, - }); + { preCommitCheck }, + ); } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; warn( `[loadPlan] Markdown-migration provenance event append failed (plan remains migrated): ${error instanceof Error ? error.message : String(error)}`, ); @@ -1665,7 +1776,9 @@ export async function savePlan( if (!(await ledgerExists(directory))) { try { options?.preCommitCheck?.(); - await initLedger(directory, planId, planHashForInit, validated); + await initLedger(directory, planId, planHashForInit, validated, { + preCommitCheck: options?.preCommitCheck, + }); } catch (initErr) { // Concurrent savePlan race: three parallel callers can pass the // ledgerExists() check before any of them writes. On Linux/macOS @@ -1690,6 +1803,7 @@ export async function savePlan( directory, validated, 'savePlan_identity_migration', + { preCommitCheck: options?.preCommitCheck }, ); warn( `[savePlan] Ledger identity mismatch (was "${existingEvents[0].plan_id}", now "${planId}") — archived the prior exact history and committed a new root.`, @@ -1846,6 +1960,7 @@ export async function savePlan( await retryCasWithBackoff(directory, eventInput, { expectedHash: currentHash, planHashAfter: hashAfter, + preCommitCheck: options?.preCommitCheck, verifyValid: async () => { const onDisk = await _internals.loadPlanJsonOnly(directory); if (!onDisk) return true; @@ -1895,6 +2010,7 @@ export async function savePlan( await retryCasWithBackoff(directory, eventInput, { expectedHash: currentHash, planHashAfter: hashAfter, + preCommitCheck: options?.preCommitCheck, verifyValid: async () => { // If another writer already persisted the transition, skip. const onDisk = await _internals.loadPlanJsonOnly(directory); @@ -1927,7 +2043,12 @@ export async function savePlan( const ledgerStatusTaskIds = collectLedgerStatusTaskIds( await readLedgerEvents(directory), ); - const replayedBeforeProjection = await _internals.replayFromLedger(directory); + const replayedBeforeProjection = await _internals.replayFromLedger( + directory, + { + preCommitCheck: options?.preCommitCheck, + }, + ); const projectionCandidate = replayedBeforeProjection ? mergeStatusesTakingPrecedence( validated, @@ -1944,6 +2065,7 @@ export async function savePlan( await takeSnapshotEvent(directory, projectionCandidate, { planHashAfter: computePlanLedgerHash(projectionCandidate), source: 'savePlan_structural_projection', + preCommitCheck: options?.preCommitCheck, }); } @@ -2022,8 +2144,14 @@ export async function savePlan( ), in_progress: true, }); - await bunWrite(markerPath, inProgressMarker); - } catch { + await commitAsyncPreparedFile( + markerPath, + inProgressMarker, + options?.preCommitCheck, + 'plan-write-marker', + ); + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; /* Advisory only */ } @@ -2043,6 +2171,7 @@ export async function savePlan( ); try { await bunWrite(mdTempPath, markdownWithHash); + options?.preCommitCheck?.(); renameSync(mdTempPath, mdPath); } finally { try { @@ -2053,6 +2182,7 @@ export async function savePlan( } invalidateCachedArtifact(mdPath); } catch (mdError) { + if (mdError instanceof PlanRecoverySupersededError) throw mdError; const message = mdError instanceof Error ? mdError.message : String(mdError); mdWriteError = message; @@ -2086,8 +2216,14 @@ export async function savePlan( tasks_count: tasksCount, in_progress: false, }); - await bunWrite(markerPath, marker); - } catch { + await commitAsyncPreparedFile( + markerPath, + marker, + options?.preCommitCheck, + 'plan-write-marker', + ); + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; /* Advisory only - marker write failure does not affect plan save */ } @@ -2115,6 +2251,7 @@ export async function savePlan( for (const [taskId, oldStatus] of oldStatuses) { const newStatus = newStatuses.get(taskId); if (oldStatus === 'completed' && newStatus !== 'completed') { + options?.preCommitCheck?.(); advanceTaskCheckpointReceiptGeneration( directory, oldIdentityHash, @@ -2127,6 +2264,7 @@ export async function savePlan( newStatus === 'completed' && oldStatuses.get(taskId) !== 'completed' ) { + options?.preCommitCheck?.(); repairTaskCheckpointReceiptForCompletion( directory, newIdentityHash, @@ -2136,6 +2274,8 @@ export async function savePlan( } } } catch (receiptError) { + if (receiptError instanceof PlanRecoverySupersededError) + throw receiptError; warn( `[savePlan] task checkpoint receipt lifecycle sync failed (plan remains authoritative): ${receiptError instanceof Error ? receiptError.message : String(receiptError)}`, ); @@ -2154,6 +2294,36 @@ export async function savePlan( return { durability: 'complete', degraded_surfaces: [] }; } +async function commitAsyncPreparedFile( + targetPath: string, + content: string, + preCommitCheck?: () => void, + tempLabel = 'atomic', +): Promise { + const tempPath = `${targetPath}.${tempLabel}.${Date.now()}.${Math.floor(Math.random() * 1e9)}`; + try { + await bunWrite(tempPath, content); + // Preparation writes only an unreferenced temp file. Check authority after + // that await, immediately before the synchronous canonical rename. + preCommitCheck?.(); + renameSync(tempPath, targetPath); + } catch (error) { + try { + unlinkSync(tempPath); + } catch { + /* Best-effort temp cleanup; preserve the original error. */ + } + throw error; + } +} + +async function writeRebuildPlanMarkdown( + tempPath: string, + content: string, +): Promise { + await bunWrite(tempPath, content); +} + /** * Rebuild plan from ledger events. * Replays the ledger to reconstruct plan state, then writes the result. @@ -2165,11 +2335,16 @@ export async function savePlan( export async function rebuildPlan( directory: string, plan?: Plan, - options?: { reason?: string }, + options?: { reason?: string; preCommitCheck?: () => void }, ): Promise { assertProjectRoot(directory); - const targetPlan = plan ?? (await replayFromLedger(directory)); + const targetPlan = + plan ?? + (await replayFromLedger(directory, { + preCommitCheck: options?.preCommitCheck, + })); if (!targetPlan) return null; + options?.preCommitCheck?.(); // Write directly without going through savePlan (avoid circular ledger append) const swarmDir = path.join(directory, '.swarm'); @@ -2193,17 +2368,29 @@ export async function rebuildPlan( swarmDir, `plan.json.rebuild.${Date.now()}.${Math.floor(Math.random() * 1e9)}`, ); - { - const fd = openSync(tempPlanPath, 'w'); + try { + { + const fd = openSync(tempPlanPath, 'w'); + try { + writeFileSync(fd, JSON.stringify(targetPlan, null, 2), 'utf8'); + fsyncSync(fd); + } finally { + closeSync(fd); + } + } + // Keep this synchronous guard adjacent to the atomic rename. Unlike an + // async post-check, it prevents a superseded coordinator from swapping the + // canonical projection after recovery work has completed. + options?.preCommitCheck?.(); + renameSync(tempPlanPath, planPath); + invalidateCachedArtifact(planPath); + } finally { try { - writeFileSync(fd, JSON.stringify(targetPlan, null, 2), 'utf8'); - fsyncSync(fd); - } finally { - closeSync(fd); + unlinkSync(tempPlanPath); + } catch { + /* already renamed or never created */ } } - renameSync(tempPlanPath, planPath); - invalidateCachedArtifact(planPath); // Write in-progress marker right after plan.json rename. try { @@ -2218,14 +2405,23 @@ export async function rebuildPlan( ), in_progress: true, }); - await bunWrite(markerPath, inProgressMarker); - } catch { + await commitAsyncPreparedFile( + markerPath, + inProgressMarker, + options?.preCommitCheck, + 'rebuild', + ); + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; /* Advisory only */ } // Also regenerate plan.md with content hash (matches the format written by savePlan/ // regeneratePlanMarkdown so that isPlanMdInSync() can detect the hash and avoid // unnecessary re-generation on the next loadPlan() call). + let markdownWriteFailed = false; + let markdownWriteError: unknown; + let markerSupersededError: PlanRecoverySupersededError | undefined; try { const contentHash = computePlanContentHash(targetPlan); const markdown = derivePlanMarkdown(targetPlan); @@ -2234,12 +2430,26 @@ export async function rebuildPlan( swarmDir, `plan.md.rebuild.${Date.now()}.${Math.floor(Math.random() * 1e9)}`, ); - await bunWrite(tempMdPath, markdownWithHash); - renameSync(tempMdPath, mdPath); - invalidateCachedArtifact(mdPath); + try { + await _internals.writeRebuildPlanMarkdown(tempMdPath, markdownWithHash); + options?.preCommitCheck?.(); + renameSync(tempMdPath, mdPath); + invalidateCachedArtifact(mdPath); + } finally { + try { + unlinkSync(tempMdPath); + } catch { + /* already renamed or never created */ + } + } + } catch (error) { + markdownWriteFailed = true; + markdownWriteError = error; } finally { - // Always reset the marker to in_progress: false, even if plan.md write failed, - // so PlanSyncWorker's unauthorized-write checks are not permanently disabled. + // Reset the marker to in_progress: false after the markdown attempt so + // PlanSyncWorker's unauthorized-write checks are not permanently disabled. + // A superseded recovery skips this advisory cleanup to preserve a newer + // writer's marker. try { const markerPath = path.join(swarmDir, '.plan-write-marker'); const tasksCount = targetPlan.phases.reduce( @@ -2253,17 +2463,44 @@ export async function rebuildPlan( tasks_count: tasksCount, in_progress: false, }); - await bunWrite(markerPath, marker); - } catch { + // Do not let a superseded recovery clear a marker published by a + // newer writer. This check is deliberately adjacent to the marker + // commit and preserves typed supersession through the cleanup path. + await commitAsyncPreparedFile( + markerPath, + marker, + options?.preCommitCheck, + 'rebuild', + ); + } catch (error) { + if (error instanceof PlanRecoverySupersededError) { + markerSupersededError = error; + } /* Advisory only */ } } + if (markerSupersededError) throw markerSupersededError; + if (markdownWriteFailed) { + if (markdownWriteError instanceof PlanRecoverySupersededError) + throw markdownWriteError; + const message = + markdownWriteError instanceof Error + ? markdownWriteError.message + : String(markdownWriteError); + warn( + `[rebuildPlan] plan.md projection write failed (non-fatal; plan.json is authoritative): ${message}`.slice( + 0, + 512, + ), + ); + } // Append plan_rebuilt ledger event for audit trail (FR-003). // This is NOT circular — rebuildPlan replays existing events to reconstruct state; // appending a metadata event that records "rebuild occurred" does not create a loop // because applyEventToPlan treats plan_rebuilt as an idempotent no-op. try { + options?.preCommitCheck?.(); const planId = derivePlanId(targetPlan); const planHashAfter = computePlanLedgerHash(targetPlan); await appendLedgerEvent( @@ -2281,9 +2518,10 @@ export async function rebuildPlan( ), }, }, - { planHashAfter }, + { planHashAfter, preCommitCheck: options?.preCommitCheck }, ); - } catch { + } catch (error) { + if (error instanceof PlanRecoverySupersededError) throw error; // Non-fatal — audit trail gap is acceptable if ledger is unavailable } diff --git a/src/session/hydration-ownership.ts b/src/session/hydration-ownership.ts index db06e4f5a..5e578911a 100644 --- a/src/session/hydration-ownership.ts +++ b/src/session/hydration-ownership.ts @@ -1,13 +1,15 @@ /** - * Project-owned, generation-fenced hydration state (issue #2667). + * Project-owned, authority-fenced hydration state (issues #2667/#2668). * * The live maps on `swarmState` are process-resident and keyed by sessionID; * hydration is a per-PROJECT operation. This module owns the per-project * registries that scope a hydration's blast radius to the project that * initiated it and fence stale generations from publishing over newer state: * - * - `projectHydrationGenerations` — monotonic counter per canonical project - * key, bumped ONLY when a hydration is initiated (`beginHydrationScope`). + * - `projectHydrationAuthorities` — bounded records containing a per-project + * generation and a process-monotonic authority epoch. The generation is + * bumped ONLY when a hydration is initiated (`beginHydrationScope`), while + * the epoch changes whenever an evicted/reset project is reintroduced. * Session creation NEVER bumps it; new sessions stamp `current + 1` so a * session created while generation `g` is latest survives any later apply * at `g`. @@ -16,10 +18,12 @@ * - `hydratedAggregateKeys` — the toolAggregates keys each project's last * hydration published, so a re-hydration replaces only its own keys. * - * Fence rule: a hydration whose scope generation `g` satisfies - * `projectHydrationGenerations[K] > g` is REJECTED before any mutation — - * some newer hydration began for that project. Stamp rule: an accepted - * hydration at `g` evicts only sessions owned by `K` with + * Fence rule: a hydration is current only while both its generation and its + * authority epoch match the bounded record for its project. This prevents an + * evicted project entry from being reintroduced with a numerically reused + * generation and accidentally reviving an old scope (ABA). Session stamp + * rule: an accepted hydration at `{ generation: g, authorityEpoch: e }` + * evicts owned sessions from an older epoch, or sessions in epoch `e` with * `hydrationStamp <= g`. * * All maps are bounded (FIFO) with an explicit reset path @@ -39,15 +43,42 @@ export const MAX_DIRECTORY_KEY_MEMO = 64; /** Opaque cache payload owned by state.ts; opaque here by design. */ export type ProjectRehydrationCache = unknown; +interface ProjectRehydrationCacheEntry { + cache: ProjectRehydrationCache; + authorityEpoch: number; +} + +interface HydratedAggregateKeysEntry { + keys: Set; + authorityEpoch: number; +} + +/** Opaque process-local authority token for a project's current record. */ +export interface HydrationAuthorityToken { + projectKey: string; + generation: number; + authorityEpoch: number; +} + /** Fence token captured when a hydration is initiated; see beginHydrationScope. */ export interface HydrationScope { projectKey: string; generation: number; + authorityEpoch: number; } -const projectHydrationGenerations = new Map(); -const rehydrationCaches = new Map(); -const hydratedAggregateKeys = new Map>(); +interface HydrationAuthorityRecord { + generation: number; + authorityEpoch: number; +} + +const projectHydrationAuthorities = new Map(); +// Deliberately process-monotonic: clearHydrationOwnershipState() clears the +// bounded records but MUST NOT reset this scalar, or old tokens could become +// valid again after a reset. +let hydrationAuthorityEpoch = 0; +const rehydrationCaches = new Map(); +const hydratedAggregateKeys = new Map(); const directoryKeyMemo = new Map(); function evictOldest(map: Map, cap: number): void { @@ -90,28 +121,85 @@ export function hydrationProjectKey(directory: string): string { } /** - * Begin a hydration scope: bump the project's generation counter and return - * the fence token. The token must be captured when the hydration is + * Begin a hydration scope: bump the project's generation, mint a non-reusable + * authority, and return the fence token. The token must be captured when the hydration is * INITIATED (e.g. at `loadSnapshot` entry) and carried into * `rehydrateState`, which refuses to apply it once any newer generation has * begun. */ export function beginHydrationScope(directory: string): HydrationScope { const projectKey = hydrationProjectKey(directory); - const generation = (projectHydrationGenerations.get(projectKey) ?? 0) + 1; + const current = projectHydrationAuthorities.get(projectKey); + const generation = (current?.generation ?? 0) + 1; // Update-in-place must not evict a DIFFERENT project's entry at the cap: // only make room when this project is not already tracked (otherwise a // bump for a tracked project would reset an unrelated project's counter). - if (!projectHydrationGenerations.has(projectKey)) { - evictOldest(projectHydrationGenerations, MAX_TRACKED_PROJECTS); + if (!current) { + evictOldest(projectHydrationAuthorities, MAX_TRACKED_PROJECTS); } - projectHydrationGenerations.set(projectKey, generation); - return { projectKey, generation }; + // The epoch identifies the project's authority incarnation, not an + // individual generation. A normal rehydration only advances generation; + // minting a new epoch here would invalidate the prior aggregate-key owner + // and cache before the replacement build can publish. An evicted or reset + // project has no current record and therefore receives a fresh epoch. + const authorityEpoch = + current?.authorityEpoch ?? nextHydrationAuthorityEpoch(); + projectHydrationAuthorities.set(projectKey, { generation, authorityEpoch }); + return { projectKey, generation, authorityEpoch }; } /** Latest initiated hydration generation for a project (0 when never). */ export function currentHydrationGeneration(projectKey: string): number { - return projectHydrationGenerations.get(projectKey) ?? 0; + return projectHydrationAuthorities.get(projectKey)?.generation ?? 0; +} + +/** + * Capture the current project authority without beginning a hydration. This + * is used by direct session rehydration, which needs an exact record to fence + * across awaits but must preserve the session-creation/no-generation-bump + * contract. + */ +export function captureCurrentHydrationAuthority( + projectKey: string, +): HydrationAuthorityToken { + const current = projectHydrationAuthorities.get(projectKey); + if (current) { + return { projectKey, ...current }; + } + const authorityEpoch = nextHydrationAuthorityEpoch(); + evictOldest(projectHydrationAuthorities, MAX_TRACKED_PROJECTS); + const record = { generation: 0, authorityEpoch }; + projectHydrationAuthorities.set(projectKey, record); + return { projectKey, ...record }; +} + +function nextHydrationAuthorityEpoch(): number { + if (hydrationAuthorityEpoch >= Number.MAX_SAFE_INTEGER) { + throw new Error('hydration authority epoch exhausted'); + } + hydrationAuthorityEpoch += 1; + return hydrationAuthorityEpoch; +} + +/** Return whether an exact authority token still names the current record. */ +export function isHydrationAuthorityCurrent( + authority: HydrationAuthorityToken, +): boolean { + const current = projectHydrationAuthorities.get(authority.projectKey); + return ( + current?.generation === authority.generation && + current.authorityEpoch === authority.authorityEpoch + ); +} + +/** + * Return whether a captured hydration scope is still the latest initiated + * scope for its project. Exact generation+epoch equality is intentional: a + * scope whose project registry entry was evicted or reset is no longer current + * either, even if a numerically equal generation is visible at the check. + */ +export function isHydrationScopeCurrent(scope: HydrationScope): boolean { + return isHydrationAuthorityCurrent(scope); } /** @@ -126,38 +214,90 @@ export function nextSessionHydrationStamp(projectKey: string): number { export function getRehydrationCache( projectKey: string, ): ProjectRehydrationCache | undefined { - return rehydrationCaches.get(projectKey); + const entry = rehydrationCaches.get(projectKey); + const authority = projectHydrationAuthorities.get(projectKey); + // A cache is valid only for the exact authority incarnation that published + // it. FIFO eviction removes the authority record but leaves this bounded + // cache entry addressable by key; requiring an exact epoch prevents that old + // entry from becoming visible after the project is reintroduced (ABA). + if ( + !entry || + !authority || + entry.authorityEpoch !== authority.authorityEpoch + ) { + return undefined; + } + return entry.cache; } export function setRehydrationCache( projectKey: string, cache: ProjectRehydrationCache, -): void { + authority?: HydrationAuthorityToken, +): boolean { + // Direct cache rebuilds (for example after compaction) may run before any + // hydration scope exists. Give them the same current authority identity + // that startAgentSession will capture, without bumping its generation. A + // rebuild that captured a token at entry must pass that exact token here so + // delayed work cannot be retagged as current after an ABA reintroduction. + const cacheAuthority = + authority ?? captureCurrentHydrationAuthority(projectKey); + if ( + cacheAuthority.projectKey !== projectKey || + !isHydrationAuthorityCurrent(cacheAuthority) + ) { + return false; + } if (!rehydrationCaches.has(projectKey)) { evictOldest(rehydrationCaches, MAX_TRACKED_PROJECTS); } - rehydrationCaches.set(projectKey, cache); + rehydrationCaches.set(projectKey, { + cache, + authorityEpoch: cacheAuthority.authorityEpoch, + }); + return true; } /** Record the aggregate keys a project's hydration published (replace set). */ export function recordHydratedAggregateKeys( projectKey: string, keys: Set, -): void { + authority?: HydrationAuthorityToken, +): boolean { + const aggregateAuthority = + authority ?? captureCurrentHydrationAuthority(projectKey); + if ( + aggregateAuthority.projectKey !== projectKey || + !isHydrationAuthorityCurrent(aggregateAuthority) + ) { + return false; + } if (!hydratedAggregateKeys.has(projectKey)) { evictOldest(hydratedAggregateKeys, MAX_TRACKED_PROJECTS); } - hydratedAggregateKeys.set(projectKey, new Set(keys)); + hydratedAggregateKeys.set(projectKey, { + keys: new Set(keys), + authorityEpoch: aggregateAuthority.authorityEpoch, + }); + return true; } /** The aggregate keys the project's last hydration published (empty if none). */ -export function hydratedAggregateKeysFor(projectKey: string): Set { - return hydratedAggregateKeys.get(projectKey) ?? new Set(); +export function hydratedAggregateKeysFor( + projectKey: string, + authority?: HydrationAuthorityToken, +): Set { + const entry = hydratedAggregateKeys.get(projectKey); + const current = authority ?? projectHydrationAuthorities.get(projectKey); + if (!entry || !current || entry.authorityEpoch !== current.authorityEpoch) { + return new Set(); + } + return entry.keys; } /** Reset every registry (invariant 8: bounded state with an explicit reset). */ export function clearHydrationOwnershipState(): void { - projectHydrationGenerations.clear(); + projectHydrationAuthorities.clear(); rehydrationCaches.clear(); hydratedAggregateKeys.clear(); directoryKeyMemo.clear(); diff --git a/src/session/snapshot-coordination-init.ts b/src/session/snapshot-coordination-init.ts index 5642682cf..9e66cfee8 100644 --- a/src/session/snapshot-coordination-init.ts +++ b/src/session/snapshot-coordination-init.ts @@ -4,12 +4,18 @@ import { createHash, randomUUID } from 'node:crypto'; import { existsSync, readFileSync, renameSync } from 'node:fs'; import { canonicalProjectKey } from '../db/canonical-project.js'; import { validateSwarmPath } from '../hooks/utils.js'; +import { loadPlan, PlanRecoverySupersededError } from '../plan/manager.js'; import { advisoryWarn } from '../services/warning-buffer.js'; -import { applyRehydrationCache, swarmState } from '../state.js'; +import { + applyRehydrationCache, + buildRehydrationCache, + swarmState, +} from '../state.js'; import { withTimeout } from '../utils/timeout.js'; import { beginHydrationScope, type HydrationScope, + isHydrationScopeCurrent, } from './hydration-ownership.js'; import { readSnapshotFileStrict, rehydrateState } from './snapshot-reader.js'; import { importSnapshotRowsOnce, readSnapshotRows } from './snapshot-store.js'; @@ -27,9 +33,13 @@ const ARCHIVE_RETRY_DELAY_MS = 25; type ReadinessState = | 'running' | 'succeeded' + | 'superseded' | 'failed' | 'timed_out' | 'closing'; +export type SnapshotCoordinationInitializationOutcome = + | 'succeeded' + | 'superseded'; interface ReadinessEntry { attemptId: number; generation: number; @@ -39,6 +49,10 @@ interface ReadinessEntry { error?: string; } +function isReadinessEntryClosing(entry: ReadinessEntry | undefined): boolean { + return entry?.state === 'closing'; +} + export interface SnapshotCoordinationStatus { state: ReadinessState | 'idle'; attemptId?: number; @@ -66,11 +80,15 @@ function isRetryableArchiveError(error: unknown): boolean { return code === 'EBUSY' || code === 'EPERM' || code === 'EACCES'; } +type LegacyArchiveOutcome = 'archived' | 'not_archived' | 'superseded'; + async function archiveLegacySnapshotIfPresent( legacyPath: string, expectedSnapshot?: SnapshotData, -): Promise { - if (!existsSync(legacyPath)) return; + shouldCommit: () => boolean = () => true, +): Promise { + if (!shouldCommit()) return 'superseded'; + if (!existsSync(legacyPath)) return 'not_archived'; if (expectedSnapshot) { try { const current = JSON.stringify( @@ -80,11 +98,11 @@ async function archiveLegacySnapshotIfPresent( advisoryWarn( '[opencode-swarm] Legacy snapshot changed after SQLite coordination; preserving it for explicit recovery.', ); - return; + return 'not_archived'; } } catch { // Do not archive an unreadable source when a peer may have replaced it. - return; + return 'not_archived'; } } const canonicalArchive = `${legacyPath}.imported`; @@ -93,12 +111,15 @@ async function archiveLegacySnapshotIfPresent( : canonicalArchive; let lastError: unknown; for (let attempt = 1; attempt <= ARCHIVE_RETRY_ATTEMPTS; attempt += 1) { + // The rename is the archive's publication boundary. Check immediately + // before it so a superseded initializer cannot move a newer legacy file. + if (!shouldCommit()) return 'superseded'; try { _snapshotCoordinationInternals.renameLegacySnapshot( legacyPath, archivePath, ); - return; + return 'archived'; } catch (error) { lastError = error; if (!isRetryableArchiveError(error) || attempt === ARCHIVE_RETRY_ATTEMPTS) @@ -113,6 +134,7 @@ async function archiveLegacySnapshotIfPresent( lastError instanceof Error ? lastError.message : String(lastError) }`, ); + return 'not_archived'; } function evictSettledEntries(): boolean { @@ -127,10 +149,15 @@ function evictSettledEntries(): boolean { async function initializeSnapshotCoordination( directory: string, scope?: HydrationScope, -): Promise { +): Promise { + const isCurrent = () => scope === undefined || isHydrationScopeCurrent(scope); + // Source selection and the first authority read are also publication + // boundaries: a stale initializer must not even start a compatibility import. + if (!isCurrent()) return 'superseded'; let legacyArchiveAttempted = false; let snapshot = readSnapshotRows(directory); if (!snapshot) { + if (!isCurrent()) return 'superseded'; const legacyPath = validateSwarmPath(directory, 'session/state.json'); const projectionPath = validateSwarmPath( directory, @@ -147,11 +174,20 @@ async function initializeSnapshotCoordination( ? 'session/state.json' : null; if (source) { + if (!isCurrent()) return 'superseded'; // Unlike the compatibility reader, import never treats corruption or an // unsupported version as absence. Authority stays fail-closed. - const candidate = await readSnapshotFileStrict(directory, source); + const candidate = + await _snapshotCoordinationInternals.readSnapshotFileStrict( + directory, + source, + ); + // Strict reading is asynchronous. Re-check immediately before the + // synchronous SQLite import so stale compatibility bytes cannot become + // authoritative after a newer hydration starts. + if (!isCurrent()) return 'superseded'; const serialized = JSON.stringify(candidate); - const outcome = importSnapshotRowsOnce( + const outcome = _snapshotCoordinationInternals.importSnapshotRowsOnce( directory, candidate, createHash('sha256').update(serialized).digest('hex'), @@ -160,28 +196,85 @@ async function initializeSnapshotCoordination( snapshot = readSnapshotRows(directory); if (outcome === 'imported' && source === 'session/state.json') { legacyArchiveAttempted = true; - await archiveLegacySnapshotIfPresent(legacyPath, candidate); + const archiveOutcome = await archiveLegacySnapshotIfPresent( + legacyPath, + candidate, + isCurrent, + ); + if (archiveOutcome === 'superseded') return 'superseded'; } } } - if (!snapshot) return; - // A prior attempt may have committed SQLite and crashed before archival. - // Repair that post-commit side effect on every authoritative restart without - // ever overwriting an earlier cold archive. - if (!legacyArchiveAttempted) { - await archiveLegacySnapshotIfPresent( - validateSwarmPath(directory, 'session/state.json'), - snapshot, + if (!isCurrent()) return 'superseded'; + + if (snapshot) { + // A prior attempt may have committed SQLite and crashed before archival. + // Repair that post-commit side effect on every authoritative restart without + // ever overwriting an earlier cold archive. + if (!legacyArchiveAttempted) { + const archiveOutcome = await archiveLegacySnapshotIfPresent( + validateSwarmPath(directory, 'session/state.json'), + snapshot, + isCurrent, + ); + if (archiveOutcome === 'superseded') return 'superseded'; + } + // Issues #2667/#2668: the apply is authority-fenced by the scope captured in + // startSnapshotCoordinationInitialization — a timed-out initializer that + // settles late cannot publish over the state of any newer hydration. + const outcome = await rehydrateState(snapshot, directory, scope); + if (!outcome.applied || !isCurrent()) return 'superseded'; + } + + // The early init reader intentionally uses only cheap projection data. Once + // post-resolution coordination is running, resolve the authoritative plan + // through the ledger-aware manager before publishing the rehydration cache. + let authoritativePlan: Awaited> | undefined; + try { + authoritativePlan = await _snapshotCoordinationInternals.loadPlan( + directory, + undefined, + { + preCommitCheck: () => { + if (!isCurrent()) { + throw new PlanRecoverySupersededError( + 'Snapshot coordination initialization superseded during plan recovery', + ); + } + }, + }, + ); + } catch (error) { + // A superseded recovery no longer owns the plan authority. Preserve the + // typed signal so the coordinator can mark readiness superseded and stop + // before applying the pre-resolution cache or publishing its projection. + if (error instanceof PlanRecoverySupersededError) throw error; + advisoryWarn( + `[opencode-swarm] Authoritative plan recovery failed; retaining pre-resolution cache: ${ + error instanceof Error ? error.message : String(error) + }`.slice(0, 512), ); } - // Issue #2667: the apply is generation-fenced by the scope captured in - // startSnapshotCoordinationInitialization — a timed-out initializer that - // settles late cannot publish over the state of any newer hydration. - await rehydrateState(snapshot, directory, scope); - for (const session of swarmState.agentSessions.values()) + if (!isCurrent()) return 'superseded'; + if (authoritativePlan !== undefined) { + const cacheResult = await buildRehydrationCache(directory, { + planOverride: authoritativePlan, + shouldCommit: isCurrent, + }); + if (!cacheResult.committed || !isCurrent()) return 'superseded'; + } + for (const session of swarmState.agentSessions.values()) { + if (!isCurrent()) return 'superseded'; applyRehydrationCache(session); + } + if (!snapshot || !isCurrent()) + return isCurrent() ? 'succeeded' : 'superseded'; try { - await _snapshotCoordinationInternals.writeProjection(directory, snapshot); + await _snapshotCoordinationInternals.writeProjection( + directory, + snapshot, + isCurrent, + ); } catch (error) { // The projection is a derived compatibility shadow. SQLite is already // authoritative and rehydrated above, so a shadow write failure must not @@ -192,6 +285,7 @@ async function initializeSnapshotCoordination( }`, ); } + return isCurrent() ? 'succeeded' : 'superseded'; } export function startSnapshotCoordinationInitialization( @@ -215,10 +309,10 @@ export function startSnapshotCoordinationInitialization( } const attemptId = nextAttemptId++; const generation = (existing?.generation ?? 0) + 1; - // Issue #2667: fence token captured at INITIATION (each fresh initializer - // bumps the shared per-project counter, which never decreases even when - // this entry is later deleted by retrySnapshotCoordinationInitialization). - const scope = beginHydrationScope(directory); + // Issues #2667/#2668: fence token captured at INITIATION. Each fresh + // initializer mints a process-unique authority that cannot be reused after + // bounded-record eviction or reset. + const scope = beginHydrationScope(root); const entry: ReadinessEntry = { attemptId, generation, @@ -228,13 +322,26 @@ export function startSnapshotCoordinationInitialization( }; const underlying = _snapshotCoordinationInternals .initialize(root, scope) - .then(() => { - if (entries.get(root) === entry && entry.state !== 'closing') - entry.state = 'succeeded'; + .then((outcome) => { + if (entries.get(root) !== entry || entry.state === 'closing') return; + if (outcome === 'superseded') { + entry.state = 'superseded'; + entry.error = + 'coordination initialization superseded by a newer hydration generation'; + return; + } + entry.state = 'succeeded'; }) .catch((error: unknown) => { - entry.state = 'failed'; - entry.error = error instanceof Error ? error.message : String(error); + if (error instanceof PlanRecoverySupersededError) { + if (entry.state !== 'closing' && entries.get(root) === entry) { + entry.state = 'superseded'; + entry.error = error.message; + } + } else { + entry.state = 'failed'; + entry.error = error instanceof Error ? error.message : String(error); + } throw error; }) .finally(() => { @@ -263,18 +370,37 @@ export async function ensureSnapshotCoordinationReady( ): Promise { const root = canonicalProjectKey(directory); const entry = entries.get(root); - if (!entry) return startSnapshotCoordinationInitialization(root); - if (entry.state === 'closing') { + if (entry?.state === 'closing') { throw new Error('coordination initialization is closing for reset-session'); } - if (entry.state === 'timed_out' && !entry.settled) { + if (entry?.state === 'timed_out' && !entry.settled) { throw new Error( 'coordination initialization remains unsettled after timeout', ); } + if (!entry || (entry.state === 'superseded' && entry.settled)) { + // Supersession is retryable only on a later readiness request. Starting + // exactly one attempt here coalesces concurrent callers and avoids an + // unbounded retry loop when hydration keeps superseding initialization. + await startSnapshotCoordinationInitialization(root); + const retried = entries.get(root); + if (retried?.state === 'closing') { + throw new Error( + 'coordination initialization is closing for reset-session', + ); + } + if (retried?.state !== 'succeeded') { + throw new Error(retried?.error ?? 'coordination initialization failed'); + } + return; + } await entry.underlying; - if (entry.state !== 'succeeded') + if (isReadinessEntryClosing(entry)) { + throw new Error('coordination initialization is closing for reset-session'); + } + if (entry.state !== 'succeeded') { throw new Error(entry.error ?? 'coordination initialization failed'); + } } export function retrySnapshotCoordinationInitialization( @@ -391,13 +517,22 @@ export function markSnapshotCoordinationClosing(directory: string): void { export const _snapshotCoordinationInternals: { entries: Map; - initialize: (directory: string, scope?: HydrationScope) => Promise; + initialize: ( + directory: string, + scope?: HydrationScope, + ) => Promise; + loadPlan: typeof loadPlan; + readSnapshotFileStrict: typeof readSnapshotFileStrict; + importSnapshotRowsOnce: typeof importSnapshotRowsOnce; renameLegacySnapshot: (from: string, to: string) => void; - writeProjection: (directory: string, snapshot: SnapshotData) => Promise; + writeProjection: typeof writeSnapshotProjection; timeoutMs: number; } = { entries, initialize: initializeSnapshotCoordination, + loadPlan, + readSnapshotFileStrict, + importSnapshotRowsOnce, renameLegacySnapshot: renameSync, writeProjection: writeSnapshotProjection, timeoutMs: READY_TIMEOUT_MS, diff --git a/src/session/snapshot-reader.ts b/src/session/snapshot-reader.ts index 17d46f351..40816f6ac 100644 --- a/src/session/snapshot-reader.ts +++ b/src/session/snapshot-reader.ts @@ -22,10 +22,12 @@ import { bunFile } from '../utils/bun-compat'; import { log } from '../utils/logger.js'; import { beginHydrationScope, - currentHydrationGeneration, + captureCurrentHydrationAuthority, type HydrationScope, hydratedAggregateKeysFor, hydrationProjectKey, + isHydrationAuthorityCurrent, + isHydrationScopeCurrent, recordHydratedAggregateKeys, } from './hydration-ownership.js'; import { @@ -40,6 +42,10 @@ import type { } from './snapshot-writer'; import { SNAPSHOT_PROJECTION_FILE } from './snapshot-writer'; +export const _internals = { + recordInterruptedExecution, +}; + /** * Transient session fields that must be reset on rehydration. * Centralised here to keep the reset logic DRY and auditable. @@ -416,9 +422,10 @@ export async function readSnapshotFileStrict( /** * Rehydrate swarmState from a SnapshotData object. * - * Issue #2667 — project-owned and generation-fenced: + * Issues #2667/#2668 — project-owned and authority-fenced: * - With a `directory`, this replaces ONLY the state owned by that project - * (sessions whose `owningProjectKey` matches and whose `hydrationStamp` is + * (sessions whose `owningProjectKey` matches and whose authority epoch is + * older than the applying epoch, or whose current-epoch `hydrationStamp` is * at or below the applying generation). Other projects' live state — and * this project's sessions created after the hydration began — survive. * - With an explicit `scope` captured at initiation, the apply is refused @@ -455,14 +462,24 @@ export async function rehydrateState( } const projectKey = hydrationProjectKey(directory); - // Implicit scope: the CURRENT generation, no bump. A stale callback that - // carries no scope cannot evade the stamp predicate — sessions created - // after the latest hydration began are stamped above it and survive. - const generation = - scope?.generation ?? currentHydrationGeneration(projectKey); - if (scope && currentHydrationGeneration(projectKey) > scope.generation) { + // Implicit scope: capture the exact CURRENT authority, no bump. A stale + // callback that carries no scope cannot evade the stamp predicate — sessions + // created after the latest hydration began are stamped above it and survive. + // The authority epoch also detects a project record that was evicted/reset + // and reintroduced with the same numeric generation (ABA). + const authority = scope ?? captureCurrentHydrationAuthority(projectKey); + const generation = authority.generation; + // A live session is newer than this hydration only when it belongs to the + // current authority incarnation and carries a stamp above the generation. + // Comparing the epoch first closes the ABA window where FIFO eviction or a + // reset reintroduces the same project at generation 1 while old sessions + // still carry a larger numeric stamp from the prior incarnation. + const isCurrentAuthoritySession = (session: AgentSessionState): boolean => + session.owningProjectKey === projectKey && + session.hydrationAuthorityEpoch === authority.authorityEpoch; + if (scope && !isHydrationAuthorityCurrent(scope)) { log( - `[snapshot-reader] Refusing superseded hydration generation ${scope.generation} for ${projectKey} (current ${currentHydrationGeneration(projectKey)})`, + `[snapshot-reader] Refusing superseded hydration generation ${scope.generation} for ${projectKey}`, ); return { applied: false, reason: 'superseded' }; } @@ -474,6 +491,75 @@ export async function rehydrateState( if (swarmState.pendingRehydrations.size > 0) { await Promise.allSettled([...swarmState.pendingRehydrations]); } + if (!isHydrationAuthorityCurrent(authority)) { + log( + `[snapshot-reader] Refusing superseded hydration generation ${generation} for ${projectKey} after waiting for pending rehydrations`, + ); + return { applied: false, reason: 'superseded' }; + } + + // Interrupted-execution reconciliation is durable-first, but its write may + // suspend while a newer hydration takes authority for this project. Prepare + // those bounded records before touching shared rehydrated state, then fence + // the one synchronous publication section below with the exact authority. + const isProtectedLiveSession = (sessionId: string): boolean => { + const live = swarmState.agentSessions.get(sessionId); + return ( + live !== undefined && + isCurrentAuthoritySession(live) && + (live.hydrationStamp ?? 0) > generation + ); + }; + const interruptedReconciliations = new Map< + string, + { + entry: { sessionId: string; agentName: string; taskId: string }; + guidance: string; + } + >(); + if (directory && snapshot.agentSessions) { + for (const [sessionId, serializedSession] of Object.entries( + snapshot.agentSessions, + )) { + if ( + isProtectedLiveSession(sessionId) || + !serializedSession || + typeof serializedSession !== 'object' || + typeof serializedSession.agentName !== 'string' || + typeof serializedSession.lastToolCallTime !== 'number' || + serializedSession.delegationActive !== true + ) { + continue; + } + const entry = { + sessionId, + agentName: serializedSession.agentName, + taskId: serializedSession.currentTaskId || '(unknown)', + }; + try { + const recorded = await _internals.recordInterruptedExecution( + directory, + entry, + ); + interruptedReconciliations.set(sessionId, { + entry, + guidance: recorded.guidance, + }); + } catch (error) { + log( + `[snapshot-reader] restart reconciliation failed for session ${sessionId}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + } + if (!isHydrationAuthorityCurrent(authority)) { + log( + `[snapshot-reader] Refusing superseded hydration generation ${generation} for ${projectKey} after restart reconciliation preflight`, + ); + return { applied: false, reason: 'superseded' }; + } // Evict ONLY this project's own snapshot-derived sessions (stamp at or // below this generation). Unowned sessions and foreign projects' sessions @@ -481,7 +567,8 @@ export async function rehydrateState( for (const [sessionId, session] of swarmState.agentSessions) { if ( session.owningProjectKey === projectKey && - (session.hydrationStamp ?? 0) <= generation + (!isCurrentAuthoritySession(session) || + (session.hydrationStamp ?? 0) <= generation) ) { swarmState.agentSessions.delete(sessionId); swarmState.activeAgent.delete(sessionId); @@ -489,25 +576,10 @@ export async function rehydrateState( } } - // A live session created after this hydration began (stamp > generation) - // must survive population too, not just eviction: the snapshot can still - // carry its sessionId from a previous process (stable host session ids), - // and an unconditional set would replace the live object — discarding its - // unsnapshotted in-memory state and downgrading its stamp (PR #2742 - // review PRR-001). - const isProtectedLiveSession = (sessionId: string): boolean => { - const live = swarmState.agentSessions.get(sessionId); - return ( - live !== undefined && - live.owningProjectKey === projectKey && - (live.hydrationStamp ?? 0) > generation - ); - }; - // toolAggregates: replace only the keys this project's previous hydration // published. Keys owned by other projects' snapshots (or produced by // runtime increments outside this project's hydration set) are untouched. - const ownAggregateKeys = hydratedAggregateKeysFor(projectKey); + const ownAggregateKeys = hydratedAggregateKeysFor(projectKey, authority); const snapshotAggregateKeys = new Set( Object.keys(snapshot.toolAggregates ?? {}), ); @@ -519,7 +591,7 @@ export async function rehydrateState( for (const [key, value] of Object.entries(snapshot.toolAggregates ?? {})) { swarmState.toolAggregates.set(key, value); } - recordHydratedAggregateKeys(projectKey, snapshotAggregateKeys); + recordHydratedAggregateKeys(projectKey, snapshotAggregateKeys, authority); // Populate agentSessions with deserialized data // v6.33.1: Skip malformed sessions missing required fields instead of injecting bad state @@ -559,6 +631,7 @@ export async function rehydrateState( // snapshot bytes — the fields are not serialized at all). session.owningProjectKey = projectKey; session.hydrationStamp = generation; + session.hydrationAuthorityEpoch = authority.authorityEpoch; // ── Timestamps ──────────────────────────────────────────────── // Refresh timestamps so the stale eviction sweep in startAgentSession @@ -637,33 +710,22 @@ export async function rehydrateState( // advisory pushed after the reset so it survives) so an // interrupted execution can never read as a clean shutdown. // Fail-open: the record must never fail the rehydrate. - if (directory && serializedSession.delegationActive === true) { - const entry = { - sessionId, - agentName: session.agentName, - taskId: serializedSession.currentTaskId || '(unknown)', - }; - try { - const recorded = await recordInterruptedExecution(directory, entry); - // pushAdvisory (not a bare push) per the advisory-injection - // ratchet: bounded queue + dedupe. The dedupe key is embedded - // literally in the message text by the builder below — - // pushAdvisory matches keys by substring against queued text. - pushAdvisory( - session, - buildInterruptedAdvisoryMessage({ - ...entry, - guidance: recorded.guidance, - }), - { dedupeKey: buildInterruptedAdvisoryDedupeKey(entry) }, - ); - } catch (error) { - log( - `[snapshot-reader] restart reconciliation failed for session ${sessionId}: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } + const reconciliation = interruptedReconciliations.get(sessionId); + if (reconciliation) { + // pushAdvisory (not a bare push) per the advisory-injection + // ratchet: bounded queue + dedupe. The dedupe key is embedded + // literally in the message text by the builder below — + // pushAdvisory matches keys by substring against queued text. + pushAdvisory( + session, + buildInterruptedAdvisoryMessage({ + ...reconciliation.entry, + guidance: reconciliation.guidance, + }), + { + dedupeKey: buildInterruptedAdvisoryDedupeKey(reconciliation.entry), + }, + ); } // ── Full-auto run-state reconciliation ──────────────────────── @@ -849,8 +911,8 @@ async function rehydrateStateGlobal(snapshot: SnapshotData): Promise { * Called on plugin init to restore state from previous session. * NEVER throws - swallows any errors silently. * - * Issue #2667: a hydration scope is captured at entry so the eventual - * rehydrateState apply is generation-fenced — a loadSnapshot whose 5 s init + * Issues #2667/#2668: a hydration scope is captured at entry so the eventual + * rehydrateState apply is authority-fenced — a loadSnapshot whose 5 s init * timeout (src/index.ts) abandoned the await is refused once any newer * hydration for the same project has begun. */ @@ -862,15 +924,21 @@ export async function loadSnapshot(directory: string): Promise { // startAgentSession() will apply this cache synchronously, ensuring // guardrails see correct workflow state without a race. The cache is // per-project (hydration-ownership), so building it here cannot clobber - // another project's cache. - await buildRehydrationCache(directory); + // another project's cache. The scope predicate is checked at the final + // cache publication point, after all plan/evidence/config reads complete. + const cacheResult = await buildRehydrationCache(directory, { + shouldCommit: () => isHydrationScopeCurrent(scope), + }); + if (!cacheResult.committed || !isHydrationScopeCurrent(scope)) return; const snapshot = await readSnapshot(directory); if (snapshot !== null) { - await rehydrateState(snapshot, directory, scope); + const outcome = await rehydrateState(snapshot, directory, scope); + if (!outcome.applied || !isHydrationScopeCurrent(scope)) return; // Apply cached plan+evidence to every restored session before the // plugin begins accepting tool calls. for (const session of swarmState.agentSessions.values()) { + if (!isHydrationScopeCurrent(scope)) return; applyRehydrationCache(session); } // reconcileTaskStatesFromPlan() removed — superseded by applyRehydrationCache() diff --git a/src/session/snapshot-writer.ts b/src/session/snapshot-writer.ts index 74c3ca67f..379ac2909 100644 --- a/src/session/snapshot-writer.ts +++ b/src/session/snapshot-writer.ts @@ -7,12 +7,12 @@ import { closeSync, existsSync, + renameSync as fsRename, fsyncSync, mkdirSync, openSync, unlinkSync, } from 'node:fs'; -import { rename as fsRename } from 'node:fs/promises'; import * as path from 'node:path'; import { TASK_WORKFLOW_SCHEMA_MARKER } from '../gate-evidence.js'; import { validateSwarmPath } from '../hooks/utils'; @@ -61,34 +61,47 @@ const SNAPSHOT_RENAME_RETRY_DELAY_MS = 50; * on. Any non-transient code fails immediately — retrying an EACCES or * ENOENT would only delay the log. * + * Production uses synchronous rename so the authority check and the atomic OS + * operation happen in one event-loop turn. The DI seam accepts an async + * adapter for deterministic race tests; a resolved adapter only counts as a + * commit when its unique temp source is gone and the target exists. This also + * handles Windows reporting a transient error after the OS completed the + * rename, before a later authority check can suppress cache invalidation. + * * Throws the last rename error when the budget is exhausted; the caller owns * temp-file cleanup and error swallowing. */ async function renameWithTransientRetry( tempPath: string, targetPath: string, -): Promise { + shouldCommit?: () => boolean, +): Promise { let lastError: unknown; + const renameCommitted = () => !existsSync(tempPath) && existsSync(targetPath); for (let attempt = 0; attempt < SNAPSHOT_RENAME_MAX_ATTEMPTS; attempt++) { + if (shouldCommit && !shouldCommit()) return false; try { - await _internals.rename(tempPath, targetPath); - return; + await _internals.rename(tempPath, targetPath, shouldCommit); + // An async adapter may finish after its authority predicate has gone + // false and decline the swap with a void return. Do not mistake that + // for a commit or invalidate a cache for a file that stayed unchanged. + return renameCommitted(); } catch (error) { lastError = error; const code = (error as NodeJS.ErrnoException).code; // Windows can report a sharing violation for a rename that actually - // committed, so a retry then finds the source already gone. Treating - // that as a failure would skip the caller's cache invalidation for a - // file that really did change — the precise stale-read the #1729 - // invalidation exists to prevent. Only a retry can observe this, so - // the check is scoped to attempt > 0. + // committed. Check immediately, before the next iteration's authority + // check can return false and skip cache invalidation. ENOENT is treated + // as this case only on a retry: on the first attempt it can also mean + // the temp source disappeared before rename was attempted. if ( - code === 'ENOENT' && - attempt > 0 && - !existsSync(tempPath) && - existsSync(targetPath) + (code === 'EEXIST' || + code === 'EBUSY' || + code === 'EPERM' || + (code === 'ENOENT' && attempt > 0)) && + renameCommitted() ) { - return; + return true; } if (code !== 'EEXIST' && code !== 'EBUSY' && code !== 'EPERM') { break; @@ -253,7 +266,9 @@ export const SESSION_TRANSIENT_FIELDS: Readonly< owningProjectKey: 'Trust boundary (issue #2667): ownership is defined by the HYDRATING/creating directory, never by snapshot bytes — never serialized.', hydrationStamp: - 'Generation recency token (issue #2667); meaningful only within this process against the per-project generation counter.', + 'Process-local generation recency token (issue #2667); paired with hydrationAuthorityEpoch and never serialized.', + hydrationAuthorityEpoch: + 'Process-local project-authority epoch paired with hydrationStamp (issue #2668); never serialized.', lastScopeViolation: 'One-shot diagnostic for the current turn; a fresh process has observed no violations.', modifiedFilesThisCoderTask: @@ -565,6 +580,7 @@ export async function writeSnapshot( export async function writeSnapshotProjection( directory: string, snapshot: SnapshotData, + shouldCommit?: () => boolean, ): Promise { const content = JSON.stringify(snapshot, null, 2); @@ -577,22 +593,34 @@ export async function writeSnapshotProjection( // Atomic write: write to temp file then rename const tempPath = `${resolvedPath}.tmp.${Date.now()}.${Math.random().toString(36).slice(2)}`; - await bunWrite(tempPath, content); - // FR-004: fsync the temp file so the rename below cannot leave us with - // an empty or partial canonical file on power-loss / kill -9. try { - const fd = openSync(tempPath, 'r+'); + await bunWrite(tempPath, content); + // FR-004: fsync the temp file so the rename below cannot leave us with + // an empty or partial canonical file on power-loss / kill -9. try { - fsyncSync(fd); - } finally { - closeSync(fd); + const fd = openSync(tempPath, 'r+'); + try { + fsyncSync(fd); + } finally { + closeSync(fd); + } + } catch { + // fsync is best-effort; OSes / filesystems that don't support it + // (e.g. tmpfs, ramdisk) shouldn't block the main path. } - } catch { - // fsync is best-effort; OSes / filesystems that don't support it - // (e.g. tmpfs, ramdisk) shouldn't block the main path. - } - try { - await renameWithTransientRetry(tempPath, resolvedPath); + const renamed = await renameWithTransientRetry( + tempPath, + resolvedPath, + shouldCommit, + ); + if (!renamed) return; + // Only after a SUCCESSFUL rename. The projection may be read through the + // cached artifact reader, and this writer runs on every + // tool.execute.after — a snapshot whose only delta is a counter or a + // timestamp field of identical width is the SAME SIZE as its predecessor, + // which the cache's stat stamp (mtime+ctime+size) cannot distinguish from + // "unchanged" inside one filesystem timestamp tick (issue #1729). + invalidateCachedArtifact(resolvedPath); } finally { // No-op after a successful swap (the temp path no longer exists); // drops the orphan when every retry failed, so a persistently locked @@ -606,13 +634,6 @@ export async function writeSnapshotProjection( /* already renamed or never created */ } } - // Only after a SUCCESSFUL rename. The projection may be read through the - // cached artifact reader, and this writer runs on every - // tool.execute.after — a snapshot whose only delta is a counter or a - // timestamp field of identical width is the SAME SIZE as its predecessor, - // which the cache's stat stamp (mtime+ctime+size) cannot distinguish from - // "unchanged" inside one filesystem timestamp tick (issue #1729). - invalidateCachedArtifact(resolvedPath); } /** @@ -656,7 +677,11 @@ export const _internals: { writeSnapshot: typeof writeSnapshot; createSnapshotWriterHook: typeof createSnapshotWriterHook; flushPendingSnapshot: typeof flushPendingSnapshot; - rename: typeof fsRename; + rename: ( + from: string, + to: string, + shouldCommit?: () => boolean, + ) => void | Promise; } = { writeSnapshot, createSnapshotWriterHook, diff --git a/src/state.ts b/src/state.ts index da7c48e79..916749f93 100644 --- a/src/state.ts +++ b/src/state.ts @@ -68,9 +68,12 @@ import { clearScopeBindings } from './scope/scope-binding.js'; import { clearScopeBindingFromDisk } from './scope/scope-persistence.js'; import { clearAllTurnLedgers } from './services/injection-budget'; import { + captureCurrentHydrationAuthority, clearHydrationOwnershipState, getRehydrationCache, + type HydrationAuthorityToken, hydrationProjectKey, + isHydrationAuthorityCurrent, nextSessionHydrationStamp, setRehydrationCache, } from './session/hydration-ownership.js'; @@ -88,6 +91,8 @@ import { AgentRunContext } from './state/agent-run-context.js'; import { telemetry } from './telemetry.js'; import * as logger from './utils/logger'; +// Kept as a read-only diagnostic seam for restart/cache reconciliation tests. +export { getRehydrationCache }; export { AgentRunContext } from './state/agent-run-context.js'; /** @@ -110,6 +115,24 @@ interface RehydrationCache { }; } +/** Optional controls for rebuilding the project-owned rehydration cache. */ +export interface RehydrationCacheBuildOptions { + /** + * Explicitly recovered plan state. Presence of this property, including a + * null value, means the caller already resolved the authoritative plan and + * the builder must not fall back to the derived plan.json projection. + */ + planOverride?: Plan | null; + /** Return false when the captured hydration scope was superseded mid-read. */ + shouldCommit?: () => boolean; +} + +/** Result of a cache rebuild, including whether its publication committed. */ +export interface RehydrationCacheBuildResult { + committed: boolean; + reason?: 'superseded'; +} + /** * Tracks plan IDs that have already received the "council disagreement" warn. * One warning per plan_id, per process lifetime. Cleared by resetSwarmState. @@ -699,11 +722,21 @@ export interface AgentSessionState { * Hydration generation this session was created/restored AT. * `startAgentSession` stamps `currentGeneration + 1` (newer than any * in-flight hydration); `rehydrateState` stamps its applying generation. - * A hydration at generation `g` evicts only owned sessions with - * `hydrationStamp <= g`, so live sessions created after `g` began always - * survive. Also never snapshotted. + * A hydration at authority `{ generation: g, epoch: e }` evicts owned + * sessions from another epoch and same-epoch sessions with + * `hydrationStamp <= g`, so sessions created after `g` began survive only + * within the same authority incarnation. + * `hydrationAuthorityEpoch` disambiguates a numerically reused generation + * after bounded project-authority eviction or reset. Also never snapshotted. */ hydrationStamp?: number; + /** + * Process-local authority epoch paired with `hydrationStamp`. A session from + * an older project-authority incarnation is stale even when its numeric + * generation is larger than the current generation (issue #2668 ABA guard). + * Deliberately omitted from snapshots with the other ownership fields. + */ + hydrationAuthorityEpoch?: number; // PRM (Process Remediation Manager) - Phase 1 /** Pattern type to detection count mapping */ @@ -2155,9 +2188,16 @@ export function startAgentSession( const owningProjectKey = directory ? hydrationProjectKey(directory) : undefined; + // Direct session rehydration does not initiate a hydration, so capture the + // exact current authority before any await. This also ensures a bounded + // project record exists after FIFO eviction without bumping generation. + const hydrationAuthority = owningProjectKey + ? captureCurrentHydrationAuthority(owningProjectKey) + : undefined; const hydrationStamp = owningProjectKey ? nextSessionHydrationStamp(owningProjectKey) : undefined; + const hydrationAuthorityEpoch = hydrationAuthority?.authorityEpoch; // Evict stale sessions based on last activity, not start time. // Default: 2 hours — should exceed typical agent durations (evicts inactive @@ -2244,6 +2284,7 @@ export function startAgentSession( sessionRehydratedAt: 0, owningProjectKey, hydrationStamp, + hydrationAuthorityEpoch, // PRM (Process Remediation Manager) - Phase 1 prmPatternCounts: new Map(), prmEscalationLevel: 0, @@ -2303,16 +2344,28 @@ export function startAgentSession( // before clearing agentSessions, preventing a race that would silently discard // in-flight workflow state. if (directory) { + const liveSession = sessionState; + const shouldCommitRehydration = () => + hydrationAuthority !== undefined && + isHydrationAuthorityCurrent(hydrationAuthority) && + swarmState.agentSessions.get(sessionId) === liveSession; let rehydrationPromise: Promise; rehydrationPromise = _internals - .rehydrateSessionFromDisk(directory, sessionState) + .rehydrateSessionFromDisk( + directory, + sessionState, + shouldCommitRehydration, + ) .then(async () => { + if (!shouldCommitRehydration()) return; // Rehydrate PR subscriptions for this session (fail-open). try { - sessionState.prSubscriptions = await rehydratePrSubscriptions( + const subscriptions = await _internals.rehydratePrSubscriptions( sessionId, directory, ); + if (!shouldCommitRehydration()) return; + sessionState.prSubscriptions = subscriptions; } catch (err) { logger.warn( '[state] PR subscription rehydration failed, starting with empty subscriptions:', @@ -3708,10 +3761,23 @@ async function readGateEvidenceFromDisk( * refreshed after compaction by the compaction hook (src/hooks/compaction-customizer.ts). * Non-fatal: missing/malformed files leave an empty cache. */ -export async function buildRehydrationCache(directory: string): Promise { +export async function buildRehydrationCache( + directory: string, + options?: RehydrationCacheBuildOptions, +): Promise { + const projectKey = hydrationProjectKey(directory); + // Capture the authority before any filesystem/config await. Publication + // must use this exact incarnation; setter-time capture would let a delayed + // pre-eviction build be mislabeled current after FIFO reintroduction (ABA). + const buildAuthority: HydrationAuthorityToken = + captureCurrentHydrationAuthority(projectKey); const planTaskStates = new Map(); - const plan = await readPlanFromDisk(directory); + const plan = + options && Object.hasOwn(options, 'planOverride') + ? options.planOverride + : await readPlanFromDisk(directory); + const cachePlan = plan ?? null; if (plan) { for (const phase of plan.phases ?? []) { for (const task of phase.tasks ?? []) { @@ -3731,11 +3797,24 @@ export async function buildRehydrationCache(directory: string): Promise { } catch { councilConfig = undefined; } - setRehydrationCache(hydrationProjectKey(directory), { - planTaskStates, - evidenceMap, - taskIdentityContext: { plan, councilConfig }, - } satisfies RehydrationCache); + if ( + (options?.shouldCommit && !options.shouldCommit()) || + !isHydrationAuthorityCurrent(buildAuthority) + ) { + return { committed: false, reason: 'superseded' }; + } + const committed = setRehydrationCache( + projectKey, + { + planTaskStates, + evidenceMap, + taskIdentityContext: { plan: cachePlan, councilConfig }, + } satisfies RehydrationCache, + buildAuthority, + ); + return committed + ? { committed: true } + : { committed: false, reason: 'superseded' }; } /** @@ -3912,8 +3991,15 @@ export function applyRehydrationCache( export async function rehydrateSessionFromDisk( directory: string, session: AgentSessionState, + shouldCommit?: () => boolean, ): Promise { - await _internals.buildRehydrationCache(directory); + const cacheBuild = shouldCommit + ? await _internals.buildRehydrationCache(directory, { shouldCommit }) + : await _internals.buildRehydrationCache(directory); + // Keep compatibility with existing DI stubs that predate the result object, + // while refusing to apply a superseded production rebuild. + if (cacheBuild && !cacheBuild.committed) return; + if (shouldCommit && !shouldCommit()) return; _internals.applyRehydrationCache(session, hydrationProjectKey(directory)); } @@ -4515,6 +4601,7 @@ export const _internals: { buildRehydrationCache: typeof buildRehydrationCache; applyRehydrationCache: typeof applyRehydrationCache; rehydrateSessionFromDisk: typeof rehydrateSessionFromDisk; + rehydratePrSubscriptions: typeof rehydratePrSubscriptions; isCouncilGateActive: typeof isCouncilGateActive; defaultRunContext: typeof defaultRunContext; } = { @@ -4534,6 +4621,7 @@ export const _internals: { buildRehydrationCache, applyRehydrationCache, rehydrateSessionFromDisk, + rehydratePrSubscriptions, isCouncilGateActive, defaultRunContext, }; diff --git a/src/utils/atomic-write.ts b/src/utils/atomic-write.ts index 099ec6c85..cd073bab4 100644 --- a/src/utils/atomic-write.ts +++ b/src/utils/atomic-write.ts @@ -131,9 +131,9 @@ export const SWARM_TEMP_GRAMMARS: readonly SwarmTempGrammar[] = [ 'src/evidence/task-file.ts (pre-#2035; migrated-to=src/evidence/task-file.ts:atomicWriteFile)', 'src/scope/scope-persistence.ts (pre-#2035; migrated-to=src/scope/scope-persistence.ts:atomicWrite)', 'src/scope/scope-persistence.ts (pre-#2035; migrated-to=src/scope/scope-persistence.ts:atomicWriteSync)', - 'src/plan/manager.ts:662', - 'src/plan/manager.ts:1988', - 'src/plan/manager.ts:2042', + 'src/plan/manager.ts:683', + 'src/plan/manager.ts:2110', + 'src/plan/manager.ts:2170', 'src/review/evidence.ts:175', 'src/turbo/lean/evidence.ts:182', 'src/summaries/manager.ts:163', @@ -149,7 +149,7 @@ export const SWARM_TEMP_GRAMMARS: readonly SwarmTempGrammar[] = [ 'src/evidence/phase-participation.ts (pre-#2035; migrated-to=src/evidence/phase-participation.ts:atomicWriteBytes)', 'src/turbo/lean/integration.ts:428', 'src/turbo/lean/reviewer.ts:403', - 'src/plan/ledger.ts:1463', + 'src/plan/ledger.ts:1471', 'src/services/synonym-map.ts:389', 'src/services/skill-optimizer/store.ts:340', 'src/services/skill-optimizer/store.ts:503', @@ -263,7 +263,7 @@ export const SWARM_TEMP_GRAMMARS: readonly SwarmTempGrammar[] = [ token: 'instance', quarantineEligible: true, parsesTarget: true, - producers: ['src/plan/manager.ts:2390', 'src/plan/manager.ts:2424'], + producers: ['src/plan/manager.ts:2369', 'src/plan/manager.ts:2431'], note: 'plan-durability (invariant 5) fd-write paths; grammars registered, writers unchanged in this PR', }, { @@ -330,7 +330,7 @@ export const SWARM_TEMP_GRAMMARS: readonly SwarmTempGrammar[] = [ 'src/hooks/skill-usage-pending.ts:817', 'src/parallel/file-locks.ts:113', 'src/plan/ledger.ts:607', - 'src/plan/ledger.ts:1777', + 'src/plan/ledger.ts:1793', ], note: 'constant-name temps: reported when stale, never auto-quarantined. file-locks meta sidecars live under the scanner-skipped .swarm/locks/ subtree; the ledger reconcile temp embeds instance tokens but its .tmp-suffix shape maps here (conservatively report-only)', }, diff --git a/tests/unit/commands/recover-coordination.test.ts b/tests/unit/commands/recover-coordination.test.ts index 4e8ab84d7..1ac64ff02 100644 --- a/tests/unit/commands/recover-coordination.test.ts +++ b/tests/unit/commands/recover-coordination.test.ts @@ -137,6 +137,7 @@ describe('/swarm recover --coordination (#2481)', () => { let calls = 0; _snapshotCoordinationInternals.initialize = async () => { calls += 1; + return 'succeeded'; }; const output = await handleRecoverCommand(directory, ['--coordination']); diff --git a/tests/unit/db/qa-gate-session-override.test.ts b/tests/unit/db/qa-gate-session-override.test.ts index 1b63358e7..a40701400 100644 --- a/tests/unit/db/qa-gate-session-override.test.ts +++ b/tests/unit/db/qa-gate-session-override.test.ts @@ -14,20 +14,24 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { getProjectDb } from '../../../src/db/project-db.js'; import { + _internals, clearAllSessionOverrides, clearOverrideForSession, getOverrideForSession, setOverrideForSession, + sweepOrphanOverrides, } from '../../../src/db/qa-gate-session-override.js'; import { canonicalMkdtemp } from '../../helpers/tmpdir.js'; let tempDir: string; +const originalDeleteOrphanOverrideBatch = _internals.deleteOrphanOverrideBatch; beforeEach(() => { tempDir = canonicalMkdtemp('qa-gate-session-override-test-'); }); afterEach(() => { + _internals.deleteOrphanOverrideBatch = originalDeleteOrphanOverrideBatch; try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { @@ -139,3 +143,47 @@ describe('clearAllSessionOverrides', () => { expect(clearAllSessionOverrides(tempDir)).toBe(0); }); }); + +describe('sweepOrphanOverrides — bounded batches (PR2767-COPILOT-002)', () => { + test('deletes all orphans in at most 500-ID batches and preserves live rows', () => { + // Before batching, cleanup issued one DELETE per orphan, scaling statement + // count linearly with stale sessions during the rehydrate transaction. + const orphanCount = 1_001; + setOverrideForSession(tempDir, 'live-session', { mutation_test: true }); + const db = getProjectDb(tempDir); + const seedOrphans = db.transaction(() => { + for (let index = 0; index < orphanCount; index += 1) { + db.run( + "INSERT INTO qa_gate_session_override (session_id, gates, updated_at) VALUES (?, ?, datetime('now'))", + [`orphan-${index}`, '{}'], + ); + } + }); + seedOrphans(); + + const batchSizes: number[] = []; + _internals.deleteOrphanOverrideBatch = (batchDb, sessionIds) => { + batchSizes.push(sessionIds.length); + return originalDeleteOrphanOverrideBatch(batchDb, sessionIds); + }; + + const removed = sweepOrphanOverrides(tempDir, new Set(['live-session'])); + + expect(removed).toBe(orphanCount); + expect(batchSizes.length).toBeGreaterThan(0); + expect( + batchSizes.every((batchSize) => batchSize > 0 && batchSize <= 500), + ).toBe(true); + expect(batchSizes.reduce((total, batchSize) => total + batchSize, 0)).toBe( + orphanCount, + ); + expect(batchSizes.length).toBeLessThan(orphanCount); + expect( + db + .query<{ session_id: string }, []>( + 'SELECT session_id FROM qa_gate_session_override ORDER BY session_id', + ) + .all(), + ).toEqual([{ session_id: 'live-session' }]); + }); +}); diff --git a/tests/unit/execute-journey/j08-restart-policy-reconciliation.test.ts b/tests/unit/execute-journey/j08-restart-policy-reconciliation.test.ts new file mode 100644 index 000000000..6e22c63d9 --- /dev/null +++ b/tests/unit/execute-journey/j08-restart-policy-reconciliation.test.ts @@ -0,0 +1,309 @@ +/** + * j08 — restart policy reconciliation through the real registered host + * (issue #2668). Durable plan identity and session-scoped QA policy are + * recovered on boot B; ephemeral auto-proceed authority is intentionally + * absent. The journey also inspects an interrupted task through the host, + * then directly exercises the settlement classifier and proves that a late + * result from the old workflow generation cannot clear the newly accepted one. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { readFileSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { handleAutoProceedCommand } from '../../../src/commands/auto-proceed'; +import { handleQaGatesCommand } from '../../../src/commands/qa-gates'; +import { + getTaskWorkflowSnapshot, + readTaskEvidence, +} from '../../../src/gate-evidence'; +import { + resetSwarmState, + resetSwarmStatePreservingSingletons, + swarmState, +} from '../../../src/state'; +import { classifySettlementWalState } from '../../../src/workflow/task-recovery-status'; +import type { CoderSettlementWalState } from '../../../src/workflow/workflow-wal-schema'; +import { + bootJourneyHost, + commitWorkingTree, + createJourneyProject, + JourneyDriver, + journeyPlanArgs, + parseToolResult, +} from '../../helpers/execute-journey-driver'; +import { createIsolatedTestEnv } from '../../helpers/isolated-test-env'; + +const TASK_ID = '1.1'; +const FILE = 'src/restart-feature.ts'; + +function oldSettlement( + directory: string, + generation: number, +): CoderSettlementWalState { + return { + taskId: TASK_ID, + state: 'DISPATCHED', + transitionId: 'journey-j08-old-generation', + actor: 'coder', + processId: 0, + runtimeId: '00000000-0000-4000-8000-000000000000', + expectedGeneration: generation, + context: { + baseline: { + directory, + gitHead: null, + dirtyHash: null, + prHeadSha: null, + scope: null, + changedFiles: [], + }, + declaredFiles: [FILE], + }, + accepted: true, + recordedAt: '2026-01-01T00:00:00.000Z', + } as unknown as CoderSettlementWalState; +} + +function passPayload(): Record { + return { + gates_passed: true, + batch_status: 'completed', + total_duration_ms: 1, + lint: { ran: true, duration_ms: 1 }, + secretscan: { + ran: true, + duration_ms: 1, + result: { + count: 0, + findings: [], + files_scanned: 1, + incomplete_files: 0, + incomplete_paths: [], + }, + }, + sast_scan: { ran: true, duration_ms: 1, result: { verdict: 'pass' } }, + quality_budget: { ran: false, duration_ms: 0 }, + }; +} + +describe('restart policy reconciliation through the registered host plus direct settlement classification (#2668)', () => { + let project: ReturnType | null = null; + let cleanupEnv: (() => void) | null = null; + + beforeEach(() => { + cleanupEnv = createIsolatedTestEnv().cleanup; + resetSwarmState(); + }); + + afterEach(() => { + resetSwarmState(); + cleanupEnv?.(); + cleanupEnv = null; + project?.cleanup(); + project = null; + }); + + test('durable policy and approved identity survive restart while process-local authority expires', async () => { + project = createJourneyProject('swarm-j08-'); + const bootA = await bootJourneyHost({ directory: project.directory }); + const driverA = new JourneyDriver(bootA); + await driverA.configure(); + await driverA.specify(journeyPlanArgs({ taskId: TASK_ID, file: FILE })); + + // Persist one gate through the registered tool and the second through the + // production qa-gates enable command. Both are plan policy, not session + // authority, and must be visible after the process reset. + const persisted = parseToolResult( + await bootA.host.tool.set_qa_gates.execute( + { test_engineer: true }, + { directory: project.directory, sessionID: driverA.sessionID }, + ), + ); + expect(persisted.success).toBe(true); + const enable = await handleQaGatesCommand( + project.directory, + ['enable', 'reviewer'], + driverA.sessionID, + ); + expect(enable).toContain('Enabled gates persisted'); + + // The QA override is durable session-scoped policy; auto-proceed is a + // process-local execution control that must expire across restart. + const override = await handleQaGatesCommand( + project.directory, + ['override', 'sast_enabled'], + driverA.sessionID, + ); + expect(override).toContain('Session overrides updated'); + expect( + await handleAutoProceedCommand( + project.directory, + ['on'], + driverA.sessionID, + ), + ).toContain('Auto-proceed is now ON'); + expect( + swarmState.agentSessions.get(driverA.sessionID)?.autoProceedOverride, + ).toBe(true); + expect( + swarmState.agentSessions.get(driverA.sessionID)?.qaGateSessionOverrides + ?.sast_enabled, + ).toBe(true); + + const { approval, binding: bindingA } = await driverA.approve( + 'journey fixture j08 approval', + ); + expect(bindingA.success).toBe(true); + await driverA.executeCoder({ + taskId: TASK_ID, + file: FILE, + mutate: () => + writeFileSync( + path.join(project!.directory, FILE), + 'export const restarted = 1;\n', + ), + }); + const beforeRestart = getTaskWorkflowSnapshot( + await readTaskEvidence(project.directory, TASK_ID), + ); + expect(beforeRestart.state).toBe('coder_delegated'); + + // Simulated process death retains durable stores but drops all session + // execution authority and in-memory ownership. + resetSwarmStatePreservingSingletons(); + const bootB = await bootJourneyHost({ directory: project.directory }); + const driverB = new JourneyDriver(bootB); + await driverB.configure(); + + const profile = parseToolResult( + await bootB.host.tool.get_qa_gate_profile.execute( + {}, + { directory: project.directory, sessionID: driverB.sessionID }, + ), + ); + expect(profile.success).toBe(true); + expect(profile.profile.gates.reviewer).toBe(true); + expect(profile.profile.gates.test_engineer).toBe(true); + const bindingB = parseToolResult( + await bootB.host.tool.get_approved_plan.execute( + { summary_only: true }, + { directory: project.directory, sessionID: driverB.sessionID }, + ), + ); + expect(bindingB.success).toBe(true); + expect(bindingB.drift_detected).toBe(false); + expect(String(approval.plan_id)).toBe(String(driverA.planBinding?.planId)); + expect(String(driverA.planBinding?.approvedPayloadHash)).toBe( + String((bindingB.approved_plan as Record).payload_hash), + ); + expect( + swarmState.agentSessions.get(driverB.sessionID)?.autoProceedOverride, + ).toBeUndefined(); + expect( + swarmState.agentSessions.get(driverB.sessionID)?.qaGateSessionOverrides, + ).toEqual({ sast_enabled: true }); + expect( + await handleQaGatesCommand( + project.directory, + ['show'], + driverB.sessionID, + ), + ).toContain( + 'Session overrides (ratchet-tighter only):\n - sast_enabled: on (override)', + ); + + const inspect = await driverB.inspectTask({ taskId: TASK_ID }); + expect(inspect.workflow).toMatchObject({ + state: 'coder_delegated', + generation: 1, + }); + + // Owner-visible inspection remains bounded and typed after restart: + // interrupted/dead is repairable, cancelled is terminal, a live foreign + // owner is uncertain, and unreadable evidence is corrupt. + const interrupted = classifySettlementWalState( + oldSettlement(project.directory, beforeRestart.generation), + beforeRestart, + ); + expect(interrupted.category).toBe('stale'); + expect(interrupted.repairAllowed).toBe(true); + const cancelled = classifySettlementWalState( + { + ...oldSettlement(project.directory, 1), + state: 'ABORTED', + } as CoderSettlementWalState, + beforeRestart, + ); + expect(cancelled.category).toBe('healthy'); + const uncertain = classifySettlementWalState( + { + ...oldSettlement(project.directory, 1), + ownedByLiveForeignPid: true, + processId: 99, + } as CoderSettlementWalState, + beforeRestart, + ); + expect(uncertain.category).toBe('ambiguous'); + expect(uncertain.uncertainExternalEffect).toBe(true); + const corrupt = classifySettlementWalState( + { + ...oldSettlement(project.directory, 1), + state: 'unreadable', + } as CoderSettlementWalState, + null, + ); + expect(corrupt.category).toBe('corrupt'); + expect(corrupt.repairAllowed).toBe(false); + + // Re-dispatch through the registered journey to open a new generation. + const lateCallID = 'journey-j08-late-old'; + await bootB.host.hooks['tool.execute.before']( + { + tool: 'pre_check_batch', + sessionID: driverB.sessionID, + callID: lateCallID, + }, + { args: { files: [FILE], directory: project.directory } }, + ); + expect( + (await driverB.preCheck({ taskId: TASK_ID, file: FILE })).gates_passed, + ).toBe(true); + await driverB.dispatchStageB({ + role: 'reviewer', + taskId: TASK_ID, + file: FILE, + verdictLine: `[REVIEWED] | task-${TASK_ID} | REJECTED | restart rework`, + }); + commitWorkingTree(project.directory, 'test: commit j08 round one'); + await driverB.driveTaskDelegation({ + role: 'coder', + callID: 'journey-j08-new-generation', + taskId: TASK_ID, + file: FILE, + mutate: () => + writeFileSync( + path.join(project!.directory, FILE), + 'export const restarted = 2;\n', + ), + output: { state: 'completed', output: 'new generation accepted' }, + }); + const newGeneration = getTaskWorkflowSnapshot( + await readTaskEvidence(project.directory, TASK_ID), + ); + expect(newGeneration.state).toBe('coder_delegated'); + expect(newGeneration.generation).toBeGreaterThan(beforeRestart.generation); + + await bootB.host.hooks['tool.execute.after']( + { + tool: 'pre_check_batch', + sessionID: driverB.sessionID, + callID: lateCallID, + }, + { output: JSON.stringify(passPayload()), metadata: null }, + ); + const afterLate = getTaskWorkflowSnapshot( + await readTaskEvidence(project.directory, TASK_ID), + ); + expect(afterLate.state).toBe(newGeneration.state); + expect(afterLate.generation).toBe(newGeneration.generation); + }, 180_000); +}); diff --git a/tests/unit/hooks/system-enhancer-load-evidence.test.ts b/tests/unit/hooks/system-enhancer-load-evidence.test.ts index 5f776a6f1..79d49050f 100644 --- a/tests/unit/hooks/system-enhancer-load-evidence.test.ts +++ b/tests/unit/hooks/system-enhancer-load-evidence.test.ts @@ -46,6 +46,7 @@ mock.module('../../../src/plan/manager.js', () => ({ // imports getCurrentTaskId; the binding must exist while manager is mocked. getCurrentTaskId: () => null, loadPlanJsonOnly: mockLoadPlanJsonOnly, + PlanRecoverySupersededError: class PlanRecoverySupersededError extends Error {}, isTaskSettled: mock(() => false), derivePlanMarkdown: mock((plan: any) => '# Derived Plan\n'), savePlan: mock(), diff --git a/tests/unit/plan/ledger-recovery-predicate-propagation-2668.test.ts b/tests/unit/plan/ledger-recovery-predicate-propagation-2668.test.ts new file mode 100644 index 000000000..cdc9aa454 --- /dev/null +++ b/tests/unit/plan/ledger-recovery-predicate-propagation-2668.test.ts @@ -0,0 +1,170 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import { mkdir, readFile, rm, unlink, writeFile } from 'node:fs/promises'; +import * as path from 'node:path'; +import type { Plan } from '../../../src/config/plan-schema'; +import { closeProjectDb } from '../../../src/db/project-db'; +import { + computePlanLedgerHash, + initLedger, + replacePlanLedgerWithRoot, +} from '../../../src/plan/ledger'; +import { + cutoverSqliteLedger, + getPlanLedgerState, +} from '../../../src/plan/ledger-sqlite'; +import { PlanRecoverySupersededError } from '../../../src/plan/manager'; +import { derivePlanId } from '../../../src/plan/utils'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +function makePlan(title = 'Ledger predicate propagation'): Plan { + return { + schema_version: '1.0.0', + title, + swarm: 'ledger-predicate-propagation-2668', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: '1.1', + phase: 1, + status: 'pending', + size: 'small', + description: 'Keep typed authority failures visible', + depends: [], + files_touched: [], + }, + ], + }, + ], + }; +} + +describe('ledger recovery predicate propagation (#2668)', () => { + let directory = ''; + + beforeEach(async () => { + directory = canonicalMkdtemp('ledger-predicate-propagation-2668-'); + await mkdir(path.join(directory, '.swarm'), { recursive: true }); + await mkdir(path.join(directory, '.git')); + }); + + afterEach(async () => { + closeProjectDb(directory); + await rm(directory, { recursive: true, force: true }); + }); + + async function seedLedger(): Promise<{ plan: Plan; ledgerPath: string }> { + const plan = makePlan(); + await writeFile( + path.join(directory, '.swarm', 'plan.json'), + JSON.stringify(plan, null, 2), + 'utf8', + ); + await initLedger( + directory, + derivePlanId(plan), + computePlanLedgerHash(plan), + plan, + ); + return { + plan, + ledgerPath: path.join(directory, '.swarm', 'plan-ledger.jsonl'), + }; + } + + test('initLedger does not swallow supersession in optional SQLite shadow catch', async () => { + const { plan: originalPlan, ledgerPath } = await seedLedger(); + await unlink(ledgerPath); + const sqlitePlanIdBefore = getPlanLedgerState(directory)?.planId; + const newPlan = makePlan('New init root'); + const observedPlanIds: Array = []; + const superseded = new PlanRecoverySupersededError('new authority'); + + await expect( + initLedger( + directory, + derivePlanId(newPlan), + computePlanLedgerHash(newPlan), + newPlan, + { + preCommitCheck: () => { + const lines = fs.existsSync(ledgerPath) + ? fs.readFileSync(ledgerPath, 'utf8').trim().split('\n') + : []; + observedPlanIds.push( + lines.length > 0 + ? (JSON.parse(lines[0]!) as { plan_id: string }).plan_id + : null, + ); + if (observedPlanIds.length === 2) throw superseded; + }, + }, + ), + ).rejects.toBe(superseded); + // The first fence is before canonical publication; the second observes + // the new root after its atomic rename but before the SQLite shadow commit. + expect(observedPlanIds).toEqual([null, derivePlanId(newPlan)]); + expect(fs.existsSync(ledgerPath)).toBe(true); + expect(getPlanLedgerState(directory)?.planId).toBe(sqlitePlanIdBefore); + expect(sqlitePlanIdBefore).toBe(derivePlanId(originalPlan)); + }); + + test('replacePlanLedgerWithRoot does not swallow supersession before SQLite shadow publication', async () => { + const { plan, ledgerPath } = await seedLedger(); + const before = await readFile(ledgerPath, 'utf8'); + const newPlan = makePlan('Replacement root'); + const sqlitePlanIdBefore = getPlanLedgerState(directory)?.planId; + const observedPlanIds: string[] = []; + const superseded = new PlanRecoverySupersededError('new authority'); + + await expect( + replacePlanLedgerWithRoot(directory, newPlan, 'test-recovery', { + preCommitCheck: () => { + const line = fs.readFileSync(ledgerPath, 'utf8').trim(); + observedPlanIds.push( + (JSON.parse(line) as { plan_id: string }).plan_id, + ); + if (observedPlanIds.length === 3) throw superseded; + }, + }), + ).rejects.toBe(superseded); + // Both pre-archive and pre-file-publication fences see the old root; the + // third fence sees the replacement export before SQLite shadow publication. + expect(observedPlanIds).toEqual([ + derivePlanId(plan), + derivePlanId(plan), + derivePlanId(newPlan), + ]); + expect(await readFile(ledgerPath, 'utf8')).not.toBe(before); + expect(getPlanLedgerState(directory)?.planId).toBe(sqlitePlanIdBefore); + expect(sqlitePlanIdBefore).toBe(derivePlanId(plan)); + }); + + test('SQLite-authoritative export fence propagates supersession outside its catch', async () => { + const { plan, ledgerPath } = await seedLedger(); + const state = getPlanLedgerState(directory); + if (!state) throw new Error('fixture did not initialize SQLite state'); + cutoverSqliteLedger(directory, { + expectedShadowStartedVersion: state.shadowStartedVersion ?? undefined, + }); + const before = await readFile(ledgerPath, 'utf8'); + let checks = 0; + const superseded = new PlanRecoverySupersededError('new authority'); + + await expect( + replacePlanLedgerWithRoot(directory, plan, 'test-recovery', { + preCommitCheck: () => { + checks++; + if (checks === 2) throw superseded; + }, + }), + ).rejects.toBe(superseded); + expect(checks).toBe(2); + expect(await readFile(ledgerPath, 'utf8')).toBe(before); + }); +}); diff --git a/tests/unit/plan/manager-corrupt-ledger-quarantine-supersession-2668.test.ts b/tests/unit/plan/manager-corrupt-ledger-quarantine-supersession-2668.test.ts new file mode 100644 index 000000000..0ab2dae8c --- /dev/null +++ b/tests/unit/plan/manager-corrupt-ledger-quarantine-supersession-2668.test.ts @@ -0,0 +1,176 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import * as path from 'node:path'; +import type { Plan } from '../../../src/config/plan-schema'; +import { + type LedgerEvent, + quarantineLedgerSuffix, +} from '../../../src/plan/ledger'; +import { + loadPlan, + PlanRecoverySupersededError, + regeneratePlanMarkdown, + resetStartupLedgerCheck, +} from '../../../src/plan/manager'; +import { derivePlanId } from '../../../src/plan/utils'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +function makePlan(status: Plan['phases'][0]['tasks'][0]['status']): Plan { + return { + schema_version: '1.0.0', + title: 'Corrupt ledger quarantine supersession', + swarm: 'quarantine-supersession-2668', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: '1.1', + phase: 1, + status, + size: 'small', + description: 'Preserve the authoritative plan', + depends: [], + files_touched: [], + }, + ], + }, + ], + }; +} + +function eventLine(event: LedgerEvent): string { + return `${JSON.stringify(event)}\n`; +} + +async function seedCorruptLedger(directory: string): Promise<{ + ledgerPath: string; + planJsonPath: string; +}> { + const authoritative = makePlan('completed'); + const prefixPlan = makePlan('pending'); + const planId = derivePlanId(authoritative); + const swarmDir = path.join(directory, '.swarm'); + const planJsonPath = path.join(swarmDir, 'plan.json'); + const ledgerPath = path.join(swarmDir, 'plan-ledger.jsonl'); + + await writeFile(planJsonPath, JSON.stringify(authoritative, null, 2), 'utf8'); + await regeneratePlanMarkdown(directory, authoritative); + + const created: LedgerEvent = { + seq: 1, + timestamp: '2026-01-01T00:00:00.000Z', + plan_id: planId, + event_type: 'plan_created', + source: 'test', + plan_hash_before: '', + plan_hash_after: 'H1', + schema_version: '1.1.0', + payload: { plan: prefixPlan, payload_hash: 'H1' }, + }; + const progress: LedgerEvent = { + seq: 2, + timestamp: '2026-01-01T00:00:01.000Z', + plan_id: planId, + event_type: 'task_status_changed', + task_id: '1.1', + from_status: 'pending', + to_status: 'in_progress', + source: 'test', + plan_hash_before: 'H1', + plan_hash_after: 'H2', + schema_version: '1.1.0', + }; + const postPoison: LedgerEvent = { + seq: 4, + timestamp: '2026-01-01T00:00:03.000Z', + plan_id: planId, + event_type: 'task_status_changed', + task_id: '1.1', + from_status: 'in_progress', + to_status: 'completed', + source: 'test', + plan_hash_before: 'H2', + plan_hash_after: 'SENTINEL_CORRUPT_LEDGER_HASH', + schema_version: '1.1.0', + }; + + await writeFile( + ledgerPath, + eventLine(created) + + eventLine(progress) + + '{ POISON — integrity read must return this suffix\n' + + eventLine(postPoison), + 'utf8', + ); + return { ledgerPath, planJsonPath }; +} + +describe('loadPlan corrupt-ledger quarantine authority fence (#2668)', () => { + let directory = ''; + + beforeEach(async () => { + directory = canonicalMkdtemp('quarantine-supersession-2668-'); + await mkdir(path.join(directory, '.swarm'), { recursive: true }); + await mkdir(path.join(directory, '.git')); + resetStartupLedgerCheck(); + }); + + afterEach(async () => { + resetStartupLedgerCheck(); + await rm(directory, { recursive: true, force: true }); + }); + + test('supersession after integrity read prevents quarantine publication', async () => { + const { ledgerPath, planJsonPath } = await seedCorruptLedger(directory); + const ledgerBefore = fs.readFileSync(ledgerPath, 'utf8'); + const planBefore = fs.readFileSync(planJsonPath, 'utf8'); + let checkCount = 0; + const preCommitCheck = () => { + checkCount++; + // loadPlan checks at entry and before claiming startup authority. The + // third check is the fence immediately after the async integrity read. + if (checkCount === 3) { + throw new PlanRecoverySupersededError( + 'hydration generation superseded', + ); + } + }; + + await expect( + loadPlan(directory, undefined, { preCommitCheck }), + ).rejects.toBeInstanceOf(PlanRecoverySupersededError); + + expect(checkCount).toBe(3); + expect( + fs + .readdirSync(path.join(directory, '.swarm')) + .filter((name) => name.startsWith('plan-ledger.quarantine.')), + ).toEqual([]); + // The superseded recovery must not mutate or remove either authoritative + // input while refusing the quarantine publication. + expect(fs.readFileSync(ledgerPath, 'utf8')).toBe(ledgerBefore); + expect(fs.readFileSync(planJsonPath, 'utf8')).toBe(planBefore); + }); + + test('quarantine propagates typed supersession before its side-file write', async () => { + const superseded = new PlanRecoverySupersededError('newer authority won'); + await expect( + quarantineLedgerSuffix(directory, '{ POISON\n', { + preCommitCheck: () => { + throw superseded; + }, + }), + ).rejects.toBe(superseded); + + expect( + fs + .readdirSync(path.join(directory, '.swarm')) + .filter((name) => name.startsWith('plan-ledger.quarantine.')), + ).toEqual([]); + }); +}); diff --git a/tests/unit/plan/manager-recovery-2531.test.ts b/tests/unit/plan/manager-recovery-2531.test.ts index 78b651332..afae40cc7 100644 --- a/tests/unit/plan/manager-recovery-2531.test.ts +++ b/tests/unit/plan/manager-recovery-2531.test.ts @@ -10,6 +10,7 @@ import { type LedgerEventInput, takeSnapshotEvent, } from '../../../src/plan/ledger'; +import { getPlanLedgerState } from '../../../src/plan/ledger-sqlite'; import { derivePlanMarkdown, loadPlan, @@ -207,6 +208,69 @@ describe('loadPlan recovery ladder (#2531)', () => { expect(loaded).toBeNull(); }); + describe('syntactically malformed plan.json recovery coverage (F-005)', () => { + test('recovers from a malformed projection using a complete embedded ledger', async () => { + directory = await freshDir('malformed-json-complete-ledger'); + const authoritative = makeRichPlan('Complete ledger authority'); + await initLedger( + directory, + derivePlanId(authoritative), + computePlanLedgerHash(authoritative), + authoritative, + ); + await writeFile( + join(directory, '.swarm', 'plan.json'), + '{not valid JSON', + ); + + const loaded = await loadPlan(directory); + + expect(loaded?.title).toBe('Complete ledger authority'); + assertRichMetadata(loaded); + }); + + test('does not replay a verified prefix from a truncated ledger when projection identity is malformed', async () => { + directory = await freshDir('malformed-json-truncated-ledger'); + const prefixPlan = makeRichPlan('Truncated ledger prefix'); + await initLedger( + directory, + derivePlanId(prefixPlan), + computePlanLedgerHash(prefixPlan), + prefixPlan, + ); + const ledgerPath = join(directory, '.swarm', 'plan-ledger.jsonl'); + const rootLine = readFileSync(ledgerPath, 'utf8'); + await writeFile( + ledgerPath, + `${rootLine}{ poison: unverified ledger suffix\n`, + ); + await writeFile( + join(directory, '.swarm', 'plan.json'), + '{not valid JSON', + ); + const markdownFallback = makeRichPlan('Markdown fallback authority'); + await writeFile( + join(directory, '.swarm', 'plan.md'), + derivePlanMarkdown(markdownFallback), + ); + expect(getPlanLedgerState(directory)?.authorityMode).toBe('file_shadow'); + + const loaded = await loadPlan(directory); + + // Without parseable projection identity, the truncated ledger cannot + // authorize replay of its verified prefix; recovery stays on the legacy + // markdown rung instead of silently choosing the prefix plan. + expect(loaded?.title).toBe('Markdown fallback authority'); + expect(loaded?.title).not.toBe('Truncated ledger prefix'); + expect(getPlanLedgerState(directory)?.authorityMode).toBe('file_shadow'); + const rewrittenProjection = JSON.parse( + readFileSync(join(directory, '.swarm', 'plan.json'), 'utf8'), + ) as Plan; + expect(rewrittenProjection.title).toBe('Markdown fallback authority'); + expect(rewrittenProjection.title).not.toBe('Truncated ledger prefix'); + }); + }); + test('degraded latest snapshot falls back to recoverable older history', async () => { directory = await freshDir('degraded-snapshot'); const older = makeRichPlan('Recoverable older snapshot'); diff --git a/tests/unit/plan/manager-recovery-replay-supersession-2668.test.ts b/tests/unit/plan/manager-recovery-replay-supersession-2668.test.ts new file mode 100644 index 000000000..2d4f14a7a --- /dev/null +++ b/tests/unit/plan/manager-recovery-replay-supersession-2668.test.ts @@ -0,0 +1,298 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import { mkdir, readdir, readFile, rm, writeFile } from 'node:fs/promises'; +import * as path from 'node:path'; +import type { Plan } from '../../../src/config/plan-schema'; +import type { LedgerEvent } from '../../../src/plan/ledger'; +import { + loadPlan, + PlanRecoverySupersededError, + rebuildPlan, + regeneratePlanMarkdown, + resetStartupLedgerCheck, + savePlan, +} from '../../../src/plan/manager'; +import { derivePlanId } from '../../../src/plan/utils'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +function makePlan( + status: Plan['phases'][0]['tasks'][0]['status'] = 'pending', +): Plan { + return { + schema_version: '1.0.0', + title: 'Recovery replay supersession', + swarm: 'recovery-replay-supersession-2668', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: '1.1', + phase: 1, + status, + size: 'small', + description: 'Preserve canonical state', + depends: [], + files_touched: [], + }, + ], + }, + ], + }; +} + +function eventLine(event: LedgerEvent): string { + return `${JSON.stringify(event)}\n`; +} + +async function seedCorruptLedger( + directory: string, + projection: 'invalid' | 'missing' | 'valid', +): Promise<{ ledgerPath: string; planPath: string; plan: Plan }> { + const authoritative = makePlan('completed'); + const prefixPlan = makePlan('pending'); + const planId = derivePlanId(authoritative); + const swarmDir = path.join(directory, '.swarm'); + const planPath = path.join(swarmDir, 'plan.json'); + const ledgerPath = path.join(swarmDir, 'plan-ledger.jsonl'); + + if (projection === 'valid') { + await writeFile(planPath, JSON.stringify(authoritative, null, 2), 'utf8'); + await regeneratePlanMarkdown(directory, authoritative); + } else if (projection === 'invalid') { + await writeFile( + planPath, + JSON.stringify({ ...authoritative, phases: 'not-an-array' }, null, 2), + 'utf8', + ); + } + + const created: LedgerEvent = { + seq: 1, + timestamp: '2026-01-01T00:00:00.000Z', + plan_id: planId, + event_type: 'plan_created', + source: 'test', + plan_hash_before: '', + plan_hash_after: 'H1', + schema_version: '1.1.0', + payload: { plan: prefixPlan, payload_hash: 'H1' }, + }; + const progress: LedgerEvent = { + seq: 2, + timestamp: '2026-01-01T00:00:01.000Z', + plan_id: planId, + event_type: 'task_status_changed', + task_id: '1.1', + phase_id: 1, + from_status: 'pending', + to_status: 'in_progress', + source: 'test', + plan_hash_before: 'H1', + plan_hash_after: 'H2', + schema_version: '1.1.0', + }; + await writeFile( + ledgerPath, + eventLine(created) + + eventLine(progress) + + '{ poison: recovery must not publish a quarantine file\n' + + 'after-poison bytes must remain canonical\n', + 'utf8', + ); + return { ledgerPath, planPath, plan: authoritative }; +} + +async function expectSuperseded( + operation: () => Promise, +): Promise { + await expect(operation()).rejects.toBeInstanceOf(PlanRecoverySupersededError); +} + +function quarantineNames(directory: string): string[] { + return fs + .readdirSync(path.join(directory, '.swarm')) + .filter((name) => name.startsWith('plan-ledger.quarantine.')); +} + +describe('manager recovery replay authority fences (#2668)', () => { + let directory = ''; + + beforeEach(async () => { + directory = canonicalMkdtemp('manager-replay-supersession-2668-'); + await mkdir(path.join(directory, '.swarm'), { recursive: true }); + await mkdir(path.join(directory, '.git')); + resetStartupLedgerCheck(); + }); + + afterEach(async () => { + resetStartupLedgerCheck(); + await rm(directory, { recursive: true, force: true }); + }); + + test('schema-invalid projection does not quarantine after replay read supersession', async () => { + const { ledgerPath, planPath } = await seedCorruptLedger( + directory, + 'invalid', + ); + const ledgerBefore = await readFile(ledgerPath, 'utf8'); + const planBefore = await readFile(planPath, 'utf8'); + let checks = 0; + const preCommitCheck = () => { + checks++; + if (checks === 2) throw new PlanRecoverySupersededError('new authority'); + }; + + await expectSuperseded(() => + loadPlan(directory, undefined, { preCommitCheck }), + ); + expect(checks).toBe(2); + expect(quarantineNames(directory)).toEqual([]); + expect(await readFile(ledgerPath, 'utf8')).toBe(ledgerBefore); + expect(await readFile(planPath, 'utf8')).toBe(planBefore); + }); + + test('missing projection does not quarantine after replay read supersession', async () => { + const { ledgerPath } = await seedCorruptLedger(directory, 'missing'); + const ledgerBefore = await readFile(ledgerPath, 'utf8'); + let checks = 0; + const preCommitCheck = () => { + checks++; + if (checks === 2) throw new PlanRecoverySupersededError('new authority'); + }; + + await expectSuperseded(() => + loadPlan(directory, undefined, { preCommitCheck }), + ); + expect(checks).toBe(2); + expect(quarantineNames(directory)).toEqual([]); + expect(await readFile(ledgerPath, 'utf8')).toBe(ledgerBefore); + expect(fs.existsSync(path.join(directory, '.swarm', 'plan.json'))).toBe( + false, + ); + }); + + test('savePlan pre-projection replay propagates supersession without mutation', async () => { + const { ledgerPath, planPath, plan } = await seedCorruptLedger( + directory, + 'valid', + ); + const ledgerBefore = await readFile(ledgerPath, 'utf8'); + const planBefore = await readFile(planPath, 'utf8'); + const preCommitCheck = () => { + throw new PlanRecoverySupersededError('new authority'); + }; + + await expectSuperseded(() => savePlan(directory, plan, { preCommitCheck })); + expect(quarantineNames(directory)).toEqual([]); + expect(await readFile(ledgerPath, 'utf8')).toBe(ledgerBefore); + expect(await readFile(planPath, 'utf8')).toBe(planBefore); + }); + + test('rebuildPlan self-replay propagates supersession without quarantine', async () => { + const { ledgerPath } = await seedCorruptLedger(directory, 'missing'); + const ledgerBefore = await readFile(ledgerPath, 'utf8'); + const preCommitCheck = () => { + throw new PlanRecoverySupersededError('new authority'); + }; + + await expectSuperseded(() => + rebuildPlan(directory, undefined, { preCommitCheck }), + ); + expect(quarantineNames(directory)).toEqual([]); + expect(await readFile(ledgerPath, 'utf8')).toBe(ledgerBefore); + expect(fs.existsSync(path.join(directory, '.swarm', 'plan.json'))).toBe( + false, + ); + }); + + describe('rebuildPlan markdown temp cleanup (F-003)', () => { + test('removes the staged markdown temp when superseded before rename', async () => { + const swarmDir = path.join(directory, '.swarm'); + let sawStagedMarkdown = false; + const preCommitCheck = () => { + const names = fs.readdirSync(swarmDir); + if (names.some((name) => name.startsWith('plan.md.rebuild.'))) { + // Before the cleanup fix, this fence rejected publication while + // leaving the already-written plan.md.rebuild.* file behind. + sawStagedMarkdown = true; + throw new PlanRecoverySupersededError('new authority'); + } + }; + + await expectSuperseded(() => + rebuildPlan(directory, makePlan(), { preCommitCheck }), + ); + + expect(sawStagedMarkdown).toBe(true); + expect( + fs + .readdirSync(swarmDir) + .filter((name) => name.startsWith('plan.md.rebuild.')), + ).toEqual([]); + }); + + test('removes the staged JSON temp when superseded before rename (R1)', async () => { + const swarmDir = path.join(directory, '.swarm'); + const planPath = path.join(swarmDir, 'plan.json'); + let sawStagedPlan = false; + const preCommitCheck = () => { + const names = fs.readdirSync(swarmDir); + if (names.some((name) => name.startsWith('plan.json.rebuild.'))) { + // Before the cleanup fix, this authority fence rejected the rename + // but left the fsynced plan.json.rebuild.* file in .swarm/. + sawStagedPlan = true; + throw new PlanRecoverySupersededError('new authority'); + } + }; + + await expectSuperseded(() => + rebuildPlan(directory, makePlan(), { preCommitCheck }), + ); + + expect(sawStagedPlan).toBe(true); + expect(fs.existsSync(planPath)).toBe(false); + expect( + fs + .readdirSync(swarmDir) + .filter((name) => name.startsWith('plan.json.rebuild.')), + ).toEqual([]); + }); + }); + + test('spec-staleness temp preparation cannot publish after supersession', async () => { + const plan = { ...makePlan(), specHash: 'stale-hash' }; + const planPath = path.join(directory, '.swarm', 'plan.json'); + await writeFile(planPath, JSON.stringify(plan, null, 2), 'utf8'); + await regeneratePlanMarkdown(directory, plan); + await writeFile( + path.join(directory, '.swarm', 'spec.md'), + 'changed spec\n', + 'utf8', + ); + const planBefore = await readFile(planPath, 'utf8'); + let checks = 0; + const preCommitCheck = () => { + checks++; + if (checks === 2) throw new PlanRecoverySupersededError('new authority'); + }; + + await expectSuperseded(() => + loadPlan(directory, undefined, { preCommitCheck }), + ); + expect(checks).toBe(2); + expect(await readFile(planPath, 'utf8')).toBe(planBefore); + const names = await readdir(path.join(directory, '.swarm')); + expect( + names.some((name) => + name.startsWith('spec-staleness.json.spec-staleness.'), + ), + ).toBe(false); + expect( + fs.existsSync(path.join(directory, '.swarm', 'spec-staleness.json')), + ).toBe(false); + }); +}); diff --git a/tests/unit/plan/pr-review-fixes.test.ts b/tests/unit/plan/pr-review-fixes.test.ts index 602670a33..3cb12ed39 100644 --- a/tests/unit/plan/pr-review-fixes.test.ts +++ b/tests/unit/plan/pr-review-fixes.test.ts @@ -152,149 +152,6 @@ function makeLedgerMock( }; } -// --------------------------------------------------------------------------- -// F-004: rebuildPlan marker reset on plan.md failure -// --------------------------------------------------------------------------- - -describe('rebuildPlan — F-004 marker reset on failure', () => { - let tempDir: string; - - beforeEach(async () => { - tempDir = await mkdtemp(join(tmpdir(), 'rebuild-plan-f004-')); - mkdirSync(join(tempDir, '.swarm'), { recursive: true }); - }); - - afterEach(async () => { - if (existsSync(tempDir)) { - rmSync(tempDir, { recursive: true, force: true }); - } - mock.restore(); - }); - - /** - * F-004 verification: rebuildPlan writes in_progress marker after plan.json rename, - * then plan.md write, then always resets marker to in_progress: false in finally. - * This test simulates plan.md bunWrite throwing and verifies the marker is still - * reset to in_progress: false (not left at true). - * - * NOTE: rebuildPlan has no catch block — only try/finally. - * So when bunWrite throws, the error propagates up and rebuildPlan rejects. - * The finally block runs before the rejection propagates, resetting the marker. - */ - test('F-004: plan.md write failure — marker still reset to in_progress: false in finally', async () => { - mock.module('../../../src/hooks/utils', () => ({ - ...realHookUtils, - readSwarmFileAsync: mock(async () => null), - validateSwarmPath: (p: string) => p, - safeHook: (name: string) => null as any, - })); - - const writeLog: Array<{ path: string; content: string }> = []; - let planMdWriteAttempted = false; - - const bunWriteMock = mock( - async (p: string, content: string | Uint8Array) => { - writeLog.push({ path: p, content: String(content) }); - if (p.includes('plan.md.rebuild.')) { - planMdWriteAttempted = true; - // Throw AFTER the await so try/catch can process it - throw new Error('disk full during plan.md write'); - } - }, - ); - - mock.module('../../../src/utils/bun-compat', () => ({ - ...makeBunCompatMock(), - bunWrite: bunWriteMock, - })); - - mock.module('../../../src/plan/ledger', () => makeLedgerMock()); - mock.module('node:fs', () => ({ - ...realFs, - renameSync: mock(() => {}), - unlinkSync: mock(() => {}), - existsSync: mock(() => true), - readdirSync: () => [], - })); - - const { rebuildPlan } = await import('../../../src/plan/manager'); - - const plan = createTestPlan(); - - // Expect rebuildPlan to reject (no catch block in rebuildPlan) - await expect( - rebuildPlan(tempDir, plan, { reason: 'test-f004' }), - ).rejects.toBeDefined(); - - expect(planMdWriteAttempted).toBe(true); - - // Verify the marker was reset to in_progress: false even though plan.md threw - const markerWrites = writeLog.filter( - (w) => - typeof w.path === 'string' && w.path.includes('.plan-write-marker'), - ); - expect(markerWrites.length).toBe(2); - - const inProgressMarker = JSON.parse(markerWrites[0].content); - expect(inProgressMarker.in_progress).toBe(true); - - const finalMarker = JSON.parse(markerWrites[1].content); - expect(finalMarker.in_progress).toBe(false); - }); - - /** - * F-004 happy path: rebuildPlan writes markers in correct sequence with correct values. - */ - test('F-004: rebuildPlan success — markers written in correct sequence', async () => { - mock.module('../../../src/hooks/utils', () => ({ - ...realHookUtils, - readSwarmFileAsync: mock(async () => null), - validateSwarmPath: (p: string) => p, - safeHook: (name: string) => null as any, - })); - - const writeLog: Array<{ path: string; content: string }> = []; - const bunWriteMock = mock( - async (p: string, content: string | Uint8Array) => { - writeLog.push({ path: p, content: String(content) }); - }, - ); - - mock.module('../../../src/utils/bun-compat', () => ({ - ...makeBunCompatMock(), - bunWrite: bunWriteMock, - })); - - mock.module('../../../src/plan/ledger', () => makeLedgerMock()); - mock.module('node:fs', () => ({ - ...realFs, - renameSync: mock(() => {}), - unlinkSync: mock(() => {}), - existsSync: mock(() => true), - readdirSync: () => [], - })); - - const { rebuildPlan } = await import('../../../src/plan/manager'); - - const plan = createTestPlan(); - await rebuildPlan(tempDir, plan, { reason: 'test-f004-happy' }); - - const markerWrites = writeLog.filter( - (w) => - typeof w.path === 'string' && w.path.includes('.plan-write-marker'), - ); - expect(markerWrites.length).toBe(2); - - const inProgressMarker = JSON.parse(markerWrites[0].content); - expect(inProgressMarker.in_progress).toBe(true); - expect(inProgressMarker.source).toBe('plan_manager'); - - const finalMarker = JSON.parse(markerWrites[1].content); - expect(finalMarker.in_progress).toBe(false); - expect(finalMarker.source).toBe('plan_manager'); - }); -}); - // --------------------------------------------------------------------------- // F-004: closePlanTerminalState marker reset on plan.md failure // --------------------------------------------------------------------------- diff --git a/tests/unit/plan/rebuild-plan-marker-supersession-2668.test.ts b/tests/unit/plan/rebuild-plan-marker-supersession-2668.test.ts new file mode 100644 index 000000000..7d6c5ada5 --- /dev/null +++ b/tests/unit/plan/rebuild-plan-marker-supersession-2668.test.ts @@ -0,0 +1,123 @@ +/** + * Issue #2668 recovery-marker supersession boundary. + * + * The marker is advisory, but it must not be cleared by an older recovery + * attempt after a newer writer has published its own marker. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import type { Plan } from '../../../src/config/plan-schema'; +import { closeAllProjectDbs } from '../../../src/db/project-db'; +import { readLedgerEvents } from '../../../src/plan/ledger'; +import { + _internals, + PlanRecoverySupersededError, + rebuildPlan, + savePlan, +} from '../../../src/plan/manager'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const temporaryDirectories: string[] = []; +const originalWriteRebuildPlanMarkdown = _internals.writeRebuildPlanMarkdown; + +function makePlan(): Plan { + return { + schema_version: '1.0.0', + title: 'Recovery marker test', + swarm: 'recovery-marker-test', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: '1.1', + phase: 1, + status: 'pending', + size: 'small', + description: 'Rebuild the recovery projection', + depends: [], + files_touched: [], + }, + ], + }, + ], + }; +} + +afterEach(() => { + _internals.writeRebuildPlanMarkdown = originalWriteRebuildPlanMarkdown; + closeAllProjectDbs(); + for (const directory of temporaryDirectories.splice(0)) { + safeRmRecursive(directory); + } +}); + +describe('rebuildPlan write-marker supersession regression (F2)', () => { + test('keeps committed plan.json and records rebuild when plan.md fails', async () => { + const directory = canonicalMkdtemp('rebuild-plan-markdown-failure-2668-'); + temporaryDirectories.push(directory); + mkdirSync(path.join(directory, '.opencode')); + const plan = makePlan(); + await savePlan(directory, plan); + _internals.writeRebuildPlanMarkdown = async () => { + throw new Error('simulated plan.md projection failure'); + }; + + await expect(rebuildPlan(directory, plan)).resolves.toEqual(plan); + + expect( + JSON.parse( + readFileSync(path.join(directory, '.swarm', 'plan.json'), 'utf8'), + ), + ).toMatchObject({ title: plan.title, swarm: plan.swarm }); + expect( + (await readLedgerEvents(directory)).some( + (event) => event.event_type === 'plan_rebuilt', + ), + ).toBe(true); + }); + + test('does not clear a newer writer marker from the cleanup path', async () => { + const directory = canonicalMkdtemp('rebuild-plan-marker-2668-'); + temporaryDirectories.push(directory); + mkdirSync(path.join(directory, '.opencode')); + const markerPath = path.join(directory, '.swarm', '.plan-write-marker'); + const newerMarker = JSON.stringify({ + source: 'newer_generation', + timestamp: '2026-09-14T00:00:00.000Z', + in_progress: true, + }); + let preCommitChecks = 0; + let superseded = false; + + // Before the fix, finally always wrote in_progress:false after the + // markdown commit check threw, overwriting this newer writer marker. + await expect( + rebuildPlan(directory, makePlan(), { + preCommitCheck: () => { + preCommitChecks += 1; + if (preCommitChecks === 4) { + superseded = true; + writeFileSync(markerPath, newerMarker, 'utf8'); + } + if (superseded) { + throw new PlanRecoverySupersededError( + 'rebuild superseded during markdown publication', + ); + } + }, + }), + ).rejects.toBeInstanceOf(PlanRecoverySupersededError); + + expect(preCommitChecks).toBe(5); + expect(JSON.parse(readFileSync(markerPath, 'utf8'))).toMatchObject({ + source: 'newer_generation', + in_progress: true, + }); + }); +}); diff --git a/tests/unit/plan/rebuild-plan-projection-failure.test.ts b/tests/unit/plan/rebuild-plan-projection-failure.test.ts new file mode 100644 index 000000000..7e2d5a13e --- /dev/null +++ b/tests/unit/plan/rebuild-plan-projection-failure.test.ts @@ -0,0 +1,262 @@ +/** + * F-004 rebuildPlan write-marker tests. + * + * A failed plan.md projection is advisory because plan.json is authoritative. + * Failure while clearing the advisory marker must not mask the original + * Markdown warning or turn a successful recovery into a failed rebuild. + */ +import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'; +import * as realFs from 'node:fs'; +import { mkdirSync } from 'node:fs'; +import { join } from 'node:path'; +import type { Plan } from '../../../src/config/plan-schema'; +import * as realHookUtils from '../../../src/hooks/utils'; +import * as realLedger from '../../../src/plan/ledger'; +import * as realUtils from '../../../src/utils'; +import { createSafeTestDir } from '../../helpers/safe-test-dir'; + +function createTestPlan(): Plan { + return { + schema_version: '1.0.0', + title: 'Test Plan', + swarm: 'test-swarm', + current_phase: 1, + phases: [ + { + id: 1, + name: 'Phase 1', + status: 'in_progress', + tasks: [ + { + id: '1.1', + phase: 1, + status: 'in_progress', + size: 'small', + description: 'Task one', + depends: [], + files_touched: [], + }, + { + id: '1.2', + phase: 1, + status: 'pending', + size: 'medium', + description: 'Task two', + depends: [], + files_touched: [], + }, + ], + }, + { + id: 2, + name: 'Phase 2', + status: 'pending', + tasks: [ + { + id: '2.1', + phase: 2, + status: 'pending', + size: 'small', + description: 'Task three', + depends: [], + files_touched: [], + }, + ], + }, + ], + }; +} + +function makeBunCompatMock() { + return { + bunWrite: mock(async (_path: string, _data: string | Uint8Array) => {}), + bunHash: mock(() => 0n), + bunFile: (_path: string) => ({ + text: async () => '', + exists: async () => false, + arrayBuffer: async () => new ArrayBuffer(0), + size: 0, + }), + isBun: () => false, + bunSpawn: () => ({ + stdout: { + text: async () => '', + bytes: async () => new Uint8Array(0), + getReader: () => ({ + read: async () => ({ done: true, value: undefined }), + }), + }, + stderr: { + text: async () => '', + bytes: async () => new Uint8Array(0), + getReader: () => ({ + read: async () => ({ done: true, value: undefined }), + }), + }, + exited: Promise.resolve(0), + exitCode: null as number | null, + kill: () => {}, + }), + bunSpawnSync: () => ({ + stdout: new Uint8Array(), + stderr: new Uint8Array(), + exitCode: 0, + success: true, + }), + }; +} + +function makeLedgerMock() { + return { + ...realLedger, + appendLedgerEvent: mock(async () => ({})), + takeSnapshotEvent: mock(async () => ({})), + ledgerExists: mock(async () => false), + initLedger: mock(async () => {}), + readLedgerEvents: mock(async () => []), + computePlanLedgerHash: mock(() => 'hash'), + computeCurrentPlanHash: mock(() => 'hash'), + getLatestLedgerSeq: mock(async () => 0), + }; +} + +describe('rebuildPlan — F-004 marker reset on failure', () => { + let tempDir: string; + let cleanupTempDir: () => void; + + beforeEach(() => { + const safeDir = createSafeTestDir('rebuild-plan-f004-'); + tempDir = safeDir.dir; + cleanupTempDir = safeDir.cleanup; + mkdirSync(join(tempDir, '.swarm'), { recursive: true }); + }); + + afterEach(() => { + cleanupTempDir(); + mock.restore(); + }); + + /** + * Before the fix, a final-marker write throw escaped from `finally` before the + * stored plan.md projection error could be handled, masking its warning and + * rejecting rebuildPlan after plan.json had already committed. + */ + test('F-004: projection and final-marker failures remain advisory', async () => { + mock.module('../../../src/hooks/utils', () => ({ + ...realHookUtils, + readSwarmFileAsync: mock(async () => null), + validateSwarmPath: (path: string) => path, + safeHook: (name: string) => null as any, + })); + + const writeLog: Array<{ path: string; content: string }> = []; + let planMdWriteAttempted = false; + let markerWriteAttempts = 0; + let finalMarkerWriteFailed = false; + const warnings: string[] = []; + const warnMock = mock((message: string) => warnings.push(message)); + mock.module('../../../src/utils', () => ({ ...realUtils, warn: warnMock })); + + const bunWriteMock = mock( + async (path: string, content: string | Uint8Array) => { + writeLog.push({ path, content: String(content) }); + if (path.includes('plan.md.rebuild.')) { + planMdWriteAttempted = true; + // Throw after the write await so rebuildPlan's projection catch handles it. + throw new Error('disk full during plan.md write'); + } + if ( + path.includes('.plan-write-marker.rebuild.') && + ++markerWriteAttempts === 2 + ) { + finalMarkerWriteFailed = true; + throw new Error('disk full during final marker write'); + } + }, + ); + + mock.module('../../../src/utils/bun-compat', () => ({ + ...makeBunCompatMock(), + bunWrite: bunWriteMock, + })); + mock.module('../../../src/plan/ledger', () => makeLedgerMock()); + mock.module('node:fs', () => ({ + ...realFs, + renameSync: mock(() => {}), + unlinkSync: mock(() => {}), + existsSync: mock(() => true), + readdirSync: () => [], + })); + + const { rebuildPlan } = await import('../../../src/plan/manager'); + const plan = createTestPlan(); + const rebuilt = await rebuildPlan(tempDir, plan, { reason: 'test-f004' }); + + expect(rebuilt).toBe(plan); + expect(planMdWriteAttempted).toBe(true); + expect(finalMarkerWriteFailed).toBe(true); + expect( + warnings.some((message) => + message.includes('disk full during plan.md write'), + ), + ).toBe(true); + expect( + warnings.some((message) => + message.includes('disk full during final marker write'), + ), + ).toBe(false); + + const markerWrites = writeLog.filter((write) => + write.path.includes('.plan-write-marker'), + ); + expect(markerWrites.length).toBe(2); + expect(JSON.parse(markerWrites[0].content).in_progress).toBe(true); + expect(JSON.parse(markerWrites[1].content).in_progress).toBe(false); + }); + + test('F-004: rebuildPlan success — markers written in correct sequence', async () => { + mock.module('../../../src/hooks/utils', () => ({ + ...realHookUtils, + readSwarmFileAsync: mock(async () => null), + validateSwarmPath: (path: string) => path, + safeHook: (name: string) => null as any, + })); + + const writeLog: Array<{ path: string; content: string }> = []; + const bunWriteMock = mock( + async (path: string, content: string | Uint8Array) => { + writeLog.push({ path, content: String(content) }); + }, + ); + + mock.module('../../../src/utils/bun-compat', () => ({ + ...makeBunCompatMock(), + bunWrite: bunWriteMock, + })); + mock.module('../../../src/plan/ledger', () => makeLedgerMock()); + mock.module('node:fs', () => ({ + ...realFs, + renameSync: mock(() => {}), + unlinkSync: mock(() => {}), + existsSync: mock(() => true), + readdirSync: () => [], + })); + + const { rebuildPlan } = await import('../../../src/plan/manager'); + const plan = createTestPlan(); + await rebuildPlan(tempDir, plan, { reason: 'test-f004-happy' }); + + const markerWrites = writeLog.filter((write) => + write.path.includes('.plan-write-marker'), + ); + expect(markerWrites.length).toBe(2); + expect(JSON.parse(markerWrites[0].content)).toMatchObject({ + in_progress: true, + source: 'plan_manager', + }); + expect(JSON.parse(markerWrites[1].content)).toMatchObject({ + in_progress: false, + source: 'plan_manager', + }); + }); +}); diff --git a/tests/unit/plan/write-marker-in-progress-manager.test.ts b/tests/unit/plan/write-marker-in-progress-manager.test.ts index fc7b6c718..b88d8d7e6 100644 --- a/tests/unit/plan/write-marker-in-progress-manager.test.ts +++ b/tests/unit/plan/write-marker-in-progress-manager.test.ts @@ -8,6 +8,7 @@ import type { Plan } from '../../../src/config/plan-schema'; import { _internals, closePlanTerminalState, + PlanRecoverySupersededError, rebuildPlan, savePlan, } from '../../../src/plan/manager'; @@ -200,6 +201,111 @@ describe('savePlan write-marker in_progress', () => { expect(lastMarker.phases_count).toBe(2); expect(lastMarker.tasks_count).toBe(3); }); + + test('authority loss after in-progress marker preparation prevents its rename', async () => { + const writes: Array<{ path: string; content: string }> = []; + const renames: Array<{ from: string; to: string }> = []; + _internals.verifyWrittenPlanJson = async () => {}; + + mock.module('../../../src/utils/bun-compat', () => ({ + bunWrite: mock(async (path: string, content: string) => { + writes.push({ path, content }); + }), + bunHash: mock(() => 0n), + })); + mock.module('node:fs', () => ({ + ...realFs, + renameSync: mock((from: string, to: string) => { + renames.push({ from, to }); + }), + readdirSync: () => [], + })); + mock.module('../../../src/plan/ledger', () => ({ + ledgerExists: mock(async () => false), + initLedger: mock(async () => {}), + appendLedgerEvent: mock(async () => ({})), + computePlanLedgerHash: mock(() => 'hash'), + computeCurrentPlanHash: mock(() => 'hash'), + readLedgerEvents: mock(async () => []), + getLatestLedgerSeq: mock(async () => 0), + takeSnapshotEvent: mock(async () => {}), + })); + + const preCommitCheck = () => { + if ( + writes.some((call) => + call.path.includes('.plan-write-marker.plan-write-marker.'), + ) + ) { + throw new PlanRecoverySupersededError('new authority'); + } + }; + await expect( + savePlan(tempDir, createTestPlan(), { preCommitCheck }), + ).rejects.toBeInstanceOf(PlanRecoverySupersededError); + expect( + writes.some((call) => + call.path.includes('.plan-write-marker.plan-write-marker.'), + ), + ).toBe(true); + expect( + renames.some((rename) => + rename.from.includes('.plan-write-marker.plan-write-marker.'), + ), + ).toBe(false); + }); + + test('authority loss after final marker preparation preserves the in-progress publication', async () => { + const writes: Array<{ path: string; content: string }> = []; + const renames: Array<{ from: string; to: string }> = []; + _internals.verifyWrittenPlanJson = async () => {}; + + mock.module('../../../src/utils/bun-compat', () => ({ + bunWrite: mock(async (path: string, content: string) => { + writes.push({ path, content }); + }), + bunHash: mock(() => 0n), + })); + mock.module('node:fs', () => ({ + ...realFs, + renameSync: mock((from: string, to: string) => { + renames.push({ from, to }); + }), + readdirSync: () => [], + })); + mock.module('../../../src/plan/ledger', () => ({ + ledgerExists: mock(async () => false), + initLedger: mock(async () => {}), + appendLedgerEvent: mock(async () => ({})), + computePlanLedgerHash: mock(() => 'hash'), + computeCurrentPlanHash: mock(() => 'hash'), + readLedgerEvents: mock(async () => []), + getLatestLedgerSeq: mock(async () => 0), + takeSnapshotEvent: mock(async () => {}), + })); + + let markerFenceCount = 0; + const preCommitCheck = () => { + const markerWrites = writes.filter((call) => + call.path.includes('.plan-write-marker.plan-write-marker.'), + ); + if (markerWrites.length > markerFenceCount) { + markerFenceCount = markerWrites.length; + if (markerFenceCount === 2) { + throw new PlanRecoverySupersededError('new authority'); + } + } + }; + await expect( + savePlan(tempDir, createTestPlan(), { preCommitCheck }), + ).rejects.toBeInstanceOf(PlanRecoverySupersededError); + expect(markerFenceCount).toBe(2); + expect( + renames.filter((rename) => + rename.from.includes('.plan-write-marker.plan-write-marker.'), + ), + ).toHaveLength(1); + }); }); // --------------------------------------------------------------------------- diff --git a/tests/unit/session/hydration-authority-aba-2668.test.ts b/tests/unit/session/hydration-authority-aba-2668.test.ts new file mode 100644 index 000000000..295dbce97 --- /dev/null +++ b/tests/unit/session/hydration-authority-aba-2668.test.ts @@ -0,0 +1,377 @@ +/** + * Issue #2668 — hydration authority ABA regression coverage. + * + * Numeric per-project generations are intentionally reusable after FIFO + * eviction and reset. The process-monotonic authority epoch must still make + * every old scope/token permanently stale. + */ + +import { afterAll, beforeEach, describe, expect, test } from 'bun:test'; +import { rmSync } from 'node:fs'; +import { + beginHydrationScope, + captureCurrentHydrationAuthority, + clearHydrationOwnershipState, + currentHydrationGeneration, + hydrationProjectKey, + isHydrationAuthorityCurrent, + isHydrationScopeCurrent, + MAX_TRACKED_PROJECTS, +} from '../../../src/session/hydration-ownership'; +import { rehydrateState } from '../../../src/session/snapshot-reader'; +import type { SnapshotData } from '../../../src/session/snapshot-writer'; +import { + buildRehydrationCache, + ensureAgentSession, + rehydrateSessionFromDisk, + resetSwarmState, + startAgentSession, + _internals as stateInternals, + swarmState, +} from '../../../src/state'; +import { writeApprovedPlan } from '../../helpers/approved-plan'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const tempDirs: string[] = []; + +function makeProject(prefix: string): string { + const directory = canonicalMkdtemp(`${prefix}-2668-`); + tempDirs.push(directory); + return directory; +} + +function makeSnapshot(sessionId: string): SnapshotData { + return { + version: 3, + writtenAt: 1, + toolAggregates: { + stale: { + tool: 'stale', + count: 1, + successCount: 1, + failureCount: 0, + totalDuration: 1, + }, + }, + activeAgent: { [sessionId]: 'coder' }, + delegationChains: {}, + agentSessions: { + [sessionId]: { + agentName: 'coder', + lastToolCallTime: 1, + lastAgentEventTime: 1, + delegationActive: false, + }, + }, + } as unknown as SnapshotData; +} + +function deferred(): { promise: Promise; release: () => void } { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +function floodProjectAuthorities(): void { + for (let index = 0; index < MAX_TRACKED_PROJECTS; index += 1) { + beginHydrationScope(makeProject(`aba-flood-${index}`)); + } +} + +async function settlePendingRehydrations(): Promise { + await Promise.allSettled([...swarmState.pendingRehydrations]); +} + +beforeEach(() => { + resetSwarmState(); +}); + +afterAll(() => { + for (const directory of tempDirs) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +describe('process-monotonic hydration authority', () => { + test('stale scope stays false after FIFO eviction and same-generation reinsertion', async () => { + const directory = makeProject('aba-scope'); + const oldScope = beginHydrationScope(directory); + const gate = deferred(); + swarmState.pendingRehydrations.add(gate.promise); + + const pending = rehydrateState( + makeSnapshot('must-not-restore'), + directory, + oldScope, + ); + floodProjectAuthorities(); + const newScope = beginHydrationScope(directory); + + expect(newScope.generation).toBe(oldScope.generation); + expect(newScope.authorityEpoch).not.toBe(oldScope.authorityEpoch); + expect(isHydrationScopeCurrent(oldScope)).toBe(false); + expect(isHydrationScopeCurrent(newScope)).toBe(true); + + gate.release(); + const outcome = await pending; + swarmState.pendingRehydrations.delete(gate.promise); + expect(outcome).toEqual({ applied: false, reason: 'superseded' }); + expect(swarmState.agentSessions.has('must-not-restore')).toBe(false); + expect(swarmState.toolAggregates.has('stale')).toBe(false); + }); + + test('clear and reset never let an old scope revive after reintroduction', async () => { + const directory = makeProject('aba-clear'); + const oldScope = beginHydrationScope(directory); + const gate = deferred(); + swarmState.pendingRehydrations.add(gate.promise); + + const pending = rehydrateState( + makeSnapshot('must-not-restore-reset'), + directory, + oldScope, + ); + clearHydrationOwnershipState(); + const newScope = beginHydrationScope(directory); + + expect(newScope.generation).toBe(oldScope.generation); + expect(newScope.authorityEpoch).not.toBe(oldScope.authorityEpoch); + expect(isHydrationScopeCurrent(oldScope)).toBe(false); + + gate.release(); + const outcome = await pending; + swarmState.pendingRehydrations.delete(gate.promise); + expect(outcome.applied).toBe(false); + expect(swarmState.agentSessions.has('must-not-restore-reset')).toBe(false); + }); + + test('implicit rehydrate captures the exact authority across its await', async () => { + const directory = makeProject('aba-implicit'); + beginHydrationScope(directory); + const gate = deferred(); + swarmState.pendingRehydrations.add(gate.promise); + + const pending = rehydrateState( + makeSnapshot('must-not-restore-implicit'), + directory, + ); + floodProjectAuthorities(); + const newAuthority = beginHydrationScope(directory); + + expect(currentHydrationGeneration(newAuthority.projectKey)).toBe(1); + expect(isHydrationAuthorityCurrent(newAuthority)).toBe(true); + + gate.release(); + const outcome = await pending; + swarmState.pendingRehydrations.delete(gate.promise); + expect(outcome.applied).toBe(false); + expect(swarmState.agentSessions.has('must-not-restore-implicit')).toBe( + false, + ); + }); + + test('direct session rehydration cannot regain authority after eviction/reinsertion', async () => { + const directory = makeProject('aba-direct'); + const originalRehydrate = stateInternals.rehydrateSessionFromDisk; + const originalSubscriptions = stateInternals.rehydratePrSubscriptions; + const gate = deferred(); + let shouldCommit: (() => boolean) | undefined; + + stateInternals.rehydrateSessionFromDisk = async ( + _directory, + session, + check, + ) => { + shouldCommit = check; + await gate.promise; + if (check?.()) session.currentTaskId = 'must-not-be-revived'; + }; + stateInternals.rehydratePrSubscriptions = async () => new Map(); + + try { + startAgentSession('direct-aba', 'coder', undefined, directory); + expect(shouldCommit).toBeDefined(); + const projectKey = hydrationProjectKey(directory); + expect(currentHydrationGeneration(projectKey)).toBe(0); + + floodProjectAuthorities(); + const replacement = captureCurrentHydrationAuthority(projectKey); + expect(currentHydrationGeneration(projectKey)).toBe(0); + expect(shouldCommit?.()).toBe(false); + expect(isHydrationAuthorityCurrent(replacement)).toBe(true); + + gate.release(); + await Promise.allSettled([...swarmState.pendingRehydrations]); + expect( + swarmState.agentSessions.get('direct-aba')?.currentTaskId, + ).toBeNull(); + } finally { + gate.release(); + stateInternals.rehydrateSessionFromDisk = originalRehydrate; + stateInternals.rehydratePrSubscriptions = originalSubscriptions; + } + }); + + test('rehydration evicts a session from an older authority despite its larger stamp', async () => { + const directory = makeProject('aba-session-stamp'); + // The live session is created under generation 1, so its recency stamp is + // 2. Eviction/reinsertion resets the visible generation to 1; numeric-only + // recency would incorrectly preserve this stale session. + const firstScope = beginHydrationScope(directory); + startAgentSession('stale-authority-session', 'coder', undefined, directory); + await settlePendingRehydrations(); + const stale = swarmState.agentSessions.get('stale-authority-session'); + expect(stale?.hydrationStamp).toBe(firstScope.generation + 1); + expect(stale?.hydrationAuthorityEpoch).toBe(firstScope.authorityEpoch); + + floodProjectAuthorities(); + const replacement = beginHydrationScope(directory); + expect(replacement.generation).toBe(firstScope.generation); + expect(replacement.authorityEpoch).not.toBe(firstScope.authorityEpoch); + + const outcome = await rehydrateState( + makeSnapshot('fresh-authority-session'), + directory, + replacement, + ); + expect(outcome).toEqual({ applied: true }); + expect(swarmState.agentSessions.has('stale-authority-session')).toBe(false); + expect(swarmState.agentSessions.has('fresh-authority-session')).toBe(true); + }); + + test('ownership reset makes old session stamps stale after authority reintroduction', async () => { + const directory = makeProject('aba-session-reset'); + const firstScope = beginHydrationScope(directory); + startAgentSession('reset-stale-session', 'coder', undefined, directory); + await settlePendingRehydrations(); + const stale = swarmState.agentSessions.get('reset-stale-session'); + expect(stale?.hydrationStamp).toBe(firstScope.generation + 1); + + // This reset intentionally leaves live sessions in place, modeling a + // registry reset that races with a still-running session rehydration. + clearHydrationOwnershipState(); + const replacement = beginHydrationScope(directory); + expect(replacement.generation).toBe(firstScope.generation); + expect(replacement.authorityEpoch).not.toBe(firstScope.authorityEpoch); + + const outcome = await rehydrateState( + makeSnapshot('fresh-after-reset'), + directory, + replacement, + ); + expect(outcome).toEqual({ applied: true }); + expect(swarmState.agentSessions.has('reset-stale-session')).toBe(false); + expect(swarmState.agentSessions.has('fresh-after-reset')).toBe(true); + }); + + test('startAgentSession rejects an evicted cache but applies the current epoch cache', async () => { + const directory = makeProject('aba-cache'); + await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/cache.ts'], status: 'completed' }, + ]); + await buildRehydrationCache(directory); + + floodProjectAuthorities(); + const projectKey = hydrationProjectKey(directory); + const replacement = captureCurrentHydrationAuthority(projectKey); + startAgentSession('stale-cache-session', 'coder', undefined, directory); + const staleSession = swarmState.agentSessions.get('stale-cache-session'); + expect(staleSession?.taskWorkflowStates.has('1.1')).toBe(false); + + await buildRehydrationCache(directory); + startAgentSession('current-cache-session', 'coder', undefined, directory); + const currentSession = swarmState.agentSessions.get( + 'current-cache-session', + ); + expect(currentSession?.taskWorkflowStates.get('1.1')).toBe('complete'); + expect(replacement.authorityEpoch).toBe( + currentSession?.hydrationAuthorityEpoch, + ); + await settlePendingRehydrations(); + }); + + test('a delayed cache build cannot publish after authority reintroduction', async () => { + const directory = makeProject('aba-delayed-cache'); + await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/cache.ts'], status: 'completed' }, + ]); + await buildRehydrationCache(directory); + + const delayedBuild = buildRehydrationCache(directory); + floodProjectAuthorities(); + const replacement = captureCurrentHydrationAuthority( + hydrationProjectKey(directory), + ); + const result = await delayedBuild; + + expect(result).toEqual({ committed: false, reason: 'superseded' }); + expect(replacement.authorityEpoch).not.toBe(0); + }); + + describe('rehydration cache committed-result contract (GAP-001)', () => { + test('public session rehydration does not apply a cache after authority supersedes its build', async () => { + const directory = makeProject('aba-cache-committed-result'); + const session = ensureAgentSession('uncommitted-cache-session', 'coder'); + const originalAuthority = beginHydrationScope(directory); + await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/cache.ts'], status: 'completed' }, + ]); + expect((await buildRehydrationCache(directory)).committed).toBe(true); + expect(session.taskWorkflowStates.has('1.1')).toBe(false); + + // The real builder captures the old authority before its async plan read. + // Evict that authority before the post-read publication check; the public + // rehydration path must honor committed:false instead of applying the old cache. + const originalBuild = stateInternals.buildRehydrationCache; + let buildResult: Awaited< + ReturnType + > | null = null; + stateInternals.buildRehydrationCache = async (root, options) => { + buildResult = await originalBuild(root, options); + return buildResult; + }; + try { + const pending = rehydrateSessionFromDisk( + directory, + session, + () => true, + ); + floodProjectAuthorities(); + const replacement = beginHydrationScope(directory); + expect(replacement.authorityEpoch).not.toBe( + originalAuthority.authorityEpoch, + ); + + await pending; + + expect(buildResult).toEqual({ + committed: false, + reason: 'superseded', + }); + expect(session.taskWorkflowStates.has('1.1')).toBe(false); + } finally { + stateInternals.buildRehydrationCache = originalBuild; + } + }); + }); + + test('aggregate ownership from an evicted epoch cannot delete new state', async () => { + const directory = makeProject('aba-aggregates'); + const first = beginHydrationScope(directory); + const initial = makeSnapshot('aggregate-old'); + const initialOutcome = await rehydrateState(initial, directory, first); + expect(initialOutcome).toEqual({ applied: true }); + swarmState.toolAggregates.get('stale')!.count = 99; + + floodProjectAuthorities(); + const replacement = beginHydrationScope(directory); + const next = makeSnapshot('aggregate-new'); + next.toolAggregates = {}; + const nextOutcome = await rehydrateState(next, directory, replacement); + + expect(nextOutcome).toEqual({ applied: true }); + expect(swarmState.toolAggregates.get('stale')?.count).toBe(99); + }); +}); diff --git a/tests/unit/session/hydration-ownership.test.ts b/tests/unit/session/hydration-ownership.test.ts index 0a04a6ddf..ace32c48f 100644 --- a/tests/unit/session/hydration-ownership.test.ts +++ b/tests/unit/session/hydration-ownership.test.ts @@ -473,7 +473,7 @@ describe('coordination-init generation fence (issue #2667 fault 3, deterministic // newer generation's state stands. expect(swarmState.agentSessions.has('sess-coord-old')).toBe(false); expect(swarmState.agentSessions.has('sess-late')).toBe(true); - expect(getSnapshotCoordinationStatus(dir).state).toBe('succeeded'); + expect(getSnapshotCoordinationStatus(dir).state).toBe('superseded'); // Recovery: retry (entry deletion + fresh generation) still // applies fresh state through the shared never-reset counter. diff --git a/tests/unit/session/restart-coordination-supersession-2668.test.ts b/tests/unit/session/restart-coordination-supersession-2668.test.ts new file mode 100644 index 000000000..9c25b89b8 --- /dev/null +++ b/tests/unit/session/restart-coordination-supersession-2668.test.ts @@ -0,0 +1,424 @@ +/** + * Issue #2668 coordination supersession boundaries. + * + * These tests hold the real post-resolution initializer at each asynchronous + * boundary and then begin a newer hydration generation. The assertions are + * on durable SQLite/projection state and readiness, not on copied helpers. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import type { Plan } from '../../../src/config/plan-schema'; +import { closeAllProjectDbs } from '../../../src/db/project-db'; +import { + loadPlan, + PlanRecoverySupersededError, + _internals as planManagerInternals, + resetStartupLedgerCheck, +} from '../../../src/plan/manager'; +import { + beginHydrationScope, + hydrationProjectKey, +} from '../../../src/session/hydration-ownership'; +import { + _snapshotCoordinationInternals, + ensureSnapshotCoordinationReady, + getSnapshotCoordinationStatus, + startSnapshotCoordinationInitialization, +} from '../../../src/session/snapshot-coordination-init'; +import { + readSnapshotRows, + writeSnapshotRows, +} from '../../../src/session/snapshot-store'; +import { + SNAPSHOT_PROJECTION_FILE, + type SnapshotData, + writeSnapshotProjection, +} from '../../../src/session/snapshot-writer'; +import { + buildRehydrationCache, + ensureAgentSession, + resetSwarmState, +} from '../../../src/state'; +import { invalidateCachedArtifact } from '../../../src/utils/swarm-artifact-cache'; +import { writeApprovedPlan } from '../../../tests/helpers/approved-plan'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { withFrozenClock } from '../../helpers/test-clock'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const originalLoadPlan = _snapshotCoordinationInternals.loadPlan; +const originalReadSnapshotFileStrict = + _snapshotCoordinationInternals.readSnapshotFileStrict; +const originalReadPlanJsonUtf8 = planManagerInternals.readPlanJsonUtf8; +const temporaryDirectories: string[] = []; + +function makeSnapshot(marker: string): SnapshotData { + return { + version: 3, + writtenAt: withFrozenClock(() => Date.now()), + toolAggregates: { [marker]: { count: 1 } }, + activeAgent: {}, + delegationChains: {}, + agentSessions: {}, + } as unknown as SnapshotData; +} + +function makeProject(label: string): string { + const directory = canonicalMkdtemp(`swarm-2668-coordination-${label}-`); + mkdirSync(path.join(directory, '.git')); + temporaryDirectories.push(directory); + return directory; +} + +async function waitFor(predicate: () => boolean): Promise { + for (let attempt = 0; attempt < 100; attempt += 1) { + if (predicate()) return; + await new Promise((resolve) => setImmediate(resolve)); + } + throw new Error('timed out waiting for coordination boundary'); +} + +beforeEach(() => { + _snapshotCoordinationInternals.entries.clear(); + _snapshotCoordinationInternals.loadPlan = originalLoadPlan; + _snapshotCoordinationInternals.readSnapshotFileStrict = + originalReadSnapshotFileStrict; + planManagerInternals.readPlanJsonUtf8 = originalReadPlanJsonUtf8; + resetStartupLedgerCheck(); + resetSwarmState(); +}); + +afterEach(() => { + _snapshotCoordinationInternals.entries.clear(); + _snapshotCoordinationInternals.loadPlan = originalLoadPlan; + _snapshotCoordinationInternals.readSnapshotFileStrict = + originalReadSnapshotFileStrict; + planManagerInternals.readPlanJsonUtf8 = originalReadPlanJsonUtf8; + resetStartupLedgerCheck(); + resetSwarmState(); + closeAllProjectDbs(); + for (const directory of temporaryDirectories.splice(0)) { + safeRmRecursive(directory); + } +}); + +describe('coordination supersession regression (#2668)', () => { + test('does not import a compatibility snapshot after strict read is superseded', async () => { + const directory = makeProject('compat-import'); + await writeSnapshotProjection(directory, makeSnapshot('stale-compat')); + + let releaseRead!: () => void; + let strictReadStarted = false; + const readBarrier = new Promise((resolve) => { + releaseRead = resolve; + }); + _snapshotCoordinationInternals.readSnapshotFileStrict = async ( + root, + relativePath, + ) => { + strictReadStarted = true; + await readBarrier; + return originalReadSnapshotFileStrict(root, relativePath); + }; + + const initialization = startSnapshotCoordinationInitialization(directory); + await waitFor(() => strictReadStarted); + beginHydrationScope(directory); + releaseRead(); + await initialization; + + expect(readSnapshotRows(directory)).toBeNull(); + expect(getSnapshotCoordinationStatus(directory)).toMatchObject({ + state: 'superseded', + settled: true, + }); + }); + + test('publishes superseded readiness instead of late projection state', async () => { + const directory = makeProject('readiness'); + writeSnapshotRows(directory, makeSnapshot('authoritative')); + await writeSnapshotProjection(directory, makeSnapshot('before-late-write')); + const projectionPath = path.join( + directory, + '.swarm', + SNAPSHOT_PROJECTION_FILE, + ); + const projectionBefore = readFileSync(projectionPath, 'utf8'); + + let releasePlan!: () => void; + let loadPlanStarted = false; + const planBarrier = new Promise((resolve) => { + releasePlan = resolve; + }); + _snapshotCoordinationInternals.loadPlan = async (root, cache, options) => { + loadPlanStarted = true; + await planBarrier; + return originalLoadPlan(root, cache, options); + }; + + const initialization = startSnapshotCoordinationInitialization(directory); + await waitFor(() => loadPlanStarted); + beginHydrationScope(directory); + releasePlan(); + await expect(initialization).rejects.toBeInstanceOf( + PlanRecoverySupersededError, + ); + + expect(getSnapshotCoordinationStatus(directory)).toMatchObject({ + state: 'superseded', + settled: true, + }); + expect(readFileSync(projectionPath, 'utf8')).toBe(projectionBefore); + }); + + test('does not let delayed loadPlan recovery mutate stale projections', async () => { + const directory = makeProject('load-plan-side-effects'); + await writeApprovedPlan(directory, [ + { + id: '1.1', + files: ['src/recovery-boundary.ts'], + status: 'completed', + }, + ]); + const planPath = path.join(directory, '.swarm', 'plan.json'); + const markdownPath = path.join(directory, '.swarm', 'plan.md'); + const invalidPlanJson = '{not valid json'; + writeFileSync(planPath, invalidPlanJson); + const markdownBefore = readFileSync(markdownPath, 'utf8'); + + let releaseRead!: () => void; + let readStarted = false; + const readBarrier = new Promise((resolve) => { + releaseRead = resolve; + }); + planManagerInternals.readPlanJsonUtf8 = async (root) => { + readStarted = true; + await readBarrier; + return originalReadPlanJsonUtf8(root); + }; + + const initialization = startSnapshotCoordinationInitialization(directory); + await waitFor(() => readStarted); + beginHydrationScope(directory); + releaseRead(); + await expect(initialization).rejects.toBeInstanceOf( + PlanRecoverySupersededError, + ); + + expect(getSnapshotCoordinationStatus(directory)).toMatchObject({ + state: 'superseded', + settled: true, + }); + expect(readFileSync(planPath, 'utf8')).toBe(invalidPlanJson); + expect(readFileSync(markdownPath, 'utf8')).toBe(markdownBefore); + }); + + test('retries a superseded attempt and runs current-generation ledger recovery', async () => { + const directory = makeProject('retry-authority'); + const executionProfile = { + parallelization_enabled: true, + max_concurrent_tasks: 2, + council_parallel: false, + locked: false, + auto_proceed: true, + commit_after_each_completed_task: false, + planning_profile: 'balanced' as const, + }; + const expected = await writeApprovedPlan( + directory, + [ + { + id: '1.1', + files: ['src/current-generation.ts'], + status: 'completed', + }, + ], + { executionProfile }, + ); + const planPath = path.join(directory, '.swarm', 'plan.json'); + writeFileSync(planPath, '{not valid json'); + writeSnapshotRows(directory, makeSnapshot('retry-authority')); + + let releaseFirstPlan!: () => void; + let loadPlanCalls = 0; + const firstPlanBarrier = new Promise((resolve) => { + releaseFirstPlan = resolve; + }); + _snapshotCoordinationInternals.loadPlan = async (root, cache, options) => { + loadPlanCalls += 1; + if (loadPlanCalls === 1) await firstPlanBarrier; + return originalLoadPlan(root, cache, options); + }; + + const staleAttempt = startSnapshotCoordinationInitialization(directory); + await waitFor(() => loadPlanCalls === 1); + beginHydrationScope(directory); + releaseFirstPlan(); + await expect(staleAttempt).rejects.toBeInstanceOf( + PlanRecoverySupersededError, + ); + expect(getSnapshotCoordinationStatus(directory).state).toBe('superseded'); + + await startSnapshotCoordinationInitialization(directory); + expect(loadPlanCalls).toBe(2); + expect(getSnapshotCoordinationStatus(directory).state).toBe('succeeded'); + expect(JSON.parse(readFileSync(planPath, 'utf8'))).toMatchObject({ + swarm: expected.swarm, + title: expected.title, + execution_profile: expected.execution_profile, + }); + }); + + test('releases a claimed startup recovery when superseded before replay (F2)', async () => { + const directory = makeProject('startup-claim'); + const expected = await writeApprovedPlan(directory, [ + { + id: '1.1', + files: ['src/startup-claim.ts'], + status: 'completed', + }, + ]); + const planPath = path.join(directory, '.swarm', 'plan.json'); + const staleProjection = JSON.parse(readFileSync(planPath, 'utf8')) as { + phases: Array<{ tasks: Array<{ status: string }> }>; + }; + // Before the cleanup fix, this valid-but-stale projection caused the + // first loadPlan call to claim the one-shot startup replay. If authority + // superseded it afterward, the claim leaked and the next invocation + // skipped the required ledger-authoritative recovery. + staleProjection.phases[0]!.tasks[0]!.status = 'pending'; + writeFileSync(planPath, JSON.stringify(staleProjection)); + invalidateCachedArtifact(planPath); + await planManagerInternals.regeneratePlanMarkdown( + directory, + staleProjection as Plan, + ); + + let preCommitChecks = 0; + await expect( + loadPlan(directory, undefined, { + preCommitCheck: () => { + preCommitChecks += 1; + if (preCommitChecks === 3) { + throw new PlanRecoverySupersededError( + 'startup replay superseded by a newer hydration generation', + ); + } + }, + }), + ).rejects.toBeInstanceOf(PlanRecoverySupersededError); + expect(preCommitChecks).toBe(3); + + const recovered = await loadPlan(directory); + expect(recovered?.phases[0]?.tasks[0]?.status).toBe('completed'); + expect( + (JSON.parse(readFileSync(planPath, 'utf8')) as typeof expected).phases[0] + ?.tasks[0]?.status, + ).toBe('completed'); + }); +}); + +describe('PR #2777 coordination feedback regressions', () => { + describe('IA-001 — plan recovery supersession', () => { + test('preserves typed supersession before stale cache and projection publication', async () => { + const directory = makeProject('IA-001-plan-recovery'); + const snapshot = makeSnapshot('authoritative'); + writeSnapshotRows(directory, snapshot); + await writeSnapshotProjection( + directory, + makeSnapshot('pre-resolution-projection'), + ); + const projectionPath = path.join( + directory, + '.swarm', + SNAPSHOT_PROJECTION_FILE, + ); + const projectionBefore = readFileSync(projectionPath, 'utf8'); + await writeApprovedPlan(directory, [ + { + id: '1.1', + files: ['src/stale-cache-probe.ts'], + status: 'completed', + }, + ]); + const cacheBuild = await buildRehydrationCache(directory); + expect(cacheBuild.committed).toBe(true); + + let releasePlan!: () => void; + let loadPlanStarted = false; + const planBarrier = new Promise((resolve) => { + releasePlan = resolve; + }); + _snapshotCoordinationInternals.loadPlan = async () => { + loadPlanStarted = true; + await planBarrier; + throw new PlanRecoverySupersededError( + 'newer plan recovery authority won', + ); + }; + + const initialization = startSnapshotCoordinationInitialization(directory); + await waitFor(() => loadPlanStarted); + const session = ensureAgentSession('ia-001-cache-probe', 'coder'); + session.owningProjectKey = hydrationProjectKey(directory); + expect(session.taskWorkflowStates.get('1.1')).toBeUndefined(); + + // Before this fix, the generic catch consumed this typed supersession, + // then applied the pre-resolution completed task and rewrote the projection. + releasePlan(); + await expect(initialization).rejects.toBeInstanceOf( + PlanRecoverySupersededError, + ); + expect(getSnapshotCoordinationStatus(directory)).toMatchObject({ + state: 'superseded', + settled: true, + }); + expect(session.taskWorkflowStates.get('1.1')).toBeUndefined(); + expect(readFileSync(projectionPath, 'utf8')).toBe(projectionBefore); + }); + }); + + describe('F-001 — readiness re-drive after supersession', () => { + test('redrives after real typed supersession from the plan pre-commit fence', async () => { + const directory = makeProject('F-001-readiness-redrive'); + await writeApprovedPlan(directory, [ + { + id: '1.1', + files: ['src/readiness-redrive-probe.ts'], + status: 'completed', + }, + ]); + let loadPlanCalls = 0; + _snapshotCoordinationInternals.loadPlan = async ( + root, + cache, + options, + ) => { + loadPlanCalls += 1; + if (loadPlanCalls === 1) beginHydrationScope(root); + return originalLoadPlan(root, cache, options); + }; + + await expect( + startSnapshotCoordinationInitialization(directory), + ).rejects.toBeInstanceOf(PlanRecoverySupersededError); + expect(getSnapshotCoordinationStatus(directory)).toMatchObject({ + state: 'superseded', + settled: true, + }); + expect(loadPlanCalls).toBe(1); + + // The first loadPlan call used the real coordinator preCommitCheck, which + // observed the newer hydration generation. Readiness then starts one fresh + // attempt under the current generation and completes normally. + await expect( + ensureSnapshotCoordinationReady(directory), + ).resolves.toBeUndefined(); + expect(loadPlanCalls).toBe(2); + expect(getSnapshotCoordinationStatus(directory)).toMatchObject({ + state: 'succeeded', + settled: true, + }); + }); + }); +}); diff --git a/tests/unit/session/restart-reconciliation-2668.test.ts b/tests/unit/session/restart-reconciliation-2668.test.ts new file mode 100644 index 000000000..585ee6f81 --- /dev/null +++ b/tests/unit/session/restart-reconciliation-2668.test.ts @@ -0,0 +1,478 @@ +/** + * Issue #2668 restart reconciliation boundaries. + * + * These checks exercise the production hydration reducer, cache publisher, + * snapshot projection writer, and post-resolution coordinator. The deferred + * barriers are deliberately placed immediately before each publication point + * so a superseded generation cannot publish a late result. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { + mkdirSync, + readdirSync, + readFileSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import path from 'node:path'; +import { resetStartupLedgerCheck } from '../../../src/plan/manager'; +import { + clearDeferredWarnings, + getDeferredWarnings, +} from '../../../src/services/warning-buffer'; +import { + beginHydrationScope, + hydrationProjectKey, +} from '../../../src/session/hydration-ownership'; +import { + _snapshotCoordinationInternals, + getSnapshotCoordinationStatus, + startSnapshotCoordinationInitialization, +} from '../../../src/session/snapshot-coordination-init'; +import { + loadSnapshot, + rehydrateState, +} from '../../../src/session/snapshot-reader'; +import { + SNAPSHOT_PROJECTION_FILE, + type SnapshotData, + writeSnapshotProjection, +} from '../../../src/session/snapshot-writer'; +import { + buildRehydrationCache, + getRehydrationCache, + resetSwarmState, + startAgentSession, + _internals as stateInternals, + swarmState, +} from '../../../src/state'; +import { writeApprovedPlan } from '../../helpers/approved-plan'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { withFrozenClock } from '../../helpers/test-clock'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const tempDirs: string[] = []; +const originalStateBuildRehydrationCache = stateInternals.buildRehydrationCache; +const originalStateRehydrateSessionFromDisk = + stateInternals.rehydrateSessionFromDisk; + +function makeProject(prefix: string): string { + const directory = canonicalMkdtemp(`swarm-2668-${prefix}-`); + mkdirSync(path.join(directory, '.git')); + tempDirs.push(directory); + return directory; +} + +function snapshot(sessionID: string, agentName = 'coder'): SnapshotData { + return { + version: 3, + writtenAt: withFrozenClock(() => Date.now()), + toolAggregates: {}, + activeAgent: { [sessionID]: agentName }, + delegationChains: {}, + agentSessions: { + [sessionID]: { + agentName, + lastToolCallTime: 1, + lastAgentEventTime: 1, + delegationActive: false, + }, + }, + } as unknown as SnapshotData; +} + +function projectionPath(directory: string): string { + return path.join(directory, '.swarm', SNAPSHOT_PROJECTION_FILE); +} + +async function waitFor(check: () => boolean): Promise { + for (let attempt = 0; attempt < 100; attempt += 1) { + if (check()) return; + await new Promise((resolve) => setImmediate(resolve)); + } + throw new Error('timed out waiting for the deferred hydration boundary'); +} + +beforeEach(() => { + _snapshotCoordinationInternals.entries.clear(); + stateInternals.buildRehydrationCache = originalStateBuildRehydrationCache; + stateInternals.rehydrateSessionFromDisk = + originalStateRehydrateSessionFromDisk; + resetStartupLedgerCheck(); + resetSwarmState(); + clearDeferredWarnings(); +}); + +afterEach(() => { + _snapshotCoordinationInternals.entries.clear(); + stateInternals.buildRehydrationCache = originalStateBuildRehydrationCache; + stateInternals.rehydrateSessionFromDisk = + originalStateRehydrateSessionFromDisk; + resetSwarmState(); + clearDeferredWarnings(); + for (const directory of tempDirs.splice(0)) safeRmRecursive(directory); +}); + +describe('reducer and cache publication fences (#2668)', () => { + test('a generation superseded during the reducer barrier publishes nothing', async () => { + const directory = makeProject('reducer'); + const old = snapshot('old-session'); + const scope = beginHydrationScope(directory); + const aggregate = { + tool: 'sentinel', + count: 7, + successCount: 7, + failureCount: 0, + totalDuration: 1, + }; + swarmState.toolAggregates.set('sentinel', aggregate); + startAgentSession('live-session', 'architect'); + + let release!: () => void; + const barrier = new Promise((resolve) => { + release = resolve; + }); + swarmState.pendingRehydrations.add(barrier); + const applying = rehydrateState(old, directory, scope); + await Promise.resolve(); + beginHydrationScope(directory); + release(); + + expect(await applying).toEqual({ applied: false, reason: 'superseded' }); + expect(swarmState.toolAggregates.get('sentinel')).toEqual(aggregate); + expect(swarmState.agentSessions.has('live-session')).toBe(true); + expect(swarmState.agentSessions.has('old-session')).toBe(false); + swarmState.pendingRehydrations.delete(barrier); + }); + + test('superseded loadSnapshot retains its cache but cannot apply it to a newer live session', async () => { + const directory = makeProject('load-snapshot'); + await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/load-snapshot.ts'], status: 'completed' }, + ]); + await writeSnapshotProjection(directory, snapshot('old-load-session')); + let release!: () => void; + let entered = false; + const barrier = { + then(resolve: () => void) { + entered = true; + return new Promise((finish) => { + release = () => { + finish(); + resolve(); + }; + }); + }, + } as unknown as Promise; + swarmState.pendingRehydrations.add(barrier); + const loading = loadSnapshot(directory); + await waitFor(() => entered); + beginHydrationScope(directory); + startAgentSession('live-load-session', 'architect', undefined, directory); + const live = swarmState.agentSessions.get('live-load-session'); + expect(live).toBeDefined(); + const liveRehydration = [...swarmState.pendingRehydrations].find( + (pending) => pending !== barrier, + ); + if (liveRehydration) await liveRehydration; + // This is newer in-memory workflow state. The stale load must not + // overwrite it with the cache it built before its reducer was superseded. + live!.taskWorkflowStates.set('1.1', 'idle'); + release(); + await loading; + expect(swarmState.agentSessions.has('old-load-session')).toBe(false); + expect(swarmState.agentSessions.has('live-load-session')).toBe(true); + const cache = getRehydrationCache(hydrationProjectKey(directory)) as { + planTaskStates: Map; + }; + expect(cache.planTaskStates.get('1.1')).toBe('complete'); + expect(live!.taskWorkflowStates.get('1.1')).toBe('idle'); + swarmState.pendingRehydrations.delete(barrier); + }); + + test('a superseded cache build leaves the prior project cache intact', async () => { + const directory = makeProject('cache'); + const planPath = path.join(directory, '.swarm', 'plan.json'); + await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/cache.ts'], status: 'completed' }, + ]); + await buildRehydrationCache(directory); + const key = hydrationProjectKey(directory); + const before = getRehydrationCache(key) as { + planTaskStates: Map; + }; + expect(before.planTaskStates.get('1.1')).toBe('complete'); + + const changed = JSON.parse(readFileSync(planPath, 'utf8')) as { + phases: Array<{ tasks: Array<{ status: string }> }>; + }; + changed.phases[0]!.tasks[0]!.status = 'pending'; + writeFileSync(planPath, JSON.stringify(changed)); + const result = await buildRehydrationCache(directory, { + shouldCommit: () => false, + }); + expect(result).toEqual({ committed: false, reason: 'superseded' }); + const after = getRehydrationCache(key) as { + planTaskStates: Map; + }; + expect(after.planTaskStates.get('1.1')).toBe('complete'); + }); + + test('a delayed session refresh cannot replace newer cache or apply old state', async () => { + const directory = makeProject('delayed-session-refresh'); + const oldPlan = await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/old-refresh.ts'], status: 'completed' }, + ]); + await buildRehydrationCache(directory); + beginHydrationScope(directory); + + let releaseOldRefresh!: () => void; + let refreshEntered = false; + const oldRefreshBarrier = new Promise((resolve) => { + releaseOldRefresh = resolve; + }); + stateInternals.buildRehydrationCache = async (root, options) => { + if (!refreshEntered) { + refreshEntered = true; + await oldRefreshBarrier; + // Reproduce the delayed operation's old read: it publishes the + // plan that was current when startAgentSession began. + return originalStateBuildRehydrationCache(root, { + ...options, + planOverride: oldPlan, + }); + } + return originalStateBuildRehydrationCache(root, options); + }; + + startAgentSession( + 'delayed-refresh-session', + 'architect', + undefined, + directory, + ); + await waitFor(() => refreshEntered); + const newerScope = beginHydrationScope(directory); + expect(newerScope.generation).toBeGreaterThan(1); + + const planPath = path.join(directory, '.swarm', 'plan.json'); + const newerProjection = JSON.parse(readFileSync(planPath, 'utf8')) as { + phases: Array<{ tasks: Array<{ status: string }> }>; + }; + newerProjection.phases[0]!.tasks[0]!.status = 'in_progress'; + writeFileSync(planPath, JSON.stringify(newerProjection)); + await originalStateBuildRehydrationCache(directory); + const newerCache = getRehydrationCache(hydrationProjectKey(directory)) as { + planTaskStates: Map; + }; + expect(newerCache.planTaskStates.get('1.1')).toBe('idle'); + + const live = swarmState.agentSessions.get('delayed-refresh-session'); + expect(live).toBeDefined(); + live!.taskWorkflowStates.set('1.1', 'idle'); + const refresh = [...swarmState.pendingRehydrations][0]; + releaseOldRefresh(); + if (refresh) await refresh; + + const finalCache = getRehydrationCache(hydrationProjectKey(directory)) as { + planTaskStates: Map; + }; + expect(finalCache.planTaskStates.get('1.1')).toBe('idle'); + expect(live!.taskWorkflowStates.get('1.1')).toBe('idle'); + }); + + test('a superseded projection writer preserves the canonical file and cleans temp output', async () => { + const directory = makeProject('projection'); + const old = snapshot('projection-old'); + const next = snapshot('projection-new', 'architect'); + await writeSnapshotProjection(directory, old); + // The approved contract adds the predicate as an optional third argument; + // this local type keeps the regression check readable while the source + // remains backward-compatible for its existing two-argument callers. + const fencedWriter = writeSnapshotProjection as unknown as ( + root: string, + value: SnapshotData, + shouldCommit?: () => boolean, + ) => Promise; + await fencedWriter(directory, next, () => false); + expect(JSON.parse(readFileSync(projectionPath(directory), 'utf8'))).toEqual( + old, + ); + const leftovers = readdirSync(path.join(directory, '.swarm')).filter( + (name) => name.startsWith(`${SNAPSHOT_PROJECTION_FILE}.tmp.`), + ).length; + expect(leftovers).toBe(0); + }); +}); + +describe('authoritative plan recovery at the post-resolution boundary (#2668)', () => { + async function assertLedgerRecovery( + mode: 'missing' | 'corrupt', + ): Promise { + const directory = makeProject(`ledger-${mode}`); + const executionProfile = { + parallelization_enabled: true, + max_concurrent_tasks: 3, + council_parallel: false, + locked: false, + auto_proceed: false, + commit_after_each_completed_task: true, + planning_profile: 'strict' as const, + }; + const expected = await writeApprovedPlan( + directory, + [{ id: '1.1', files: ['src/recovered.ts'], status: 'completed' }], + { executionProfile }, + ); + const planPath = path.join(directory, '.swarm', 'plan.json'); + const markdownPath = path.join(directory, '.swarm', 'plan.md'); + if (mode === 'missing') { + unlinkSync(planPath); + unlinkSync(markdownPath); + } else { + writeFileSync(planPath, '{not valid json'); + unlinkSync(markdownPath); + } + + await startSnapshotCoordinationInitialization(directory); + expect(getSnapshotCoordinationStatus(directory).state).toBe('succeeded'); + const rebuilt = JSON.parse( + readFileSync(planPath, 'utf8'), + ) as typeof expected; + expect(rebuilt.swarm).toBe(expected.swarm); + expect(rebuilt.title).toBe(expected.title); + expect(rebuilt.execution_profile).toEqual(expected.execution_profile); + expect(rebuilt.phases[0]!.tasks[0]!.id).toBe('1.1'); + expect(rebuilt.phases[0]!.tasks[0]!.status).toBe('completed'); + startAgentSession(`restored-${mode}`, 'architect', undefined, directory); + expect( + swarmState.agentSessions + .get(`restored-${mode}`) + ?.taskWorkflowStates.get('1.1'), + ).toBe('complete'); + } + + test('missing plan projection is rebuilt from the valid ledger', async () => { + await assertLedgerRecovery('missing'); + }); + + test('corrupt plan projection is rebuilt from the valid ledger', async () => { + await assertLedgerRecovery('corrupt'); + }); + + test('a corrupt projection cannot replace the ledger execution profile with legacy markdown defaults', async () => { + const directory = makeProject('ledger-profile-authority'); + const executionProfile = { + parallelization_enabled: true, + max_concurrent_tasks: 4, + council_parallel: true, + locked: true, + auto_proceed: true, + commit_after_each_completed_task: true, + planning_profile: 'strict' as const, + }; + const expected = await writeApprovedPlan( + directory, + [{ id: '1.1', files: ['src/profile-authority.ts'], status: 'completed' }], + { executionProfile }, + ); + const planPath = path.join(directory, '.swarm', 'plan.json'); + const markdownPath = path.join(directory, '.swarm', 'plan.md'); + writeFileSync(planPath, '{not valid json'); + // A valid legacy projection intentionally omits execution_profile and has + // different identity/status. Only the verified ledger can preserve the + // approved profile and completed task here. + writeFileSync( + markdownPath, + '# Legacy Markdown Default\nSwarm: legacy-default\nPhase: 1\n\n## Phase 1: Legacy Phase [PENDING]\n- [ ] 1.1: Legacy task [SMALL]\n', + ); + + await startSnapshotCoordinationInitialization(directory); + expect(getSnapshotCoordinationStatus(directory).state).toBe('succeeded'); + const rebuilt = JSON.parse( + readFileSync(planPath, 'utf8'), + ) as typeof expected; + expect(rebuilt.swarm).toBe(expected.swarm); + expect(rebuilt.title).toBe(expected.title); + expect(rebuilt.execution_profile).toEqual(expected.execution_profile); + expect(rebuilt.phases[0]!.tasks[0]!.status).toBe('completed'); + }); + + test('a valid ledger with no snapshot rows still refreshes plan-derived session state', async () => { + const directory = makeProject('ledger-no-snapshot'); + const executionProfile = { + parallelization_enabled: true, + max_concurrent_tasks: 2, + council_parallel: true, + locked: false, + auto_proceed: true, + commit_after_each_completed_task: false, + planning_profile: 'balanced' as const, + }; + const expected = await writeApprovedPlan( + directory, + [{ id: '1.1', files: ['src/no-snapshot.ts'], status: 'completed' }], + { executionProfile }, + ); + unlinkSync(path.join(directory, '.swarm', 'plan.json')); + unlinkSync(path.join(directory, '.swarm', 'plan.md')); + + await startSnapshotCoordinationInitialization(directory); + const rebuilt = JSON.parse( + readFileSync(path.join(directory, '.swarm', 'plan.json'), 'utf8'), + ) as typeof expected; + expect(rebuilt.swarm).toBe(expected.swarm); + expect(rebuilt.execution_profile).toEqual(expected.execution_profile); + startAgentSession('no-snapshot-session', 'architect', undefined, directory); + expect( + swarmState.agentSessions + .get('no-snapshot-session') + ?.taskWorkflowStates.get('1.1'), + ).toBe('complete'); + }); +}); + +describe('fail-open readiness when authoritative recovery is unavailable (#2668)', () => { + test('a plan-less load succeeds without an advisory or stale cache', async () => { + const directory = makeProject('planless'); + await buildRehydrationCache(directory); + const original = _snapshotCoordinationInternals.loadPlan; + _snapshotCoordinationInternals.loadPlan = async () => null; + try { + await startSnapshotCoordinationInitialization(directory); + expect(getSnapshotCoordinationStatus(directory).state).toBe('succeeded'); + expect(getRehydrationCache(hydrationProjectKey(directory))).toMatchObject( + { + planTaskStates: new Map(), + }, + ); + } finally { + _snapshotCoordinationInternals.loadPlan = original; + } + }); + + test('a throwing authoritative loader keeps readiness successful and retains cache with an advisory', async () => { + const directory = makeProject('plan-throw'); + await writeApprovedPlan(directory, [ + { id: '1.1', files: ['src/retained.ts'], status: 'completed' }, + ]); + await buildRehydrationCache(directory); + const original = _snapshotCoordinationInternals.loadPlan; + _snapshotCoordinationInternals.loadPlan = async () => { + throw new Error('injected ledger unavailable'); + }; + try { + await startSnapshotCoordinationInitialization(directory); + expect(getSnapshotCoordinationStatus(directory).state).toBe('succeeded'); + const cache = getRehydrationCache(hydrationProjectKey(directory)) as { + planTaskStates: Map; + }; + expect(cache.planTaskStates.get('1.1')).toBe('complete'); + expect(getDeferredWarnings().join('\n')).toContain( + 'Authoritative plan recovery failed', + ); + } finally { + _snapshotCoordinationInternals.loadPlan = original; + } + }); +}); diff --git a/tests/unit/session/restart-subscription-fence-2668.test.ts b/tests/unit/session/restart-subscription-fence-2668.test.ts new file mode 100644 index 000000000..093da7da5 --- /dev/null +++ b/tests/unit/session/restart-subscription-fence-2668.test.ts @@ -0,0 +1,137 @@ +/** + * Issue #2668 — the follow-on PR-subscription read must share the session + * refresh fence with plan/evidence rehydration. + */ + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdirSync } from 'node:fs'; +import path from 'node:path'; +import { beginHydrationScope } from '../../../src/session/hydration-ownership'; +import { + type PrSubscriptionState, + resetSwarmState, + startAgentSession, + _internals as stateInternals, + swarmState, +} from '../../../src/state'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const tempDirs: string[] = []; +const originalRehydrateSessionFromDisk = + stateInternals.rehydrateSessionFromDisk; +const originalRehydratePrSubscriptions = + stateInternals.rehydratePrSubscriptions; + +function deferred(): { + promise: Promise; + resolve: (value: T) => void; +} { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + +function makeProject(): string { + const directory = canonicalMkdtemp('swarm-2668-subscriptions-'); + tempDirs.push(directory); + mkdirSync(path.join(directory, '.swarm'), { recursive: true }); + return directory; +} + +function makeSubscriptions(status: string): Map { + return new Map([ + [ + 'owner/repo::1', + { + prNumber: 1, + repoFullName: 'owner/repo', + prUrl: 'https://github.com/owner/repo/pull/1', + lastKnownStatus: status, + lastPollTime: 1, + errorCount: 0, + isWatching: true, + }, + ], + ]); +} + +async function settlePendingRehydration(): Promise { + await Promise.allSettled([...swarmState.pendingRehydrations]); +} + +beforeEach(() => { + stateInternals.rehydrateSessionFromDisk = originalRehydrateSessionFromDisk; + stateInternals.rehydratePrSubscriptions = originalRehydratePrSubscriptions; + resetSwarmState(); +}); + +afterEach(() => { + stateInternals.rehydrateSessionFromDisk = originalRehydrateSessionFromDisk; + stateInternals.rehydratePrSubscriptions = originalRehydratePrSubscriptions; + resetSwarmState(); + for (const directory of tempDirs.splice(0)) { + safeRmRecursive(directory); + } +}); + +describe('session PR-subscription refresh fence (#2668)', () => { + test('does not start a stale subscription read after a newer hydration begins', async () => { + const directory = makeProject(); + const sessionID = 'subscription-before-read'; + const refreshStarted = deferred(); + const releaseRefresh = deferred(); + let subscriptionReadStarted = false; + + stateInternals.rehydrateSessionFromDisk = async () => { + refreshStarted.resolve(); + await releaseRefresh.promise; + }; + stateInternals.rehydratePrSubscriptions = async () => { + subscriptionReadStarted = true; + return makeSubscriptions('stale'); + }; + + startAgentSession(sessionID, 'coder', undefined, directory); + const liveSession = swarmState.agentSessions.get(sessionID); + expect(liveSession).toBeDefined(); + await refreshStarted.promise; + + beginHydrationScope(directory); + releaseRefresh.resolve(); + await settlePendingRehydration(); + + expect(subscriptionReadStarted).toBe(false); + expect(liveSession?.prSubscriptions).toEqual(new Map()); + }); + + test('does not assign a delayed stale result to the exact live session', async () => { + const directory = makeProject(); + const sessionID = 'subscription-after-await'; + const subscriptionReadStarted = deferred(); + const releaseSubscriptionRead = + deferred>(); + const staleSubscriptions = makeSubscriptions('stale'); + + stateInternals.rehydrateSessionFromDisk = async () => {}; + stateInternals.rehydratePrSubscriptions = async () => { + subscriptionReadStarted.resolve(); + return releaseSubscriptionRead.promise; + }; + + startAgentSession(sessionID, 'coder', undefined, directory); + const liveSession = swarmState.agentSessions.get(sessionID); + expect(liveSession).toBeDefined(); + await subscriptionReadStarted.promise; + + beginHydrationScope(directory); + releaseSubscriptionRead.resolve(staleSubscriptions); + await settlePendingRehydration(); + + expect(swarmState.agentSessions.get(sessionID)).toBe(liveSession); + expect(liveSession?.prSubscriptions).toEqual(new Map()); + expect(liveSession?.prSubscriptions).not.toBe(staleSubscriptions); + }); +}); diff --git a/tests/unit/session/snapshot-coordination-cap.test.ts b/tests/unit/session/snapshot-coordination-cap.test.ts index e4c754e41..d444eea18 100644 --- a/tests/unit/session/snapshot-coordination-cap.test.ts +++ b/tests/unit/session/snapshot-coordination-cap.test.ts @@ -27,7 +27,10 @@ describe('snapshot coordination readiness bounds', () => { const blocked = new Promise((resolve) => { release = resolve; }); - _snapshotCoordinationInternals.initialize = async () => blocked; + _snapshotCoordinationInternals.initialize = async () => { + await blocked; + return 'succeeded'; + }; _snapshotCoordinationInternals.timeoutMs = 1; const attempts = Array.from({ length: 32 }, (_, index) => diff --git a/tests/unit/session/snapshot-coordination-init.test.ts b/tests/unit/session/snapshot-coordination-init.test.ts index 3ec5654d8..5ca712026 100644 --- a/tests/unit/session/snapshot-coordination-init.test.ts +++ b/tests/unit/session/snapshot-coordination-init.test.ts @@ -417,6 +417,7 @@ describe('snapshot coordination post-resolution initialization', () => { _snapshotCoordinationInternals.initialize = async () => { calls += 1; if (calls === 1) await blocked; + return 'succeeded'; }; const underlying = startSnapshotCoordinationInitialization(tempDir); await new Promise((resolve) => setTimeout(resolve, 20)); @@ -448,7 +449,8 @@ describe('snapshot coordination post-resolution initialization', () => { release = resolve; }); _snapshotCoordinationInternals.timeoutMs = 5; - _snapshotCoordinationInternals.initialize = () => blocked; + _snapshotCoordinationInternals.initialize = () => + blocked.then(() => 'succeeded'); const initialization = startSnapshotCoordinationInitialization(tempDir); const startedAt = performance.now(); diff --git a/tests/unit/session/snapshot-coordination-root-binding-SRC-002.test.ts b/tests/unit/session/snapshot-coordination-root-binding-SRC-002.test.ts new file mode 100644 index 000000000..eab56862d --- /dev/null +++ b/tests/unit/session/snapshot-coordination-root-binding-SRC-002.test.ts @@ -0,0 +1,64 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { + _snapshotCoordinationInternals, + startSnapshotCoordinationInitialization, +} from '../../../src/session/snapshot-coordination-init.js'; +import { _internals as canonicalRootInternals } from '../../../src/utils/canonical-root.js'; +import { canonicalMkdtemp } from '../../helpers/tmpdir.js'; + +describe('snapshot coordination root binding — regression: SRC-002', () => { + const originalInitialize = _snapshotCoordinationInternals.initialize; + const originalRealpathSyncNative = canonicalRootInternals.realpathSyncNative; + const originalRealpathSync = canonicalRootInternals.realpathSync; + let tempDir: string | undefined; + + afterEach(() => { + _snapshotCoordinationInternals.initialize = originalInitialize; + _snapshotCoordinationInternals.entries.clear(); + canonicalRootInternals.realpathSyncNative = originalRealpathSyncNative; + canonicalRootInternals.realpathSync = originalRealpathSync; + if (tempDir) fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + test('retargeted alias cannot split hydration authority from initializer root', async () => { + tempDir = canonicalMkdtemp('snapshot-coordination-root-binding-'); + const alias = path.join(tempDir, 'moving-alias'); + const rootA = path.join(tempDir, 'project-a'); + const rootB = path.join(tempDir, 'project-b'); + const resolvedAlias = path.resolve(alias); + const normalized = (root: string) => + process.platform === 'win32' + ? path.resolve(root).toLowerCase() + : path.resolve(root); + let aliasResolutions = 0; + canonicalRootInternals.realpathSyncNative = (candidate) => { + const resolved = path.resolve(String(candidate)); + if (resolved === resolvedAlias) { + aliasResolutions += 1; + // Simulate an A→B symlink/junction retarget after the initializer + // captures its project root but before it starts hydration. + return aliasResolutions === 1 ? rootA : rootB; + } + return resolved; + }; + + let initialized: { directory: string; projectKey?: string } | undefined; + _snapshotCoordinationInternals.initialize = async (directory, scope) => { + initialized = { directory, projectKey: scope?.projectKey }; + return 'succeeded'; + }; + + await startSnapshotCoordinationInitialization(alias); + + const expectedRootA = normalized(rootA); + const expectedRootB = normalized(rootB); + expect(aliasResolutions).toBe(1); + expect(expectedRootA).not.toBe(expectedRootB); + expect(initialized).toEqual({ + directory: expectedRootA, + projectKey: expectedRootA, + }); + }); +}); diff --git a/tests/unit/session/snapshot-field-parity-guard.test.ts b/tests/unit/session/snapshot-field-parity-guard.test.ts index 10b53e6f8..2c917277f 100644 --- a/tests/unit/session/snapshot-field-parity-guard.test.ts +++ b/tests/unit/session/snapshot-field-parity-guard.test.ts @@ -132,6 +132,7 @@ function buildFullSessionState(): AgentSessionState { // on live state so their omission from the snapshot is asserted below. owningProjectKey: 'canonical-project-key-fixture', hydrationStamp: 1, + hydrationAuthorityEpoch: 1, lastScopeViolation: null, scopeViolationDetected: true, modifiedFilesByTask: new Map([['task-1', ['src/a.ts', 'src/b.ts']]]), diff --git a/tests/unit/session/snapshot-reconciliation-authority-SRC-001.test.ts b/tests/unit/session/snapshot-reconciliation-authority-SRC-001.test.ts new file mode 100644 index 000000000..b31b46155 --- /dev/null +++ b/tests/unit/session/snapshot-reconciliation-authority-SRC-001.test.ts @@ -0,0 +1,123 @@ +/** + * SRC-001: restart-reconciliation writes must not reopen a superseded + * hydration's shared-state publication window. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdirSync } from 'node:fs'; +import path from 'node:path'; +import { beginHydrationScope } from '../../../src/session/hydration-ownership'; +import { readRestartReconciliation } from '../../../src/session/restart-reconciliation'; +import { + rehydrateState, + _internals as snapshotReaderInternals, +} from '../../../src/session/snapshot-reader'; +import type { SnapshotData } from '../../../src/session/snapshot-writer'; +import { resetSwarmState, swarmState } from '../../../src/state'; +import { safeRmRecursive } from '../../helpers/safe-test-dir'; +import { canonicalMkdtemp } from '../../helpers/tmpdir'; + +const SESSION = 'src-001-same-session'; +const originalRecordInterruptedExecution = + snapshotReaderInternals.recordInterruptedExecution; + +let project: string; + +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + const promise = new Promise((complete) => { + resolve = complete; + }); + return { promise, resolve }; +} + +function snapshot( + agentName: string, + taskId: string, + delegationActive: boolean, +): SnapshotData { + return { + version: 3, + writtenAt: 1, + toolAggregates: {}, + activeAgent: { [SESSION]: agentName }, + delegationChains: {}, + agentSessions: { + [SESSION]: { + agentName, + lastToolCallTime: 1, + lastAgentEventTime: 1, + delegationActive, + currentTaskId: taskId, + }, + }, + } as unknown as SnapshotData; +} + +beforeEach(() => { + project = canonicalMkdtemp('snapshot-reconcile-src-001-'); + mkdirSync(path.join(project, '.swarm', 'session'), { recursive: true }); + snapshotReaderInternals.recordInterruptedExecution = + originalRecordInterruptedExecution; + resetSwarmState(); +}); + +afterEach(() => { + snapshotReaderInternals.recordInterruptedExecution = + originalRecordInterruptedExecution; + resetSwarmState(); + safeRmRecursive(project); +}); + +describe('restart reconciliation hydration authority (SRC-001)', () => { + test('superseded write cannot publish stale session or advisory', async () => { + // Before the fix, the old hydration awaited this durable write after it + // had started applying its snapshot, then published its stale session and + // advisory over the newer hydration when the write resolved. + const writeStarted = deferred(); + const releaseWrite = deferred(); + snapshotReaderInternals.recordInterruptedExecution = async ( + directory, + entry, + ) => { + writeStarted.resolve(); + await releaseWrite.promise; + return originalRecordInterruptedExecution(directory, entry); + }; + + const oldScope = beginHydrationScope(project); + const oldApply = rehydrateState( + snapshot('architect', 'old-task', true), + project, + oldScope, + ); + await writeStarted.promise; + + const newScope = beginHydrationScope(project); + const newOutcome = await rehydrateState( + snapshot('coder', 'new-task', false), + project, + newScope, + ); + expect(newOutcome).toEqual({ applied: true }); + const newerSession = swarmState.agentSessions.get(SESSION); + expect(newerSession?.agentName).toBe('coder'); + expect(newerSession?.currentTaskId).toBe('new-task'); + + releaseWrite.resolve(); + const oldOutcome = await oldApply; + + expect(oldOutcome).toEqual({ applied: false, reason: 'superseded' }); + expect(swarmState.agentSessions.get(SESSION)).toBe(newerSession); + expect(newerSession?.pendingAdvisoryMessages ?? []).toHaveLength(0); + expect( + swarmState.agentSessions.get(SESSION)?.pendingAdvisoryMessages ?? [], + ).toHaveLength(0); + expect(readRestartReconciliation(project).entries).toContainEqual( + expect.objectContaining({ + sessionId: SESSION, + taskId: 'old-task', + classification: 'interrupted', + }), + ); + }); +}); diff --git a/tests/unit/session/snapshot-writer-rename-retry.test.ts b/tests/unit/session/snapshot-writer-rename-retry.test.ts index b8bcb7419..d8213a302 100644 --- a/tests/unit/session/snapshot-writer-rename-retry.test.ts +++ b/tests/unit/session/snapshot-writer-rename-retry.test.ts @@ -31,6 +31,7 @@ import { SNAPSHOT_RENAME_MAX_ATTEMPTS, type SnapshotData, writeSnapshot, + writeSnapshotProjection, } from '../../../src/session/snapshot-writer'; import { _internals as artifactCacheInternals, @@ -131,12 +132,13 @@ describe('writeSnapshot — regression: transient rename failure must not drop t ).toEqual([]); }); - it('treats ENOENT on a retry as the spurious-failure success it is, so the cache is still invalidated', async () => { - // Windows can report a sharing violation for a rename that actually - // committed; the retry then finds the temp already gone. Reporting that - // as a failure would skip invalidateCachedArtifact for a file that - // really did change — the exact stale cached read issue #1729 guards - // against. + it('treats ENOENT after a retry commits as success, so the cache is still invalidated', async () => { + // Windows can report a transient failure before the swap, then report + // ENOENT for the retry that actually commits. Reporting that second + // result as a failure would skip invalidateCachedArtifact for a file + // that really did change — the exact stale cached read issue #1729 guards + // against. A commit followed by EPERM on the first attempt is covered + // separately below. // // The assertion has to be the cache entry, not the file: on this path // the snapshot lands on disk either way, so asserting file contents @@ -171,11 +173,10 @@ describe('writeSnapshot — regression: transient rename failure must not drop t _internals.rename = mock(async (oldPath: string, newPath: string) => { calls++; if (calls === 1) { - // The move lands despite the reported sharing violation. - await originalRename(oldPath, newPath); throw transientError('EPERM'); } - // The real filesystem now fails this way: the source is gone. + // The move lands despite the reported missing source on retry. + await originalRename(oldPath, newPath); throw transientError('ENOENT'); }); @@ -201,4 +202,147 @@ describe('writeSnapshot — regression: transient rename failure must not drop t readdirSync(sessionDir()).filter((f) => f.includes('.tmp.')), ).toEqual([]); }); + + it('re-checks authority inside a delayed async rename adapter before the atomic swap', async () => { + mkdirSync(sessionDir(), { recursive: true }); + const oldSnapshot: SnapshotData = { + version: 3, + writtenAt: 1_700_000_000_000, + toolAggregates: {}, + activeAgent: {}, + delegationChains: {}, + agentSessions: {}, + }; + const oldSnapshotText = JSON.stringify(oldSnapshot); + writeFileSync(statePath(), oldSnapshotText, 'utf8'); + const primed = await readCachedTextFile(statePath(), async () => + readFileSync(statePath(), 'utf8'), + ); + expect(primed).toBe(oldSnapshotText); + const frozenStat = await fsp.stat(statePath()); + artifactCacheInternals.stat = (async () => + frozenStat) as typeof artifactCacheInternals.stat; + + let allowCommit = true; + let renameCalls = 0; + let releaseRename!: () => void; + const renameEntered = new Promise((resolve) => { + releaseRename = resolve; + }); + let renameStarted!: () => void; + const renameStartedPromise = new Promise((resolve) => { + renameStarted = resolve; + }); + _internals.rename = mock( + async ( + oldPath: string, + newPath: string, + shouldCommit?: () => boolean, + ) => { + renameCalls++; + renameStarted(); + await renameEntered; + if (shouldCommit && !shouldCommit()) return; + return originalRename(oldPath, newPath); + }, + ); + + const nextSnapshot: SnapshotData = { + ...oldSnapshot, + writtenAt: 1_700_000_001_000, + }; + const writing = writeSnapshotProjection( + testDir, + nextSnapshot, + () => allowCommit, + ); + await renameStartedPromise; + allowCommit = false; + releaseRename(); + await writing; + + expect(renameCalls).toBe(1); + expect(readFileSync(statePath(), 'utf8')).toBe(oldSnapshotText); + let directReads = 0; + const observed = await readCachedTextFile(statePath(), async () => { + directReads++; + return readFileSync(statePath(), 'utf8'); + }); + expect(observed).toBe(oldSnapshotText); + // A declined write leaves the warmed cache intact; invalidating it would + // turn this into an unnecessary direct read even though the file is old. + expect(directReads).toBe(0); + expect( + readdirSync(sessionDir()).filter((f) => f.includes('.tmp.')), + ).toEqual([]); + }); + + it('invalidates cache when Windows reports a transient error after the rename commits, even if authority changes before retry', async () => { + mkdirSync(sessionDir(), { recursive: true }); + const oldSnapshot: SnapshotData = { + version: 3, + writtenAt: 1_700_000_000_000, + toolAggregates: {}, + activeAgent: {}, + delegationChains: {}, + agentSessions: {}, + }; + const oldSnapshotText = JSON.stringify(oldSnapshot); + writeFileSync(statePath(), oldSnapshotText, 'utf8'); + const primed = await readCachedTextFile(statePath(), async () => + readFileSync(statePath(), 'utf8'), + ); + expect(primed).toBe(oldSnapshotText); + const frozenStat = await fsp.stat(statePath()); + artifactCacheInternals.stat = (async () => + frozenStat) as typeof artifactCacheInternals.stat; + + let allowCommit = true; + let renameCalls = 0; + let reportFailure!: () => void; + const failureReleased = new Promise((resolve) => { + reportFailure = resolve; + }); + let signalCommitted!: () => void; + const committed = new Promise((resolve) => { + signalCommitted = resolve; + }); + _internals.rename = mock(async (oldPath: string, newPath: string) => { + renameCalls++; + await originalRename(oldPath, newPath); + signalCommitted(); + await failureReleased; + throw transientError('EPERM'); + }); + + const nextSnapshot: SnapshotData = { + ...oldSnapshot, + writtenAt: 1_700_000_001_000, + }; + const writing = writeSnapshotProjection( + testDir, + nextSnapshot, + () => allowCommit, + ); + await committed; + // Model a newer writer superseding this operation after Windows has + // moved the file but before the adapter reports its transient error. + allowCommit = false; + reportFailure(); + await writing; + + expect(renameCalls).toBe(1); + let directReads = 0; + const observed = await readCachedTextFile(statePath(), async () => { + directReads++; + return readFileSync(statePath(), 'utf8'); + }); + expect(directReads).toBe(1); + expect((JSON.parse(observed ?? 'null') as SnapshotData).writtenAt).toBe( + nextSnapshot.writtenAt, + ); + expect( + readdirSync(sessionDir()).filter((f) => f.includes('.tmp.')), + ).toEqual([]); + }); });