Serialize the audit state owner's writes so concurrent invocations cannot corrupt the record

.changeset/issue-1040-serialize-audit-state-writes.md

@@ -0,0 +1,7 @@
+---
+bump: patch
+type: Fixed
+---
+
+- **Serialize the create-issue audit state owner's writes so concurrent invocations cannot corrupt the record.** `scripts/issue-audit-state.py` now wraps every mutating subcommand in an exclusive-create sentinel critical section (a `.lock` file beside the state document, created with `os.O_CREAT | os.O_EXCL`), so two concurrent invocations for the same slug produce a document reflecting one of them entirely and then the other, never a mixture. `save_state` obtains a unique per-writer temporary path from `tempfile.mkstemp` (retaining the `.json.tmp` suffix) and retries `os.replace` over `PermissionError`, so two writers never share and truncate one temporary file. Read-only subcommands (`query-*`, `emit-body`, `check-claim-staleness`) acquire no sentinel and stay unserialized; an abandoned sentinel is recovered by age. A new fail-closed transitive call-graph check in `lib/test/check-audit-lifecycle-contracts.py` proves `save_state` is unreachable from every read-only-classified subcommand. The mechanism is standard-library only (no `fcntl`/`msvcrt`), adds no state-document field, and produces no new mutation-exit class — every section failure is a `could not persist state to …` condition the shipped routing already carries. The decision channel (`next_call=` / `query-*`) remains unserialized, and its non-authoritative-under-concurrency residual is stated in `docs/DEVFLOW_SYSTEM_OVERVIEW.md` §11. (#1045)
+- **#1004 migration note.** This change adds two test-only environment variables in the frozen `DEVFLOW_` namespace — `DEVFLOW_IAS_ACQUIRE_WINDOW_S` and `DEVFLOW_IAS_STALE_AFTER_S` — which the Tier-3 env-var rename (#1004) must migrate alongside the existing `DEVFLOW_*` members. (#1045)

docs/DEVFLOW_SYSTEM_OVERVIEW.md

