diff --git a/.changeset/steady-tunas-tumble.md b/.changeset/steady-tunas-tumble.md new file mode 100644 index 000000000..abdcf7567 --- /dev/null +++ b/.changeset/steady-tunas-tumble.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4681 +--- +**Parallel ledger writers no longer silently lose windows entries** — two concurrent `gsd_run windows append` (or waive/fixed) invocations both reported success while one entry vanished from `WINDOWS.md`, false-greening the /gsd-ship gate; the mutating commands now serialize on a cross-process ledger lock and refuse with a typed `windows_ledger_lock` error only when a live writer holds it past the retry budget. (#3780) diff --git a/CONTEXT.md b/CONTEXT.md index 652b62806..9094f16f6 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -585,7 +585,7 @@ The producer half of the async external-job contract (#1164, part of #1105). Def Default-off Capability (`capabilities/live-dom-uat/capability.json`, `role: feature`, `tier: full`, `runtimeCompat.supported: ["*"]`, `activationKey: workflow.live_dom_uat`) that confines browser MCP reach to ONE purpose-built agent (#2856). Owns the boolean key `workflow.live_dom_uat` (default `false`), the agent `gsd-dom-verifier` (`tools:` carries `mcp__chrome-devtools__*` + `mcp__claude-in-chrome__*`, and deliberately NO `Bash` and NO `mcp__playwright__*`), and one `step` hook at `execute:wave:post` (`ref.agent`, `fragment: fragments/execute-wave-post.md`, `produces: DOM-VERIFY.md`, `consumes: PLAN.md`, `when: workflow.live_dom_uat`, `onError: skip`) — additive per `RULESET.CAPABILITY.step-additive-gate-blocks`; `gates: []`, it can never halt a wave. **`agents/gsd-executor.md` is NOT widened, in any configuration** — the shape the report proposed and triage refused: for a first-party agent the static `tools:` list is the only control that exists (ADR-1244 D2 forbids a capability granting tools to one; ADR-857 D4's `contribution` injects prose, never permissions; there is no per-dispatch tool override), and ADR-1244 D5's *"there is no sandbox"* governs installed capabilities, not what a spawned subagent does with a granted tool. Containment is TWO independent fail-closed gates: `isCapabilityActive` renders a hook only on `state.active === true` (an installed-but-config-disabled capability renders nothing), plus the step's own `when`. Tool presence alone never activates it — a browser MCP configured for unrelated work is the exact case the default-off key exists for. The orchestrator half extends `gsd-core/workflows/verify-work/steps/automated-ui-verification.md` with a `` block naming both new families AND the key; **the pre-existing `mcp__playwright__*` branch keeps its prior gating (presence + `state:ui-phase-active`) and stays OUTSIDE that block** — pulling it behind a default-off key would silently remove working behavior from every current Playwright-MCP user on upgrade (Hyrum). The glob list is a SHARED CONSTANT across two surfaces (agent frontmatter + workflow block), so `tests/live-dom-uat.test.cjs` carries a parity assertion per `DEFECT.GENERATIVE-FIX-DIVERGENCE`. Landing it also closed a host gap: `execute:wave:post` dispatched only `contribution` + `gate`, so ANY registered `step` was declared and silently never run — `execute-phase.md` step 5.75 now dispatches every `kind == "step"` per `gsd-core/references/loop-hook-dispatch.md`, the same single-kind hand-roll that document names. **Browser-profile lock is tolerated, never coordinated**: `chrome-devtools-mcp` holds an exclusive lock on `$HOME/.cache/chrome-devtools-mcp/chrome-profile` and `--isolated` is a flag on the OPERATOR's own MCP-server registration that GSD neither launches nor parameterizes, so the verifier reports `could_not_look`/`profile_locked`, names the flag, and stops — no retry, no wait, no lease manager over a resource GSD does not own. `DOM-VERIFY.md` frontmatter is scalars only with a closed reason enum (`ok | no_criteria | no_browser_mcp | profile_locked | target_unreachable`) under a closed `outcome` (`verified | nothing_to_report | could_not_look`); **`nothing_to_report` and `could_not_look` are never conflated** — a report claiming "no issues" that never opened a browser is the ambiguous-run-notes defect the issue was filed about. Test seam: `tests/live-dom-uat.test.cjs`. Docs: `docs/how-to/enable-live-dom-verification.md`, `docs/explanation/live-dom-uat-capability.md`. ### Broken Windows Ledger -The enforced cross-phase defect register operationalizing GSD's no-defer discipline as a tracked artifact (#1950). Markdown file at `.planning/WINDOWS.md` (project-level, cross-phase) with YAML frontmatter carrying scalar counts (`schema_version`, `open_count`, `waived_count`, `fixed_count`, `total_count`, `last_updated`) for the FAST path the gate reads via jq without parsing JSON, plus a JSON code block as the AUTHORITATIVE entries source; the two cross-check and fail closed on drift. The rendered markdown table is a THIRD projection of that same source and is cross-checked the same way, but at the WRITE seam rather than the read seam (#3689): `writeLedgerAtomic` compares the on-disk table against `renderTable()` and refuses with `WINDOWS_LEDGER_TABLE_DRIFT` before writing, so a hand-edited cell is never silently reverted and a table-only row is never silently erased. Deliberately NOT enforced in `parseLedger`: hardening the read would break `windows status` and the ship gate on exactly the ledgers an operator needs to inspect. Each entry: `{ id, kind, phase, file, line, description, status, reason, recorded_at, resolved_at }`; kinds are closed (`stub | todo | fixme | skipped-test | lint-warning | unmet-truth | unrun-verify | deviation`); statuses are closed (`open | waived | fixed`). The `broken-windows` Capability (`capabilities/broken-windows/capability.json`) registers one `ship:pre` gate with predicate `artifact-frontmatter-equals WINDOWS.md open_count == 0`; federated config key `workflow.windows_enforce` (default `false` — opt-in enforcement, tracking-only by default so a project can adopt the ledger before turning the gate on). Population is best-effort and never blocks execution: `agents/gsd-executor.md` appends stubs/skipped-tests/unrun-verifies via `gsd_run windows append` after writing SUMMARY.md. Source of truth: `src/broken-windows.cts` → `gsd-core/bin/lib/broken-windows.cjs` (pure `parseLedger`/`renderLedger`/`appendWindow`/`markWaived`/`markFixed` + I/O `cmdWindowsStatus`/`Append`/`Waive`/`MarkFixed`); CLI surface `gsd-tools windows status|append|waive|fixed`. Ship gate enforcement is a `capId == "broken-windows"` named specialization inside `gsd-core/workflows/ship.md` preflight's generic `kind == "gate"` dispatch loop (sibling to the `security` specialization; #3559 made that loop generic, so every OTHER capability's `ship:pre` gate is now evaluated through `gsd_run check predicate` instead of being resolved and silently dropped, while these two keep their bespoke fail-closed reads and are each visited exactly once); it reads `gsd_run windows status --raw` and fails closed on a non-zero/non-numeric `open_count` (an unparseable ledger is itself a broken window). `/gsd:progress` surfaces the open+waived count. The ledger is optional and backward-compatible: a project with no `.planning/WINDOWS.md` reports `open_count: 0` and ships cleanly, and with `workflow.windows_enforce=false` (the default) ship never blocks on it. Frozen `REASON` enum: `WINDOWS_LEDGER_MISSING | WINDOWS_LEDGER_MALFORMED | WINDOWS_LEDGER_TABLE_DRIFT | WINDOWS_ID_NOT_FOUND | WINDOWS_ALREADY_RESOLVED | WINDOWS_WAIVE_REASON_EMPTY | WINDOWS_INVALID_KIND | WINDOWS_INVALID_FILE | WINDOWS_INVALID_ID | WINDOWS_APPEND_MISSING_FIELD | WINDOWS_USAGE | WINDOWS_OK` — surfaced through `--json-errors` for typed test assertions. Test seam: `tests/broken-windows.test.cjs`. Origin: *The Pragmatic Programmer* Topic 3 (Hunt & Thomas — software transplant of Wilson & Kelling's broken-windows metaphor) plus Cunningham's debt metaphor (decay accrues interest ⇒ accounting, not just habit). +The enforced cross-phase defect register operationalizing GSD's no-defer discipline as a tracked artifact (#1950). Markdown file at `.planning/WINDOWS.md` (project-level, cross-phase) with YAML frontmatter carrying scalar counts (`schema_version`, `open_count`, `waived_count`, `fixed_count`, `total_count`, `last_updated`) for the FAST path the gate reads via jq without parsing JSON, plus a JSON code block as the AUTHORITATIVE entries source; the two cross-check and fail closed on drift. The rendered markdown table is a THIRD projection of that same source and is cross-checked the same way, but at the WRITE seam rather than the read seam (#3689): `writeLedgerAtomic` compares the on-disk table against `renderTable()` and refuses with `WINDOWS_LEDGER_TABLE_DRIFT` before writing, so a hand-edited cell is never silently reverted and a table-only row is never silently erased. Deliberately NOT enforced in `parseLedger`: hardening the read would break `windows status` and the ship gate on exactly the ledgers an operator needs to inspect. Each entry: `{ id, kind, phase, file, line, description, status, reason, recorded_at, resolved_at }`; kinds are closed (`stub | todo | fixme | skipped-test | lint-warning | unmet-truth | unrun-verify | deviation`); statuses are closed (`open | waived | fixed`). The `broken-windows` Capability (`capabilities/broken-windows/capability.json`) registers one `ship:pre` gate with predicate `artifact-frontmatter-equals WINDOWS.md open_count == 0`; federated config key `workflow.windows_enforce` (default `false` — opt-in enforcement, tracking-only by default so a project can adopt the ledger before turning the gate on). Population is best-effort and never blocks execution: `agents/gsd-executor.md` appends stubs/skipped-tests/unrun-verifies via `gsd_run windows append` after writing SUMMARY.md. Source of truth: `src/broken-windows.cts` → `gsd-core/bin/lib/broken-windows.cjs` (pure `parseLedger`/`renderLedger`/`appendWindow`/`markWaived`/`markFixed` + I/O `cmdWindowsStatus`/`Append`/`Waive`/`MarkFixed`); CLI surface `gsd-tools windows status|append|waive|fixed`. Ship gate enforcement is a `capId == "broken-windows"` named specialization inside `gsd-core/workflows/ship.md` preflight's generic `kind == "gate"` dispatch loop (sibling to the `security` specialization; #3559 made that loop generic, so every OTHER capability's `ship:pre` gate is now evaluated through `gsd_run check predicate` instead of being resolved and silently dropped, while these two keep their bespoke fail-closed reads and are each visited exactly once); it reads `gsd_run windows status --raw` and fails closed on a non-zero/non-numeric `open_count` (an unparseable ledger is itself a broken window). `/gsd:progress` surfaces the open+waived count. The ledger is optional and backward-compatible: a project with no `.planning/WINDOWS.md` reports `open_count: 0` and ships cleanly, and with `workflow.windows_enforce=false` (the default) ship never blocks on it. Frozen `REASON` enum: `WINDOWS_LEDGER_MISSING | WINDOWS_LEDGER_MALFORMED | WINDOWS_LEDGER_TABLE_DRIFT | WINDOWS_LEDGER_LOCK | WINDOWS_ID_NOT_FOUND | WINDOWS_ALREADY_RESOLVED | WINDOWS_WAIVE_REASON_EMPTY | WINDOWS_INVALID_KIND | WINDOWS_INVALID_FILE | WINDOWS_INVALID_TEXT | WINDOWS_INVALID_ID | WINDOWS_APPEND_MISSING_FIELD | WINDOWS_USAGE | WINDOWS_OK` — surfaced through `--json-errors` for typed test assertions (`WINDOWS_LEDGER_LOCK`, #3780: the mutating commands serialize on the `.planning/.WINDOWS.lock` cross-process lock, shared with refactor-trigger-command-router's own ledger writers; it fires only when a live writer holds the lock past the bounded retry budget. `WINDOWS_INVALID_TEXT` predates this list and was omitted from it). Test seam: `tests/broken-windows.test.cjs`. Origin: *The Pragmatic Programmer* Topic 3 (Hunt & Thomas — software transplant of Wilson & Kelling's broken-windows metaphor) plus Cunningham's debt metaphor (decay accrues interest ⇒ accounting, not just habit). ### Emitted Artifact Provenance Cross-seam principle (ADR-2719, epic #2719): a committed artifact that is a pure function of the source tree is not reviewable state — it is derived state wearing a review costume, and it must be *attributable* rather than *pinned*. Concept, not a Module: it ships nothing, so it takes no `Module` suffix (follows the `### Resolution Provenance` precedent). Scope is the emitted-artifact family named by `RULESET.EMITTED_ATTRIBUTION`. The principle: every emitted path whose hash moved between `next` HEAD and PR HEAD must be attributable — through a declarative provenance table — to a path the pull request actually changed; unattributable deltas are a hard failure that *names them* rather than an anomaly a reviewer must notice inside 7,500 lines of hex. Totality is enforced, so an emitted path matching no rule fails loudly instead of passing through unattributed. The escape hatch is a commit trailer on the PR's own commits (ADR-3942, superseding ADR-2719 §3), not a committed document — two structurally distinct key spaces (separate maps, closing a latent defect where a growth key could satisfy a hash lookup by naming coincidence and vice versa): `Emitted-Drift-Ack-Hash:` keyed on the emitted path (always contains `/`), `Emitted-Drift-Ack-Growth:` keyed on the bare filename under `gsd-core/workflows/` or `agents/` — key and reason separated by ` — ` (space, em dash, space; split on the FIRST occurrence), deliberately not a flag or env var — a trailer appears in the PR's commit list ONLY when something rippled unexpectedly, so adding one IS the alarm, whereas today 100% of emitted-byte changes touch fixtures and touching them signals nothing. Read from `git log $(git merge-base HEAD)..HEAD` — the SAME merge-base `changedPaths` uses via `git diff base...HEAD`, so the ack set and the change set cannot disagree about which commits are this PR's; a two-dot range would be a defect. "Spent" no longer exists: a merged trailer is out of range by construction, not by computation, since there is no base-side copy to compare against. An uncomputable range (shallow clone) THROWS rather than passing vacuously — zero trailers means a PR needing one fails, a false red rather than a false green. `staleAcks` is retained: a trailer declaring a key nothing consumed is still a hard error, and the message names WHICH key space. This design is the terminus of a chain that began because the ack was PR-lifetime data kept in permanent shared state: the single legacy `tests/emitted-drift-ack.json` was a guaranteed merge-conflict cell (#2789; 5 of 6 conflicting PRs in one open queue collided on it and nothing else); #2914 split it into per-PR fragments under `tests/emitted-drift-acks/`, ending the FILE conflict but not the KEY conflict, since two sources could never name the same path; #3078 found a fully-spent fragment left on `next` still walled off every path it owned until swept; #3842 and #3823 tried automated and hand-authored sweeps and each created the next conflict; #3875's timed sweeper automated the remedy but could not merge its own PRs. ADR-3942 ends the chain by moving the ack off the tree entirely (see `RULESET.EMITTED_ATTRIBUTION`). The same differential machine carries the size ratchet: growth is reported with exact byte deltas and needs the same acknowledgment, so anti-creep survives without pinning a number. Distinguish from the absolute check that remains: `tests/fixtures/install-tree/*.json` stays committed and normally-merged (ADR-2719 §7) because "the installer stopped shipping X" must fail with no attribution reasoning involved. Delivery was phased — #2721 naming + interim merge relief, #2722 provenance table + totality guard, #2723 differential check dual-run beside `golden-install-parity.test.cjs`, #2724 cutover (COMPLETE: the dual-run window observed agreement on real PRs after fixing #2750/#2760, and the golden fixtures/test/generator/merge-driver bridge are now deleted; the differential is the sole gate). The table LANDED in #2722 as `tests/helpers/emitted-provenance.cjs` (19 rules, guarded by `tests/emitted-provenance.test.cjs`); it maps emitted path → repo source and is TOTAL over EVERY emitted path in all 19 manifests — exactly one rule per path, with zero-match, two-match, AND dead-rule (a rule matching nothing) all hard failures, so table rot is loud in both directions. Deliberately NO path/family counts are recorded here: those move with every shipped-content edit, and a hand-maintained number in glossary canon is the exact silent-drift failure this whole seam exists to end. The guard recomputes them from the fixtures on every run — read them from a failure message, never from prose. What IS stable is the rule count, which changes only when a new emitted family or host appears. Note the surface is materially wider than #2722 estimated from `claude.json` alone (its "13 families / 15-20 rules" was a single-runtime sample; the 19-manifest surface spans runtime-specific roots — `.agents/`, `.kimi/hooks/`, `command/`, `agents/subagents/`, `.clinerules/`, `plugins/`, `extensions/`, `.gsd/`, the hermes `skills/gsd/` category and the #69 nested `skills//skills//` layout). Two design invariants carry forward to #2723: emitted SHAPES are hard-coded (deriving them from the installer would make the guard tautological — it would follow any installer change silently), while source PATHS may read a first-party descriptor where that descriptor is the sole declaration (`hostBehaviors.nativePlugin.source`); and attribution is keyed on `(rel, runtime)`, never `rel` alone, because one emitted path has different sources per host (`plugins/gsd-core.js` ← `.opencode/` vs `.kilo/`). Emitted skills attribute to `commands/gsd/*.md`, NEVER the repo `skills/` dir — that dir is itself generated from `commands/gsd` by `scripts/gen-plugin-skills.cjs`, so attributing to it is false attribution that still passes totality. Totality does NOT catch a rule pointing at the WRONG source (the recorded residual); the guard against that is the companion assertion that every attributed source EXISTS in the repo, which caught three real cases while the table was built (Copilot's `.agent.md` rename, Kimi's code-literal `agents/gsd.{yaml,md}` root agent, and Copilot's `hooks/gsd-session.json`). A `sources` entry ending in `/` is a PREFIX, not a file — and prefix matching is SEGMENT-AWARE, so a source of `agents/` must not attribute `agentsfoo/x.md`. The differential check LANDED in #2723 as `tests/helpers/emitted-diff.cjs` (the conservation law, a PURE function — no fs/git/installer/clock) + `tests/helpers/emitted-baseline.cjs` (baseline resolution), guarded by `tests/emitted-attribution.test.cjs`. It ran DUAL beside `golden-install-parity.test.cjs` through the #2723 dual-run window with both green and fixtures untouched; #2724 deleted the golden fixtures/test/generator and the check is now the sole gate. Purity is deliberate and load-bearing: the naive one-big-integration-test shape would need ~38 installer spawns per assertion, so the four failing-first criteria would not in practice get written — which is exactly how a phase ships promised-but-not-built. Buckets are CONSERVED: every moved emitted path lands in exactly one of `attributed | unattributable | acked` (property-tested), and a path the provenance table cannot resolve surfaces as an ERROR rather than a silent skip. Four asymmetries worth knowing: an ADDED emitted key is a ripple too (not just modified ones); `synthesized` paths are exempt but `code-derived` ones are NOT (that is why Phase 2 refused to mark them exempt — exempt means permanently blind); SHRINKAGE needs no ack while growth does (gating shrinkage would punish what the ratchet wants); and a STALE ack — a declared key nothing consumed — is a hard failure (#2789 built spent/live detection against a base-relative document that persisted after merge, requiring `readAckFileAtRef`/`baseAck`/`spentAcks` and a `git show`-vs-`ls-tree` absence check to tell "never explained anything" from "already merged, and reddening `next` and every branching PR when it wasn't told apart" (#2768); ADR-3942 makes staleness structural instead of computed — every trailer read is definitionally THIS diff's, scoped by `git merge-base HEAD`, so there is no base copy to compare against and no re-arm-by-reword mechanic to defend, and the base-document corruption hazard the standalone ack linter existed to guard against is gone with the document). Since ADR-3942 there is exactly ONE ack source and it is not a file: the `Emitted-Drift-Ack-Hash:` / `Emitted-Drift-Ack-Growth:` trailers on the PR's own commits, read by `readAckTrailers` (`tests/helpers/emitted-runtime.cjs`) over `git log $(git merge-base HEAD)..HEAD` and parsed by `parseAckTrailers` (`tests/helpers/emitted-diff.cjs`) into TWO structurally distinct key-space maps — closing the latent defect where one shared `paths` map let a growth key satisfy a hash lookup by naming coincidence. Absent = no acks; a declared key nothing consumed is a hard `staleAcks` error that now names WHICH space; a non-empty reason per key is required — "name them and say why" is the contract (ADR-2719 §3, retained). A key declared twice with conflicting reasons is a hard error; declared twice identically it is deduped. An UNCOMPUTABLE range (shallow clone) THROWS rather than reading as zero acks — the inverse of the fragment guard's vacuous-pass failure, and fail-closed in the safe direction. Baseline is CACHED not committed, keyed on the `next` sha; a stale key is REFUSED, never used — absence fails loudly and gets fixed, whereas staleness produces a confident wrong answer. An explicitly-pointed-at (`GSD_EMITTED_BASELINE`) stale baseline is a hard stop, while a stale CACHE falls through to the in-job build. No baseline-unavailable path may `return` (in `node:test` that is a PASS, not a skip — ADR-2719 §6). Supersedes ADR-2264 §2–§4 and its Amendment; ADR-2264 Phase 1 (`buildParityManifest` and the exclusion constants in `tests/helpers/install-shared.cjs`) is retained and depended upon. diff --git a/src/broken-windows.cts b/src/broken-windows.cts index aeafb5b7a..2860ed7f4 100644 --- a/src/broken-windows.cts +++ b/src/broken-windows.cts @@ -6,7 +6,10 @@ * When `workflow.windows_enforce` is true, `/gsd-ship` blocks while any entry is * `open`; an entry can be `waived` only with a recorded reason or `fixed`. * - * LEAF MODULE — imports ONLY: node:fs, node:path. No other src/ imports. + * LEAF MODULE — imports node:fs + node:path, plus two compiled sibling lib + * modules require()d at runtime: workstream-inventory.cjs (the #4487 + * milestone stamp) and capability-lock.cjs (the #3780 cross-process ledger + * lock). No other src/ imports. * * Storage format (`.planning/WINDOWS.md`): * --- @@ -76,6 +79,11 @@ export const REASON = Object.freeze({ // sole source of truth) at the pre-write seam — refuse rather than silently // reconcile by overwriting the operator's hand-edit or dropping a row. WINDOWS_LEDGER_TABLE_DRIFT: 'windows_ledger_table_drift', + // #3780: the read-compute-write cycle is serialized on a cross-process + // ledger lock; this fires only when another writer held the lock past the + // whole bounded retry budget — a typed, actionable refusal instead of a + // silently-lost mutation reported as success. + WINDOWS_LEDGER_LOCK: 'windows_ledger_lock', }); /** Allowed window kinds. Aligned with the issue's enumerated sources. */ @@ -289,12 +297,14 @@ function nextId(entries: WindowEntry[]): number { * Append a window to the ledger. Assigns the next dense id (max+1), sets * status=open, timestamps via opts.now. * - * Concurrency (issue #1950 review L2): NOT safe for concurrent writers. Two - * parallel `gsd_run windows append` invocations both read the same snapshot, - * both compute the same nextId, both write — the second atomic rename wins - * and the first append (and the entry it added) is silently lost. This is - * acceptable in the current single-executor-per-phase model; document if the - * executor ever gains parallel wave-level append. + * Concurrency (issue #1950 review L2, superseded by #3780): as a PURE + * function this operates on whatever ledger snapshot it is passed and cannot + * see concurrent writers — serialization is the CALLER's job. The I/O entry + * points below (cmdWindowsAppend/Waive/MarkFixed) now discharge that duty by + * holding the cross-process ledger lock across their whole + * read-compute-write cycle, so the previously-documented loss (two parallel + * writers, second rename wins, first entry silently gone) can no longer + * occur through the CLI. */ export function appendWindow( ledger: Ledger, @@ -878,6 +888,98 @@ function ledgerPath(cwd: string): string { return path.join(cwd, '.planning', LEDGER_FILE_NAME); } +// ─── #3780: cross-process ledger mutation lock ───────────────────────────── + +interface LedgerLockHandle { path: string; token: string; dev: number | null; ino: number | null } + +interface LockModule { + acquireLock: ( + lockPath: string, + opts?: { maxAttempts?: number; waitForFresh?: boolean }, + ) => LedgerLockHandle | null; + releaseLock: (handle: LedgerLockHandle | null) => void; +} + +/** + * The SHARED hardened cross-process lock primitive (single source of truth + * for capability-lifecycle + capability-consent, extracted so locks cannot + * diverge). Required LAZILY: capability-lock captures the process start time + * at module load — a `ps` subprocess on macOS, PowerShell on win32 — and + * this module is loaded by lock-free readers (`windows status`, the + * /gsd-ship gate) that must not pay that per-invocation cost (#3780 + * review). `require` is cached, so writers pay it once per process. + */ +let _lockMod: LockModule | null = null; +function ledgerLock(): LockModule { + if (_lockMod === null) { + /* eslint-disable @typescript-eslint/no-require-imports */ + _lockMod = require('./capability-lock.cjs') as LockModule; + /* eslint-enable @typescript-eslint/no-require-imports */ + } + return _lockMod; +} + +/** + * Budget mirrors capability-consent's CONSENT_LOCK_MAX_ATTEMPTS: two + * genuinely-racing writers must SERIALIZE, not fail. The ledger's critical + * section is sub-millisecond and the primitive backs off ~25-50ms per + * attempt, so 50 attempts is orders of magnitude beyond any real contention + * while keeping the worst case (a holder that never releases until the + * primitive's own liveness/deadman protocol reclaims it) bounded at ~2s + * before the typed refusal below. + */ +const LEDGER_LOCK_MAX_ATTEMPTS = 50; + +function ledgerLockPath(cwd: string): string { + return path.join(cwd, '.planning', '.WINDOWS.lock'); +} + +function acquireLedgerLock(cwd: string): LedgerLockHandle | null { + return ledgerLock().acquireLock(ledgerLockPath(cwd), { + maxAttempts: LEDGER_LOCK_MAX_ATTEMPTS, + // A contended fresh/live holder is WAITED FOR (back off + retry), not + // failed-fast — racing wave-level executors serialize (issue #3780). + waitForFresh: true, + }); +} + +function releaseLedgerLock(handle: LedgerLockHandle | null): void { + ledgerLock().releaseLock(handle); +} + +/** + * Run `fn` (a full ledger read-compute-write cycle) while holding the + * cross-process ledger lock. Throws a typed WindowsError — never falls back + * to an unlocked mutation — when the lock cannot be acquired within the + * budget, mirroring capability-consent finding 3: a locked store must refuse + * the write rather than silently race for it. Readers (cmdWindowsStatus, the + * ship gate) deliberately do NOT take this lock: the atomic rename already + * gives them a whole-file snapshot. + * + * EXPORTED (#3780) because `withLedgerLock` is the ONE serialization seam + * for WINDOWS.md: every writer of the ledger — the cmd* entry points here + * and any sibling module with its own read-compute-write cycle on the same + * file (refactor-trigger-command-router's strict-window record/resolve) — + * must hold this lock, or the lost-update race #3780 fixed survives on that + * path. + */ +export function withLedgerLock(cwd: string, fn: () => T): T { + const handle = acquireLedgerLock(cwd); + if (!handle) { + throw new WindowsError( + REASON.WINDOWS_LEDGER_LOCK, + `Another writer holds the ledger lock at ${ledgerLockPath(cwd)}; WINDOWS.md ` + + 'mutations are serialized per project. Re-run the command once the other ' + + 'writer finishes — the lock is reclaimed automatically if its holder died.', + ); + } + try { + return fn(); + } finally { + releaseLedgerLock(handle); + } +} + function readLedgerOrNull(cwd: string): Ledger | null { const p = ledgerPath(cwd); let raw: string; @@ -1129,37 +1231,43 @@ export function cmdWindowsAppend( required: ['--kind', '--phase', '--description'], }); - let ledger: Ledger; - try { - ledger = readLedgerOrNull(cwd) ?? emptyLedger(nowIso()); - } catch (e) { - if (e instanceof WindowsError) throw e; - throw new WindowsError(REASON.WINDOWS_LEDGER_MALFORMED, (e as Error).message); - } + // #3780: the whole read-compute-write cycle — snapshot, milestone stamp, + // id allocation, atomic rename — holds the ledger lock, so two parallel + // invocations can no longer compute the same nextId from the same snapshot + // and silently lose the first append to the second rename. + withLedgerLock(cwd, () => { + let ledger: Ledger; + try { + ledger = readLedgerOrNull(cwd) ?? emptyLedger(nowIso()); + } catch (e) { + if (e instanceof WindowsError) throw e; + throw new WindowsError(REASON.WINDOWS_LEDGER_MALFORMED, (e as Error).message); + } - // #4487: stamp the workstream's resolved milestone at record time -- the - // same STATE.md-first, ROADMAP-fallback resolution workstream-inventory.cts - // already uses. Best-effort: an unreadable/missing STATE.md or ROADMAP.md - // resolves to null, same as an entry recorded before this field existed. - const milestone = workstreamInventory.readCurrentMilestoneVersion( - path.join(cwd, '.planning', 'STATE.md'), - path.join(cwd, '.planning', 'ROADMAP.md'), - ); + // #4487: stamp the workstream's resolved milestone at record time -- the + // same STATE.md-first, ROADMAP-fallback resolution workstream-inventory.cts + // already uses. Best-effort: an unreadable/missing STATE.md or ROADMAP.md + // resolves to null, same as an entry recorded before this field existed. + const milestone = workstreamInventory.readCurrentMilestoneVersion( + path.join(cwd, '.planning', 'STATE.md'), + path.join(cwd, '.planning', 'ROADMAP.md'), + ); - const result = appendWindow( - ledger, - { - kind: parsed.values['--kind'] as WindowKind, - phase: parsed.values['--phase'] ?? '', - file: parsed.values['--file'] ?? '', - line: parsed.values['--line'] == null ? null : Number(parsed.values['--line']), - description: parsed.values['--description'] ?? '', - milestone, - }, - { now: nowIso() }, - ); - writeLedgerAtomic(cwd, result.ledger); - emit({ ok: true, ledger: result.ledger, entry: result.entry }); + const result = appendWindow( + ledger, + { + kind: parsed.values['--kind'] as WindowKind, + phase: parsed.values['--phase'] ?? '', + file: parsed.values['--file'] ?? '', + line: parsed.values['--line'] == null ? null : Number(parsed.values['--line']), + description: parsed.values['--description'] ?? '', + milestone, + }, + { now: nowIso() }, + ); + writeLedgerAtomic(cwd, result.ledger); + emit({ ok: true, ledger: result.ledger, entry: result.entry }); + }); } /** `gsd-tools windows waive ""`. */ @@ -1175,17 +1283,21 @@ export function cmdWindowsWaive( const id = parseIdOrThrow(idStr); - let ledger: Ledger; - try { - ledger = readLedgerOrNull(cwd) ?? emptyLedger(nowIso()); - } catch (e) { - if (e instanceof WindowsError) throw e; - throw new WindowsError(REASON.WINDOWS_LEDGER_MALFORMED, (e as Error).message); - } + // #3780: same serialization as append — a concurrent append holding a stale + // snapshot would otherwise overwrite the waive and resurrect the entry. + withLedgerLock(cwd, () => { + let ledger: Ledger; + try { + ledger = readLedgerOrNull(cwd) ?? emptyLedger(nowIso()); + } catch (e) { + if (e instanceof WindowsError) throw e; + throw new WindowsError(REASON.WINDOWS_LEDGER_MALFORMED, (e as Error).message); + } - const updated = markWaived(ledger, id, reason ?? '', { now: nowIso() }); - writeLedgerAtomic(cwd, updated); - emit({ ok: true, ledger: updated }); + const updated = markWaived(ledger, id, reason ?? '', { now: nowIso() }); + writeLedgerAtomic(cwd, updated); + emit({ ok: true, ledger: updated }); + }); } /** `gsd-tools windows fixed `. */ @@ -1198,17 +1310,21 @@ export function cmdWindowsMarkFixed( const { positionals } = parseArgs(args, { flags: [], required: [], positionals: 1 }); const id = parseIdOrThrow(positionals[0]); - let ledger: Ledger; - try { - ledger = readLedgerOrNull(cwd) ?? emptyLedger(nowIso()); - } catch (e) { - if (e instanceof WindowsError) throw e; - throw new WindowsError(REASON.WINDOWS_LEDGER_MALFORMED, (e as Error).message); - } + // #3780: same serialization as append — a concurrent writer holding a + // stale snapshot would otherwise overwrite the resolved status. + withLedgerLock(cwd, () => { + let ledger: Ledger; + try { + ledger = readLedgerOrNull(cwd) ?? emptyLedger(nowIso()); + } catch (e) { + if (e instanceof WindowsError) throw e; + throw new WindowsError(REASON.WINDOWS_LEDGER_MALFORMED, (e as Error).message); + } - const updated = markFixed(ledger, id, { now: nowIso() }); - writeLedgerAtomic(cwd, updated); - emit({ ok: true, ledger: updated }); + const updated = markFixed(ledger, id, { now: nowIso() }); + writeLedgerAtomic(cwd, updated); + emit({ ok: true, ledger: updated }); + }); } function parseIdOrThrow(raw: string | undefined): number { diff --git a/src/refactor-trigger-command-router.cts b/src/refactor-trigger-command-router.cts index ccb5bb208..8ffa04e52 100644 --- a/src/refactor-trigger-command-router.cts +++ b/src/refactor-trigger-command-router.cts @@ -406,6 +406,36 @@ function loadWindowsOrDegrade( return { ok: true, windows, ledger }; } +/** + * #3780: lazy (module-load cost, see file header) require of the ledger's + * serialization seam — the same `.planning/.WINDOWS.lock` the windows cmd* + * writers hold, so this router's own read-compute-write cycles on + * WINDOWS.md cannot lose updates against them. Required from the REAL + * compiled module, never through the injectable `windowsOverride` seam: + * the lock is infrastructure, not a parser stand-in. + */ +function withLedgerLock(cwd: string, fn: () => T): T { + let lockMod: { + withLedgerLock: (cwd: string, fn: () => T) => T; + }; + try { + /* eslint-disable @typescript-eslint/no-require-imports */ + lockMod = require('./broken-windows.cjs') as { + withLedgerLock: (cwd: string, fn: () => T) => T; + }; + /* eslint-enable @typescript-eslint/no-require-imports */ + } catch { + // Ledger module unavailable (the #1953 degrade world — see the row-86 + // notesLedgerUnavailableWithoutBrokenWindows contract): no lock-holding + // writer can exist either, because every WINDOWS.md writer requires this + // same module. Run the body unlocked and let loadWindowsOrDegrade + // produce the canonical degrade note — wrapping that world in lock + // ceremony would only rewrite the note the degrade contract pins. + return fn(); + } + return lockMod.withLedgerLock(cwd, fn); +} + /** * Strict-mode window append (step 9 of `evaluate`). Degrades to * `{ recorded: false, note }` per `loadWindowsOrDegrade` — never an error, @@ -418,6 +448,28 @@ function recordStrictWindow( padded: string, target: Candidate, windowsOverride: WindowsModule | undefined, +): { recorded: boolean; note?: string } { + // #3780: hold the same cross-process ledger lock the windows cmd* writers + // hold — this site's read-compute-write on WINDOWS.md is otherwise the + // same lost-update race: a concurrent `gsd_run windows append` (or another + // evaluator) could silently overwrite this entry, or be overwritten by + // it. The lock comes from the real ledger module, not the injectable + // `windowsOverride` seam — it is infrastructure, not a parser stand-in. + // The wrapper keeps the #1953-defect-2 degrade contract: the lock's typed + // refusal degrades to `{ recorded: false, note }` like every other + // failure, it never throws out of this function. + try { + return withLedgerLock(cwd, () => recordStrictWindowLocked(cwd, padded, target, windowsOverride)); + } catch (e) { + return { recorded: false, note: `failed to record broken-windows entry: ${e instanceof Error ? e.message : String(e)}` }; + } +} + +function recordStrictWindowLocked( + cwd: string, + padded: string, + target: Candidate, + windowsOverride: WindowsModule | undefined, ): { recorded: boolean; note?: string } { const loaded = loadWindowsOrDegrade(cwd, windowsOverride); if (!loaded.ok) return { recorded: false, note: loaded.note }; @@ -460,6 +512,26 @@ function resolveLedgerWindow( kind: 'accept' | 'decline', reasonText: string, windowsOverride: WindowsModule | undefined, +): { resolved: boolean; note?: string } { + // #3780: same serialization as recordStrictWindow — a concurrent ledger + // writer holding a stale snapshot would silently revert this resolve (or + // lose its own write to this one). Same degrade contract: the lock's + // typed refusal degrades, never throws. + try { + return withLedgerLock(cwd, () => resolveLedgerWindowLocked(cwd, padded, file, line, kind, reasonText, windowsOverride)); + } catch (e) { + return { resolved: false, note: `failed to resolve broken-windows entry: ${e instanceof Error ? e.message : String(e)}` }; + } +} + +function resolveLedgerWindowLocked( + cwd: string, + padded: string, + file: string, + line: number, + kind: 'accept' | 'decline', + reasonText: string, + windowsOverride: WindowsModule | undefined, ): { resolved: boolean; note?: string } { const loaded = loadWindowsOrDegrade(cwd, windowsOverride); if (!loaded.ok) return { resolved: false, note: loaded.note }; diff --git a/tests/broken-windows.test.cjs b/tests/broken-windows.test.cjs index 9716e0432..b3c2e3279 100644 --- a/tests/broken-windows.test.cjs +++ b/tests/broken-windows.test.cjs @@ -24,11 +24,20 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const { spawn } = require('node:child_process'); -const { createTempDir, cleanup, runGsdTools } = require('./helpers.cjs'); +const { + createTempDir, + cleanup, + runGsdTools, + TOOLS_PATH, + TEST_ENV_BASE, +} = require('./helpers.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const fc = require('./helpers/fast-check-setup.cjs'); const brokenWindowsLib = require('../gsd-core/bin/lib/broken-windows.cjs'); +const lockMod = require('../gsd-core/bin/lib/capability-lock.cjs'); const { REASON, WindowsError, @@ -1773,3 +1782,223 @@ describe('#1950-H2 / #3689: writeLedgerAtomic pre-image read failure', () => { ); }); }); + +// --------------------------------------------------------------------------- +// #3780: parallel writers must not silently lose ledger mutations +// +// cmdWindowsAppend/Waive/MarkFixed each ran an unlocked read-compute-write +// cycle ending in an atomic rename: two parallel invocations read the same +// snapshot, computed the same nextId, and the second rename won — the first +// mutation was silently lost while its invocation still reported ok:true +// (a false-green /gsd-ship gate, since the ship decision reads open_count). +// The fix serializes the mutating commands on a `.planning/.WINDOWS.lock` +// ledger lock backed by the shared capability-lock primitive (the +// capability-consent precedent: waitForFresh + raised budget, typed throw +// when the lock cannot be acquired). Readers stay lock-free. +// --------------------------------------------------------------------------- + +describe('#3780: parallel writers serialize on the ledger lock', () => { + const lockRelPath = path.join('.planning', '.WINDOWS.lock'); + + /** + * The sync process seam cannot interleave two writers in one thread, and a + * concurrency regression needs genuinely concurrent children. Async spawn, + * bounded by the CLI-probe class timeout, env built exactly the way + * runGsdTools builds it (process env + TEST_ENV_BASE). + */ + function spawnAppend(tmp, description) { + return new Promise((resolve) => { + const child = spawn( + process.execPath, + [TOOLS_PATH, 'windows', 'append', '--kind', 'todo', '--phase', '1', '--description', description], + { cwd: tmp, env: { ...process.env, ...TEST_ENV_BASE } }, + ); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (d) => { stdout += d; }); + child.stderr.on('data', (d) => { stderr += d; }); + const timer = setTimeout(() => child.kill('SIGKILL'), PROBE_TIMEOUT_MS); + child.on('close', (code) => { + clearTimeout(timer); + resolve({ code, stdout, stderr }); + }); + }); + } + + function readEntries(tmp) { + const raw = fs.readFileSync(path.join(tmp, '.planning', LEDGER_FILE_NAME), 'utf8'); + return parseLedger(raw).entries; + } + + /** Acquire the ledger lock as a stand-in live writer (same-host, never stolen). */ + function acquireHeldLock(tmp) { + const handle = lockMod.acquireLock(path.join(tmp, lockRelPath), { maxAttempts: 1 }); + assert.ok(handle, 'test setup: the test process must be able to acquire the ledger lock'); + return handle; + } + + test('two concurrent gsd-tools append processes both land their entries', async (t) => { + const tmp = createTempDir('bw-3780-race-'); + t.after(() => cleanup(tmp)); + + const [a, b] = await Promise.all([ + spawnAppend(tmp, 'writer-A'), + spawnAppend(tmp, 'writer-B'), + ]); + + assert.equal(a.code, 0, `writer-A must exit 0, stderr: ${a.stderr}`); + assert.equal(b.code, 0, `writer-B must exit 0, stderr: ${b.stderr}`); + assert.equal(JSON.parse(a.stdout).ok, true, 'writer-A must observe success'); + assert.equal(JSON.parse(b.stdout).ok, true, 'writer-B must observe success'); + + const entries = readEntries(tmp); + assert.equal(entries.length, 2, 'both appends must be present in the ledger'); + assert.deepEqual(entries.map((e) => e.id).sort(), [1, 2], 'ids must be distinct — no shared nextId'); + assert.deepEqual( + entries.map((e) => e.description).sort(), + ['writer-A', 'writer-B'], + 'neither description may be lost', + ); + }); + + test('append refuses typed while another writer holds the ledger lock, and proceeds after release', (t) => { + const tmp = createTempDir('bw-3780-held-append-'); + t.after(() => cleanup(tmp)); + const handle = acquireHeldLock(tmp); + t.after(() => lockMod.releaseLock(handle)); + + const res = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '1', '--description', 'blocked writer'], + tmp, + { GSD_JSON_ERRORS: '1' }, + ); + assert.equal(res.success, false, 'append must refuse while the ledger lock is held by a live writer'); + const parsed = JSON.parse(res.error); + // String literal, not REASON.WINDOWS_LEDGER_LOCK — the constant does not + // exist on the pre-fix module and `undefined === undefined` would pass + // vacuously (the #3689 precedent at the table-drift assertion). + assert.equal(parsed.reason, 'windows_ledger_lock', `expected typed lock reason, got: ${res.error}`); + assert.equal( + fs.existsSync(path.join(tmp, '.planning', LEDGER_FILE_NAME)), + false, + 'a refused append must not write the ledger', + ); + + // After release the same mutation proceeds — the refusal was contention, + // not corruption. + lockMod.releaseLock(handle); + const res2 = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '1', '--description', 'after release'], + tmp, + ); + assert.equal(res2.success, true, `post-release append must succeed: ${res2.error || ''}`); + assert.equal(JSON.parse(res2.output).entry.description, 'after release'); + }); + + test('append releases the ledger lock after success', (t) => { + const tmp = createTempDir('bw-3780-release-ok-'); + t.after(() => cleanup(tmp)); + const res = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '1', '--description', 'solo'], + tmp, + ); + assert.equal(res.success, true, `stderr: ${res.error || ''}`); + assert.equal( + fs.existsSync(path.join(tmp, lockRelPath)), + false, + 'no lock file may survive a successful append', + ); + }); + + test('append releases the ledger lock even when the append itself fails', (t) => { + const tmp = createTempDir('bw-3780-release-fail-'); + t.after(() => cleanup(tmp)); + const bad = runGsdTools( + ['windows', 'append', '--kind', 'no-such-kind', '--phase', '1', '--description', 'x'], + tmp, + ); + assert.equal(bad.success, false, 'invalid kind must fail as before'); + assert.equal( + fs.existsSync(path.join(tmp, lockRelPath)), + false, + 'no lock file may survive a failed append', + ); + const good = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '1', '--description', 'retry'], + tmp, + ); + assert.equal(good.success, true, `a valid append after a failed one must proceed: ${good.error || ''}`); + }); + + test('waive refuses typed while the ledger lock is held and proceeds after release', (t) => { + const tmp = createTempDir('bw-3780-held-waive-'); + t.after(() => cleanup(tmp)); + const seed = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '1', '--description', 'entry'], + tmp, + ); + assert.equal(seed.success, true, `seed append failed: ${seed.error || ''}`); + const handle = acquireHeldLock(tmp); + t.after(() => lockMod.releaseLock(handle)); + + const res = runGsdTools(['windows', 'waive', '1', 'waiver reason'], tmp, { GSD_JSON_ERRORS: '1' }); + assert.equal(res.success, false, 'waive must refuse while the ledger lock is held'); + const parsed = JSON.parse(res.error); + assert.equal(parsed.reason, 'windows_ledger_lock', `expected typed lock reason, got: ${res.error}`); + + lockMod.releaseLock(handle); + const res2 = runGsdTools(['windows', 'waive', '1', 'waiver reason'], tmp); + assert.equal(res2.success, true, `post-release waive must succeed: ${res2.error || ''}`); + }); + + test('fixed refuses typed while the ledger lock is held', (t) => { + const tmp = createTempDir('bw-3780-held-fixed-'); + t.after(() => cleanup(tmp)); + const seed = runGsdTools( + ['windows', 'append', '--kind', 'todo', '--phase', '1', '--description', 'entry'], + tmp, + ); + assert.equal(seed.success, true, `seed append failed: ${seed.error || ''}`); + const handle = acquireHeldLock(tmp); + t.after(() => lockMod.releaseLock(handle)); + + const res = runGsdTools(['windows', 'fixed', '1'], tmp, { GSD_JSON_ERRORS: '1' }); + assert.equal(res.success, false, 'fixed must refuse while the ledger lock is held'); + const parsed = JSON.parse(res.error); + assert.equal(parsed.reason, 'windows_ledger_lock', `expected typed lock reason, got: ${res.error}`); + }); + + test('status stays lock-free — reads do not block on the writer lock', (t) => { + const tmp = createTempDir('bw-3780-status-free-'); + t.after(() => cleanup(tmp)); + const handle = acquireHeldLock(tmp); + t.after(() => lockMod.releaseLock(handle)); + const res = runGsdTools(['windows', 'status', '--raw'], tmp); + assert.equal(res.success, true, `status must not take the writer lock: ${res.error || ''}`); + assert.equal(JSON.parse(res.output).ledger.open_count, 0); + }); + + test('REASON enum gains WINDOWS_LEDGER_LOCK and stays frozen+closed', () => { + assert.equal(Object.isFrozen(REASON), true); + // Assert on the VALUES (the wire codes --json-errors can emit), not + // Object.keys — the keys are the UPPER_CASE identifiers. Closure over + // all 14 codes is the contract: adding/removing a code must update this + // list in the same commit (the three-coordinated-changes rule). + assert.deepEqual(Object.values(REASON).sort(), [ + 'windows_already_resolved', + 'windows_append_missing_field', + 'windows_id_not_found', + 'windows_invalid_file', + 'windows_invalid_id', + 'windows_invalid_kind', + 'windows_invalid_text', + 'windows_ledger_lock', + 'windows_ledger_malformed', + 'windows_ledger_missing', + 'windows_ledger_table_drift', + 'windows_ok', + 'windows_usage', + 'windows_waive_reason_empty', + ]); + }); +}); diff --git a/tests/refactor-trigger-cli.test.cjs b/tests/refactor-trigger-cli.test.cjs index 76d6302d7..efb1b0ae4 100644 --- a/tests/refactor-trigger-cli.test.cjs +++ b/tests/refactor-trigger-cli.test.cjs @@ -46,6 +46,7 @@ const { } = require('../gsd-core/bin/lib/complexity-trigger.cjs'); const gitBaseBranch = require('../gsd-core/bin/lib/git-base-branch.cjs'); const windowsModule = require('../gsd-core/bin/lib/broken-windows.cjs'); +const lockModule = require('../gsd-core/bin/lib/capability-lock.cjs'); const { routeRefactorTriggerCommand } = require('../gsd-core/bin/lib/refactor-trigger-command-router.cjs'); const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); const { validateCapability, VALID_LOOP_POINTS } = require('../gsd-core/bin/lib/capability-validator.cjs'); @@ -654,6 +655,36 @@ describe('refactor-trigger: disposition + ledger', () => { // ─── Rows 80-86 — strict mode -> broken-windows ledger ─────────────────────── +describe('refactor-trigger router: locked-ledger degrade (#3780)', () => { + test('evaluate degrades ledger_recorded to false with a note when a writer holds the ledger lock', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-3780-lock-', true); + t.after(() => cleanup(dir)); + const lockPath = path.join(dir, '.planning', '.WINDOWS.lock'); + const handle = lockModule.acquireLock(lockPath, { maxAttempts: 1 }); + assert.ok(handle, 'test setup: the test process must be able to acquire the ledger lock'); + t.after(() => lockModule.releaseLock(handle)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0, 'evaluate must degrade, never fail, on lock contention'); + const parsed = parseStdout(result); + assert.strictEqual(parsed.artifact_written, true, 'the local proposal artifact is independent of the ledger'); + assert.strictEqual(parsed.ledger_recorded, false); + assert.match(parsed.ledger_note, /ledger lock/, `note must name the contention: ${parsed.ledger_note}`); + assert.strictEqual( + fs.existsSync(path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME)), + false, + 'a degraded record must not write the ledger', + ); + + // After release the same evaluation records — the refusal was contention. + lockModule.releaseLock(handle); + const result2 = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result2.exitCode, 0); + assert.strictEqual(parseStdout(result2).ledger_recorded, true); + }); +}); + + describe('refactor-trigger: strict mode -> broken-windows ledger', () => { test('appendsNoWindowInAdvisoryMode', (t) => { const dir = setupTriggeringProject('gsd-refactor-cli-80-', false);