@@ -635,7 +635,7 @@ The skill exists to prevent "option-listing" issues. Steps:
 4. **Self-steelman (Step 3.5, mandatory — issue #304):** before the user sees the draft, the skill stress-tests it against the *actual code* — fresh targeted reads/greps, never the ambient context the draft came from. It verifies every load-bearing claim, file reference, and acceptance criterion; runs a **universal-quantifier sweep** (every "never/always/each/every/all/cannot" grounded — pinned per-arm, scoped, or removed — with a planted-defect positive-control obligation on detector-coverage claims); grounds every **occurrence count and coupled-site list** by an executed whitespace-normalized search or the cited evidence records, never recall; reads a "code does X" premise with its **enclosing gates/conditionals and their defaults**; hunts for missed ACs, missed edge cases, wrong assumptions, unstated scope, and an unreconciled multi-state contract (a within-text check that no summary or table form lists fewer causes for a state than the per-state ACs specify); it also flags an **AC mutual-consistency** conflict (no AC forbids a surface another AC's discharge must touch) and a **trust-boundary closure** gap (a protected executable-artifact set defined over the transitive source/exec/import closure of its entry points); binds every **stated-but-unbound input** (a mechanism input named only by role) to a named referent with a cited code reference or an implementer-obligation AC; revises the draft itself and re-runs the no-options gate on the revision; then — at every revision event, per the shared **Revision-delta verification** procedure (stated once, referenced by every revise-and-re-gate site) — walks the revision's edit-batch delta across six classes (mechanisms, lifecycle rules, execution-tier assumptions, dependencies, universal guarantees, and a total-making residual class) and verifies each non-empty class against the code, so audit rounds and the declined-re-audit filing path see delta that has been walked and verified rather than only language-gated; and reports a one-line outcome summary either way, including the universal-quantifier sweep's zero arm (a silent pass is indistinguishable from a skipped step). A genuinely new decision fork routes through the existing Step 2 question machinery, not a new path.
 5. **Fresh-context audit (Step 3.6, mandatory — issue #443, extended by #522):** after Step 3.5 passes (which now also **self-checks the draft against the same audit dimension checklist** before any dispatch) and before the user sees the draft, the skill dispatches **one synchronous audit subagent** whose entire value is that it did **not** draft the issue — the mechanism is *information removal* (separated-context critics outperform same-session self-review on critical errors). On the normal **file arm**, the orchestrator writes the current rendered draft to the canonical `issue-draft-<slug>.md` before each round and the auditor **reads that file as the sole draft source** (closing the condensation-drift channel a hand-embedded copy opened) with a carriage/identity check; the drafting conversation, Step 1 findings, and the *reasoning* artifacts stay out of bounds while the draft file itself is the artifact under audit. When the write fails (read-only sandbox), a fallback **embed arm** carries the full body verbatim with its own sentinel carriage check, and the on-disk draft path stays out of bounds. The auditor runs an **adversarial pre-mortem** audit-prompt: a per-finding bar, scope exclusions at issue altitude, one assessed "Quiet Killer" slot, no cap on the number of findings paired with a per-finding length discipline, and a mandatory verdict line with three legal values — `VERDICT: FILE`, `VERDICT: REVISE`, or `VERDICT: DRAFT-UNREADABLE` (file arm only). On `REVISE` the orchestrator verifies each finding against the code, revises, re-runs the no-options gate, runs the shared **Revision-delta verification** procedure over the revision's delta (the same procedure the Step 3.5 and Step 4 revise loops reference), and re-audits **at most once** automatically — the audit informs, it never deadlocks filing. Past that automatic budget the skill **offers user-chosen rounds** (up to 3) via the question tool whenever the run is demonstrably unconverged — the user, not the skill, spends the tokens. Every accepted round's findings are **adjudicated** into must-revise / advisory / invalid; since issue #743 the advisory and invalid grades are no longer bare counts but carry a **durable per-finding record** — a one-line summary and rationale, an impact-class tag, and the auditor's returned finding block byte-preserved — recorded through the state owner (`record-adjudication --advisory-records-file/--invalid-records-file`, refused when a count and its records disagree), read back with `query-adjudication-records`, and rendered to the user **before** the approval election. A **calibration** layer (`query-calibration`, a `calibration=` sibling of the coverage boundary offer) surfaces an advisory grade on an impact-bearing finding (`implementation-correctness`/`scope`/`safety`/`verifiability`) that carries no recorded evidence, so an under-evidenced grade is named to the maintainer rather than silently converged past — disclosure only, **never a filing block**. Full evidence record: [`docs/advisory-adjudication-calibration.md`](advisory-adjudication-calibration.md).
 
-**The lifecycle itself is owned by a tested state-owner CLI, not by prose (issue #546).** Every deterministic rule above — transition legality, round numbering, the automatic budget and the bounded retries, arm routing and its three embed markers, digest computation and comparison, sentinel generation and comparison, the offer triggers, override records, presentation eligibility, and the audit-summary field set — is executed by `scripts/issue-audit-state.py`; the skill records lifecycle events through it and **obeys its answers** rather than re-deriving them each turn. Its two-class contract is what the prose branches on: queries always exit 0 once the arguments parse (an argparse usage error still exits 2) with a decided answer line, except for the multi-line read-back queries `query-findings`, issue #704's `query-claim-baselines` / `query-finding-evidence`, `query-coverage`, and `query-adjudication-records`, which print one decided line per record, and issue #795's composite `query-boundary`, which prints one decided line per boundary component; all of them are strictly read-only, while mutations exit non-zero with a named breadcrumb on an illegal transition or an unpersistable state. Since issue #795 most subcommands print a second and final `next_call=` line naming the next legal invocation, with every state-derivable operand filled and every caller-supplied one bare in a `needs=` field; it is a generated suggestion the caller reviews before running, never an instruction, and the decided answer line is unchanged and stays first. The same issue lets the five subcommands whose round the state uniquely determines resolve an omitted `--round` from state, while every subcommand where the flag selects an operation or names a caller-chosen round keeps it required. Run state persists to a cwd/worktree-anchored `.prflow/tmp/issue-audit-state-<slug>.json`, replacing the markdown event log the offer used to read — the skill still writes the observable audit artifact (`.prflow/tmp/issue-audit-<slug>.md`, same gate/read-only-stand-in convention as the derivation artifact), and both state paths plus the **retired** `.md` leftover stay declared out of bounds so an auditor with repository read access cannot re-anchor on this run's prior verdicts. Only the current draft counts as audited: eligibility is grounded on a completed clean-verdict round whose recorded identity still holds — byte-digest equality against the canonical file on file-arm epochs, revision ordering on the embed and inline arms where no trustworthy file exists — or on an explicitly recorded override that no later revision has invalidated, and every `eligible` answer carries a deterministic token bound to the answering digest or revision ordinal (matching the ground that answered) which the summary line quotes verbatim. That **narrows** the prose-compliance gap and makes a skipped eligibility check detectable in the transcript; it does not close it, since no in-process component can force an orchestrator that never invokes it. Where the tool cannot run at all, a named bounded fallback runs one round, asks once, and marks the summary line `state-owner unavailable` — distinct from `degraded`, which keeps its meaning of the inline audit arm.
+**The lifecycle itself is owned by a tested state-owner CLI, not by prose (issue #546).** Every deterministic rule above — transition legality, round numbering, the automatic budget and the bounded retries, arm routing and its three embed markers, digest computation and comparison, sentinel generation and comparison, the offer triggers, override records, presentation eligibility, and the audit-summary field set — is executed by `scripts/issue-audit-state.py`; the skill records lifecycle events through it and **obeys its answers** rather than re-deriving them each turn. Its two-class contract is what the prose branches on: queries always exit 0 once the arguments parse (an argparse usage error still exits 2) with a decided answer line, except for the multi-line read-back queries `query-findings`, issue #704's `query-claim-baselines` / `query-finding-evidence`, `query-coverage`, and `query-adjudication-records`, which print one decided line per record, and issue #795's composite `query-boundary`, which prints one decided line per boundary component; all of them are strictly read-only, while mutations exit non-zero with a named breadcrumb on an illegal transition or an unpersistable state. Since issue #795 most subcommands print a second and final `next_call=` line naming the next legal invocation, with every state-derivable operand filled and every caller-supplied one bare in a `needs=` field; it is a generated suggestion the caller reviews before running, never an instruction, and the decided answer line is unchanged and stays first. The same issue lets the five subcommands whose round the state uniquely determines resolve an omitted `--round` from state, while every subcommand where the flag selects an operation or names a caller-chosen round keeps it required. Run state persists to a cwd/worktree-anchored `.prflow/tmp/issue-audit-state-<slug>.json`, replacing the markdown event log the offer used to read — the skill still writes the observable audit artifact (`.prflow/tmp/issue-audit-<slug>.md`, same gate/read-only-stand-in convention as the derivation artifact), and both state paths plus the **retired** `.md` leftover stay declared out of bounds so an auditor with repository read access cannot re-anchor on this run's prior verdicts. **Writes to that state document are serialized (issue #1040).** Every mutating subcommand runs inside an exclusive-create sentinel critical section — a `.lock` file beside the state document, created with `os.O_CREAT | os.O_EXCL` (standard-library only) — so two concurrent invocations for the same slug produce a document reflecting one of them entirely and then the other, never an interleaved mixture; each writer also persists through its own unique `tempfile.mkstemp` temporary path (with a bounded `os.replace` retry over `PermissionError`) so two writers never share and truncate one temp file. **Read-only subcommands acquire no sentinel and stay unserialized** (`query-*`, `emit-body`, `check-claim-staleness`), and a fail-closed transitive call-graph check proves `save_state` is unreachable from every read-only-classified subcommand. An **abandoned sentinel is recovered by age**, and the threshold that decides it is `stale_after_s` — *not* the longer `acquire_window_s` a contending writer is willing to wait: a sentinel whose mtime age exceeds `stale_after_s` is unlinked, and the exclusive create is then re-attempted exactly once before control returns to the ordinary acquire loop. Because the shipped `stale_after_s` is strictly shorter than the shipped `acquire_window_s`, a writer contending with an abandoned sentinel always reaches that break inside its own acquire window, which is what makes a crashed writer unable to wedge the slug permanently; the acquire-window expiry is the fail-closed arm for a host whose overrides invert that relation, and it refuses the mutation rather than proceeding. That relation is the load-bearing part — an override that sets `stale_after_s` above `acquire_window_s` trades permanent-wedge immunity for the refusal. This guarantee is **document integrity only**. The **decision channel is a distinct quantity with a named residual**: because `_emit_next_call` re-reads the state after the critical section has released, and every `query-*` reads unserialized, a batch of concurrent mutations renders each `next_call=` line and each query answer against whichever post-image that process happened to observe — so those answers are **not authoritative under concurrent invocation**, even though the persisted document they read is always internally consistent. §11 is the canonical home for this system contract; the skill's operational rule points here rather than restating it. Only the current draft counts as audited: eligibility is grounded on a completed clean-verdict round whose recorded identity still holds — byte-digest equality against the canonical file on file-arm epochs, revision ordering on the embed and inline arms where no trustworthy file exists — or on an explicitly recorded override that no later revision has invalidated, and every `eligible` answer carries a deterministic token bound to the answering digest or revision ordinal (matching the ground that answered) which the summary line quotes verbatim. That **narrows** the prose-compliance gap and makes a skipped eligibility check detectable in the transcript; it does not close it, since no in-process component can force an orchestrator that never invokes it. Where the tool cannot run at all, a named bounded fallback runs one round, asks once, and marks the summary line `state-owner unavailable` — distinct from `degraded`, which keeps its meaning of the inline audit arm.
 
 **Runtime main-thread context (issue #767).** Separate from the skill's *structure* and *static shipped size* discussed above, a long create-issue run accumulates a large **runtime main-thread context** across its many turns. The behavioral instrument `scripts/create-issue-context-eval.py` (maintainer-run over a transcript corpus; never on the skill's runtime path, so no new tool grant) measures it, and the determination of which appended-content classes are authoritative versus safely-removable redundant additions — with the reduction that removes the primary safely-removable class (re-emission of an already-produced block, replaced by a pointer) — lives in [`docs/create-issue-context.md`](create-issue-context.md). That doc is the single source of truth for this axis; it is not paraphrased here.