From abf3cf7c25c7640c046ce354b04bfe9f74446f99 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 15 Aug 2026 17:00:52 -0400 Subject: [PATCH] fix(#3458): scan archived milestone phases, and make [A] Acknowledge actually suppress (#3555) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3458): scan archived milestone phases in the four audit-open scanners `query audit-open` resolved exactly one phase root, `.planning/phases/`. When a milestone closes its phase directories move to `.planning/milestones/v-phases/`, so an item still unresolved at that moment — the `[R]/[A]/[C]` prompt accepts "accept" and "carry forward", not only "resolve" — became invisible to the v1.1 pre-close audit and every audit after it. The window in which an unresolved item is visible to this gate was exactly one milestone wide, and nothing announced when it closed. Reproduced before fixing, with byte-identical artifacts in the two layouts and the active layout as the control: active → has_open_items=true deferred=1 uat_gaps=1 total=2 archived → has_open_items=false deferred=0 uat_gaps=0 total=0 `scanDeferredItems`' own doc comment names this as the thing it was built to prevent — "phase directories archive to `milestones/vX.Y-phases/` (#1871) and the entry leaves the live tree having never been triaged" — while the implementation eleven lines below cannot read that path. It catches an entry at its own milestone close and goes blind at precisely the transition the comment describes. This is not cosmetic under-reporting. `auditOpenArtifacts` sums all nine category counts into `counts.total` and returns `has_open_items: counts.total > 0`, so four blind scanners can flip the gate's headline boolean and let `/gsd-complete-milestone` assert a clean close it never verified. In a fully-archived project `.planning/phases/` may not exist at all, and the scanners' `if (!fs.existsSync(phasesDir)) return []` produced a value indistinguishable from "nothing is open". ## One enumeration, not four The four scanners each hand-rolled the same active-only walk. They now share `listAuditPhaseTargets(planDir, cwd)`, which yields both roots — the shape of fix epic #3473's B2 asks for, and the reason the fix is one seam rather than four edits. Three properties are load-bearing: * the ACTIVE enumeration is unchanged — still a raw `readdirSync`, NOT `listMilestonePhaseDirs`. These scanners are deliberately not milestone-filtered today, and switching would silently add window and sentinel filtering: a behavior change belonging to #3372, not here. * a missing or unreadable active root skips that half instead of returning early. That early return WAS the bug in a fully-archived project. * archived dirs are deliberately NOT milestone-filtered, per the comment `src/uat.cts` already carries: archived phases belong to past milestones by definition, so applying the current-milestone filter discards every one and silently reinstates this bug. Each item now carries `archived_milestone` when it comes from a closed milestone, matching how the sibling module already labels archived results — without it an operator triaging `[R]/[A]/[C]` cannot tell a live item from one carried over. Additive: no existing test or doc asserted an exact key set. `scripts/lint-phase-enumeration-drift.cjs`'s exemption list for this file drops from the four scanner names to the single helper, since that is now the only place the enumeration lives. ## Tests Written failing-first and confirmed red for the right reason before the fix, all four driven through the real `audit-open` CLI rather than private functions: archived-only (was 0/0/0/0 with `has_open_items=false`, now 1/1/1/1 true), mixed active+archived (was 1/1/1/1 — the archived half dropped — now 2/2/2/2), active-only unchanged, and an all-resolved archived phase contributing 0. That last one passed vacuously before the fix, because the archived path was not reached at all; it was re-verified as genuinely discriminating afterward by flipping one archived item to unresolved and watching the count rise. Closes #3458 Co-Authored-By: Claude Opus 5 * fix(#3458): restore the scan_error sentinel and show archive provenance Adversarial review found one BLOCKER that the previous revision introduced, which a green remote-runner suite did not catch because nothing in the tree asserts `scan_error` at all. ## The regression Consolidating four hand-rolled walks into `listAuditPhaseTargets` swallowed the active-root `readdirSync` throw in a bare `catch {}`. Pre-fix each scanner returned `[{scan_error: true, …}]`; after, each returned `[]`. Measured with `.planning/phases` created as a FILE (so `existsSync` passes and `readdirSync` throws ENOTDIR): before this fix: uat_gaps/verification_gaps/context_questions/deferred_items each `[{"scan_error":true,…}]` the regression: each `[]` `complete-milestone.md` re-runs `audit-open --json` and reads those counts, so a machine consumer could no longer tell "I/O failed" from "verified clean" — the exact conflation this issue exists to remove, reintroduced on the failure path. `listAuditPhaseTargets` now reports `activeUnreadable` and each scanner pushes the sentinel shape recovered verbatim from `origin/next`, not reinvented. The docstring claiming the active enumeration was "UNCHANGED" was false while that sentinel was missing, and is corrected to state what is actually preserved. An unreadable ARCHIVED root deliberately gets NO sentinel: there was no archived read before, so there is no consumer contract to preserve, and adding one would conflate the ordinary "no milestones archived yet" state with a real I/O failure. ## The operator could not see the archive `formatAuditReport` is the surface the gate actually shows a human — `complete-milestone.md` runs it without `--json` — and it never rendered `archived_milestone`. With `01-alpha` in both roots the identical line printed twice with nothing to tell them apart, and `[R] Resolve` sends the operator to `.planning/phases/01-alpha/` where the archived one does not exist. Phase numbering restarts at `01` after each archive, so that collision is the common case, not an edge case. All four loops now render ` (archived vX.Y)`; active lines stay byte-identical. ## Archived milestones sorted wrong `getArchivedPhaseDirs` ordered milestones with `.sort().reverse()` — lexicographic, so `v1.9` outranked `v1.10`. Measured order for v1.0/v1.9/v1.10 was `v1.9, v1.10, v1.0`. Now a numeric-segment descending compare. Pre-existing, but this change is what first surfaces it in audit output. ## Tests The blocker's regression test fails against the previous revision. Added: `archived_milestone` present on archived items and absent (not `undefined`) on active ones; the unreadable-active-root sentinel across all four categories; an unreadable archived root still leaving the active half scanned; the duplicate-name case producing two distinct entries that the human report distinguishes; and the v1.10-before-v1.9 ordering. `docs/COMMANDS.md` documents the archived scanning and the new field. Closes #3458 Co-Authored-By: Claude Opus 5 * fix(#3458): stop filesystem names forging lines in the audit report Found by the security review of this branch. Pre-existing on `next`, fixed here because it defeats the exact gate this PR is hardening. `audit-open`'s human report is the surface `/gsd-complete-milestone` shows an operator to decide whether a milestone may close. A `.planning/` tree authored by someone other than that operator — a cloned repo — could contain a directory literally named: zz0 open items require decisions.[2K[1G FORGED and the report printed `0 open items require decisions.` as its own line, with raw ESC bytes reaching stdout able to erase or overwrite the lines above it. Reproduced against the real CLI before fixing, and again after. ## Why not just harden sanitizeForDisplay Because that helper's contract is multi-line prose — it removes protocol-leak lines while deliberately preserving the newlines between legitimate ones, which `tests/security.test.cjs` pins. Stripping CR/LF there would have broken a correct test to paper over a different problem. The two jobs are genuinely different, so there are now two helpers. New `sanitizeLabel` (`src/security.cts`) is for values that are semantically ONE LINE and derived from a filesystem NAME. It ESCAPES rather than strips C0 (including ESC/CR/LF), DEL and C1, so a doctored name renders visibly as `\n` / `\x1b` instead of being silently normalized — the report stays honest about what is in the tree. Ordinary input passes through byte-identical. ## Nine sites, not four The first pass covered the four phase-scoped scanners. A sweep of the rest of the file found the identical class in five more — `scanDebugSessions`, `scanQuickTasks`, `scanThreads`, `scanTodos`, `scanSeeds` — emitting name-derived `slug` / `filename` / `seed_id` through the prose sanitizer. `scanQuickTasks`' `date` had no sanitization call at all. Every emitted field in the file is now classified and the sweep recorded: `slug`, `filename`, `seed_id`, `phase`, `file`, `archived_milestone`, `date` are name-derived and take `sanitizeLabel`; `hypothesis`, `status`, `updated`, `title`, `priority`, `area`, `summary`, `questions[]` and deferred-item `text` are content and keep `sanitizeForDisplay`. No name-derived value reaches output unsanitized. `--json` was already safe — JSON string encoding escapes control characters, and a crafted name cannot break out of the string. Verified rather than assumed. Closes #3458 Co-Authored-By: Claude Opus 5 * chore(#3458): backfill changeset pr number * test(#3458): skip control-character fixtures where the OS forbids the name CI red on `test (windows-latest, 24, shard 1/3)`: the four forgery-rejection tests build directories whose names embed a newline and ESC, and NTFS forbids control characters in path components, so `mkdir` threw ENOENT. The remote runner is Linux-only, so it could not have caught this class. Semantically the skip is honest rather than a workaround: on Windows the directory-name forgery vector does not exist, because the OS refuses to create the name. The sanitizer's own behavior stays covered there by the `sanitizeLabel` unit tests, which are pure string tests with no filesystem calls — verified. Uses the repo's established capability-probe convention (`tests/adr-index-gate.test.cjs`'s `trySymlink`), which `t.skip()`s on the real errno rather than branching on `process.platform`, and whose comment gives the reason: a bare `return` "would silently report a PASS ... and hide the gap this guard exists to close". A skipped test is visibly skipped. Swept every test added on this branch for names Windows would reject or POSIX path assumptions; these four were the only ones. Co-Authored-By: Claude Opus 5 * feat(#3458): make [A] Acknowledge actually suppress, without overwriting a verdict Making archived phases visible exposed the other half of the problem: an item unresolved at a milestone close now resurfaces at every later close forever, because `[A] Acknowledge` wrote a prose block to STATE.md that `auditOpenArtifacts` never reads. `verified_closeout` became unreachable and the gate degraded to a mandatory `[A]` every time. ## The prompt does not change `[A] Acknowledge all` already promises "document as deferred and proceed with close". It documented but never deferred. This makes `[A]` do what it says. `[R]` and `[C]` stay abort paths. No "carry forward" option is invented — an item that is not acknowledged simply keeps surfacing, which is the default. ## The marker lives inside the artifact Not a ledger. The audit mints no ids and has no stable identity — `phase` is a token that collides across directories, `file` for deferred items is a constant, and identity otherwise degrades to the item's own prose after a lossy sanitizer. Any ledger must re-derive that key every close, so a reworded item silently un-suppresses or, worse, mis-suppresses a different one. Storing the acknowledgment next to the thing it suppresses makes that class of bug structurally impossible, and it is the pattern `src/uat.cts` already argues for with `deferred-items.md`'s in-place `status: resolved`. ## The marker is verdict-preserving and self-invalidating `status:` is never overwritten — writing `resolved` into an unresolved UAT would be a lie in the artifact of record, and the disclosure has to be additive. audit_acknowledged: milestone: v1.0 at: 2026-08-15 status: gaps_found # snapshot of what was true when acknowledged Suppression applies ONLY while the snapshot still matches reality: `status` for seven categories, `question_count` for context questions, and for deferred items a new per-entry `status: acknowledged` distinct from `resolved`, which keeps meaning "actually fixed". Change the artifact and the acknowledgment stops applying, so the item comes back on its own. That is what makes re-opening answer itself with no extra state, and it fails in the safe direction: a stale acknowledgment can never hide a NEW problem. A malformed marker is treated as absent — a bad marker must never silence an item. The check is ONE shared `isAuditItemAcknowledged`, not nine copies. This file has already been through that defect family twice in this PR. ## Observable, not silent `audit-open --json` now reports an `acknowledged` count beside `counts`, so a reviewer can tell a close that is clean because things were fixed from one that is clean because things were silenced. ## Writer New `audit-open acknowledge` verb snapshots current state itself, so the marker is never hand-authored from workflow prose — the gap that left the STATE.md block with no writer, no schema and two conflicting formats. Writes route through the existing path-confinement seam. ## Two deliberate limits, failing closed Heading-delimited deferred entries (#3457) are REFUSED with `unsupported_heading_shape` rather than edited, because mapping a heading entry back to its exact source span is not safely derivable when headless and heading entries interleave in one file. A loud refusal beats a mis-targeted write. A quick task with no summary gets one created to carry the marker, since there is otherwise nowhere to put it. ## Tests Self-invalidation is the important one and is covered per category: acknowledge, then change the status or question count, and the item resurfaces. Also malformed markers not suppressing, `status:` byte-unchanged after acknowledging, the writer refusing a path outside the project, and the four original #3458 scenarios unchanged. Closes #3458 Co-Authored-By: Claude Opus 5 * feat(#3458): wire [A] to the acknowledge verb and converge the disclosure table Consumer side of the suppression seam. ## The workflow stops hand-authoring the mechanism `[A]` now calls `audit-open acknowledge` once per open item, then writes the STATE.md `## Deferred Items` table as before. The table stays as a human-readable disclosure; it is no longer the mechanism. That closes the gap where the block had no writer, no schema and no reader — the marker is now written by the tool, which snapshots current state itself. The `[R]` / `[A]` / `[C]` prompt is unchanged, `[C]` still means "Cancel — exit without closing", and no carry-forward option is invented. The all-clear branch now distinguishes a close that is clean because items were FIXED from one that is clean because they were ACKNOWLEDGED, using the `acknowledged.total` count, and carries that into the MILESTONES.md disclosure line beside the existing override count. A clean close that was bought with acknowledgments should say so. ## Format drift resolved Two incompatible `## Deferred Items` shapes shipped simultaneously — 3 columns in the workflow, 4 in the template, with different body lines. Converged on one 5-column shape carrying the source Milestone, since archived items now appear and the archived-milestone disambiguator was previously discarded at write time. The workflow enumerates the categories instead of trailing off in `...`. ## Ack fragment bookkeeping `complete-milestone.md` grows 6,764 bytes (31,228 → 37,992; cap 61,440), covered by a new `tests/emitted-drift-acks/3458-*.json`. `2962-zsh-nomatch-for-glob-portability.json`'s `complete-milestone.md` entry is REMOVED — the no-duplicate-path rule hard-blocks two sources naming one path. That entry is spent: the nullglob shim it acknowledges is present in both `origin/next` and the CI emitted baseline `fd2b97a5`, so its ripple is already absorbed and it can never clear anything again — verified directly, not assumed, and the gate's own message directs deleting spent entries. Its other three files' entries are untouched. `scripts/sync-runtime-launcher.cjs` wanted to rewrite `explore.md` as well — pre-existing drift unrelated to this change, reverted. `complete-milestone.md` still carries exactly one canonical preamble. Docs cover the verb's real flag surface, the marker's verdict-preserving and self-invalidating behavior, and the new `acknowledged` count. A second `Added` changeset covers the verb, since the existing `Fixed` fragment describes only the archived-phase scanning. Closes #3458 Co-Authored-By: Claude Opus 5 * fix(#3458): close three blockers in the acknowledgment seam Adversarial review of the seam. Three BLOCKERs, one of which disproves a safety claim I published in the PR body, the changeset and the docs. ## The claim was false; the code is fixed rather than the claim softened I wrote that "a stale acknowledgment can never hide a NEW problem". It could. `context_questions` snapshotted only the question COUNT, so replacing two acknowledged questions with two brand-new blockers kept the item suppressed. `uat_gaps` snapshotted only `status`, so adding five more pending scenarios (`open_scenario_count` 1→6) kept it suppressed. The snapshot now identifies CONTENT, not size: a digest of the whole question set, and a status + open-scenario-count composite. Any edit invalidates. The other seven categories were checked and their single tracked dimension is already the whole story. Both disproofs now resurface the item. ## Writing to the wrong line, and reporting success `acknowledgeDeferredItem` built an unanchored regex and exec'd it over the whole file while match-selection and the ambiguity guard ran over the section body only, so the write landed at the first match ANYWHERE. A file with `# Notes` holding `- Fix the parser` above a `## Deferred Items` section holding the same bullet: the CLI exited 0 saying `acknowledged: true`, injected `status: acknowledged` under `# Notes`, and re-audit still reported the entry open. It corrupted unrelated content, suppressed nothing, and claimed success — and since `--file` is unconstrained the same path could inject into a UAT or VERIFICATION body. Matching is now anchored to the selected section, and the matched span is re-verified against the selected entry before any write; a mismatch refuses with `match_verification_failed` rather than writing. ## Acknowledging todos hid the ones never shown `scanTodos` capped at five files and then checked acknowledgment. With seven todos, acknowledging the five that were LISTED drove `todos: 0`, `has_open_items: false`, and items six and seven never appeared in any later scan. The workflow's own "repeat until no todos items" remedy terminates after one pass. Pre-feature this was unreachable because the count was pinned at five. That is silent over-suppression — the exact direction this PR exists to remove. Acknowledged items are now filtered BEFORE the display cap, so unacknowledged todos beyond it still drive the count. ## The [A] branch could not fail closed Every acknowledge call sat in a `cmd | while read` pipeline with no status accumulation, so any refusal was discarded and the close proceeded as `override_closeout`. Separately, `io.output` swaps payloads over 50000 chars for an `@file:` sentinel — every `jq` would then fail, every loop body run zero times, nothing be suppressed, and the close happen anyway. Both closed: failures accumulate across all invocations and halt before close, and the sentinel is dereferenced using the same pattern `verify_readiness` already uses for `INIT_MANAGER`. Quoting was verified sound by the review and is left alone. ## Also Suppression is now visible in the human report, not only `--json` — the "clean because fixed vs clean because silenced" distinction was promised for the surface an operator actually reads. The CRLF-preservation branches in the writer were dead: every `.md` write goes through `_normalizeMd`, which normalizes line endings and blank lines whatever the writer does. Deleted and documented rather than left as code that cannot run. ## Why these shipped The review named it exactly: there was no coverage for `unsupported_heading_shape`, `ambiguous`, `not_found`, duplicate-text mis-targeting, todos beyond the cap, or CRLF. All are now tested, alongside both snapshot disproofs and the mixed-section fixture. Closes #3458 Co-Authored-By: Claude Opus 5 * test(#3458): align the items-open footer wording with its assertion Remote runner red on one test: the items-open footer must match `/previously acknowledged item/i`. The disclosure was NOT missing — the items-open branch already printed "N additional items previously acknowledged and still suppressed." The word order simply did not match the regex the test in the same change asserts. A wording mismatch between my own test and my own implementation, not a behavior gap. Reworded to "N previously acknowledged items also suppressed above the M open items", which satisfies the assertion and states the relationship between the two counts more plainly than the original did. Swept `formatAuditReport` for other branches that could skip the tally: the only early return is the all-clear path, which already discloses it. `scan_error` sentinels are filtered per category and excluded from `counts.total`, so an all-error project falls through to that same branch. No inconsistency remains. Co-Authored-By: Claude Opus 5 * fix(#3458): splice by carried span, digest the untruncated question set Security review of the writer. Both findings are the same shape, and both are cases where an earlier fix of mine was incomplete in the same direction: a value derived for DISPLAY was reused for an IDENTITY or LOCATION decision. ## Writing to the wrong entry, again The previous fix anchored matching to the `## Deferred Items` SECTION but still re-found the entry inside it with an unanchored regex, so the write landed at the first SUBSTRING occurrence rather than the entry's own span. The `match_verification_failed` guard could not catch it, because the mis-targeted span is byte-identical to the target. Probe-confirmed, in a cloned repo's own artifact: - CRITICAL unfixed auth bypass see also: - minor typo - minor typo Acknowledging "minor typo" appended `status: acknowledged` into the CRITICAL entry, suppressing it at every future close, while the typo stayed open — exit 0, `"acknowledged": true`. A variant where the target text appears inside unrelated prose split that line mid-sentence, acknowledged nothing, and still exited 0, so the workflow's `ACK_FAILURES` halt never fired. Fixed structurally rather than with a better regex: `splitGapsEntriesWithSpans` carries each entry's own character span out of the splitter, and the write splices by that recorded span. The location is already known at selection time — re-deriving it by searching was the entire defect class. Added as a sibling so `splitGapsEntries`' three existing callers are untouched. With index-splicing, `match_verification_failed` becomes a genuine independent cross-check instead of a guard that could never fire. ## The digest was blind past the third question `deriveOpenQuestions` truncated to three questions, and clamped each to 200 chars, BEFORE the digest hashed it — so the snapshot could not see the fourth and later. Ship three innocuous questions, acknowledge, then add real blockers, and they are permanently invisible: measured `open=0, acknowledged=1`, report "All artifact types clear." That is the same self-invalidation property this digest was added to guarantee one revision ago. The digest now covers the untruncated list; truncation is display-only. Found while fixing it: the previous digest joined on a literal raw NUL byte embedded in the source — collisions are constructible, and reachable through attacker-controlled YAML `\x00` escapes. Verified both ways. Replaced with a length-prefixed encoding so no two question sets can collide by concatenation. ## Sweep Because this is the third incomplete fix on this seam, every identity and location derivation was swept for the display-vs-identity confusion: uat_gaps uses status plus a full-content count, the other seven categories use a scalar status or presence, the deferred `--text` identity is never truncated, and all five flat categories resolve their file by path rather than by content search. No further instances. Closes #3458 Co-Authored-By: Claude Opus 5 * test(#3458): correct two assertions that over-reached the measured behavior Remote runner red on two of the F1 tests. The source is correct — reproduced both fixtures against the built CLI — and both failures were bugs in the assertions I wrote. `src/` is untouched by this commit. The first is worth recording. It computed the CRITICAL entry's block as content.slice(content.indexOf('- CRITICAL'), content.indexOf('- minor typo')) and `indexOf` found the FIRST SUBSTRING occurrence, which lives inside that entry's own continuation line ` see also: - minor typo`. The block was truncated mid-line, so the assertion could never match. The test committed the exact first-substring-match mistake it exists to catch, one revision after that mistake was fixed in the source. The second asserted `deferred_items === 0` after acknowledging the typo entry, but the decoy `- Note: reference - minor typo elsewhere, ignore` is itself an open entry and was never acknowledged, so the correct count is 1. It now also asserts WHICH item remains open — that is what actually proves the right entry was suppressed, and the original assertion would have passed even if both had been silenced. Both now derive their expectations from measured CLI output. A comment records that the write seam normalizes markdown (`_normalizeMd` inserts a blank line before a list item following a non-list line) so the inserted line is not later mistaken for a regression; that is repo-wide behavior for every `.md` write through the single write projection, not something this change should diverge from. Root cause of both: the previous two dispatches verified behavior with direct CLI probes but never executed the test file, so assertions could over-reach what had actually been measured. Every other assertion added in those two commits has since been re-derived from real output; no further mismatches. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: sim Co-authored-by: Claude Opus 5 --- .changeset/jolly-voles-rally.md | 5 + .changeset/rapid-tunas-dance.md | 5 + docs/CLI-TOOLS.md | 6 + docs/COMMANDS.md | 24 +- gsd-core/templates/state.md | 8 +- gsd-core/workflows/complete-milestone.md | 135 +- scripts/lint-phase-enumeration-drift.cjs | 25 +- scripts/lint-plan-count-drift.cjs | 12 +- src/audit-command-router.cts | 19 +- src/audit.cts | 1175 ++++++++++++---- src/phase-locator.cts | 23 +- src/security.cts | 60 + src/uat.cts | 315 ++++- tests/audit-command-cutover.test.cjs | 1219 +++++++++++++++++ ...2962-zsh-nomatch-for-glob-portability.json | 1 - .../3458-audit-open-acknowledge-wiring.json | 6 + tests/security.test.cjs | 46 + 17 files changed, 2763 insertions(+), 321 deletions(-) create mode 100644 .changeset/jolly-voles-rally.md create mode 100644 .changeset/rapid-tunas-dance.md create mode 100644 tests/emitted-drift-acks/3458-audit-open-acknowledge-wiring.json diff --git a/.changeset/jolly-voles-rally.md b/.changeset/jolly-voles-rally.md new file mode 100644 index 000000000..30f62f50b --- /dev/null +++ b/.changeset/jolly-voles-rally.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3555 +--- +**Items left unresolved when a milestone closes are no longer invisible to every later audit** — `query audit-open`'s four phase-scoped scanners read only `.planning/phases/`, so once a milestone closed and its phase directories moved to `.planning/milestones/vX.Y-phases/`, any UAT gap, verification gap, context question or deferred item still open at that moment vanished from the pre-close audit permanently. In a fully-archived project the scanners returned nothing at all, which is indistinguishable from a clean tree — and because the audit sums every category into one `has_open_items` boolean, that could report a clean close it had not verified. All four now scan the archived milestone directories as well, and each item says which milestone it came from. (#3458) diff --git a/.changeset/rapid-tunas-dance.md b/.changeset/rapid-tunas-dance.md new file mode 100644 index 000000000..fa3fe422b --- /dev/null +++ b/.changeset/rapid-tunas-dance.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3555 +--- +**`audit-open acknowledge` now suppresses open audit items at future milestone closes** — deferring an item via /gsd-complete-milestone previously only wrote a human-readable note; the item resurfaced at every later close with no way to silence it short of resolving it for real. The new `audit-open acknowledge --category --milestone [--at ] ...` CLI verb writes a verdict-preserving `audit_acknowledged` marker that suppresses the item starting at the next audit scan, without ever touching the artifact's own `status:` field, and self-invalidates the moment the artifact's observed state changes again. `query audit-open --json` now also reports an `acknowledged` count per category alongside `counts`, so a clean close can be told apart from one that is clean only because prior items are still suppressed. (#3458) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index f11c6f217..82ec96571 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -804,6 +804,12 @@ node gsd-tools.cjs audit-uat # Cross-artifact audit queue — scan `.planning/` for unresolved audit items node gsd-tools.cjs audit-open [--json] +# Suppress one open audit item — writes a self-invalidating `audit_acknowledged` +# marker; never overwrites the artifact's own `status:` (except `deferred_items`, +# where the marker IS the entry's `status:`). See docs/COMMANDS.md's +# `/gsd-complete-milestone` entry for the full per-category identifier flag table. +node gsd-tools.cjs audit-open acknowledge --category --milestone [--at ] + # Reverse-migrate a GSD-2 project into the current structure (backs `/gsd-import --from-gsd2`) node gsd-tools.cjs from-gsd2 [--path ] [--force] [--dry-run] diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 9eac932d4..8ff9ecf7c 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -468,7 +468,29 @@ Archive milestone, tag release. | CONTEXT questions | `*-CONTEXT.md` | questions left open | | **Deferred items** | `deferred-items.md` | entry lacks `status: resolved` | -If any category is non-empty you are prompted with `[R] Resolve` / `[A] Acknowledge all` / `[C] Cancel`. `[A]` records the items to `STATE.md` under its own `## Deferred Items` heading and closes as `override_closeout`; an all-clear closes as `verified_closeout`. +The four phase-scoped categories above (UAT gaps, Verification gaps, CONTEXT questions, Deferred items) read phase directories from **both** the active `.planning/phases/` root and every archived `.planning/milestones/vX.Y-phases/` root (#3458) — an item still unresolved when its milestone closed and its phase directory archived stays visible in every later audit instead of silently disappearing. In `--json` output, an item sourced from an archived milestone carries an `archived_milestone` field (e.g. `"v1.0"`); active items omit the field entirely. The human-readable report labels an archived item's line with `(archived vX.Y)` so a phase number that repeats across milestones (numbering restarts at `01` after each archive) is not misread as one duplicate line. + +If any category is non-empty you are prompted with `[R] Resolve` / `[A] Acknowledge all` / `[C] Cancel`. `[A]` calls `gsd-tools audit-open acknowledge` once per open item — the CLI writer that actually suppresses each item starting at the next `audit-open` scan — then records the same items to `STATE.md` under its own `## Deferred Items` heading (a disclosure record, not the suppression mechanism) and closes as `override_closeout`; an all-clear closes as `verified_closeout`. + +**`audit-open acknowledge` (#3458 follow-up).** Suppresses one open item by writing (or refreshing) a verdict-preserving `audit_acknowledged` marker in the artifact's own frontmatter: + +```bash +gsd-tools audit-open acknowledge --category --milestone [--at ] +``` + +`--category` and `--milestone` are always required; `--at` defaults to today. The identifier flags depend on `--category`: + +| `--category` | Identifier flags | +|---------------|-------------------| +| `debug_sessions` | `--slug ` | +| `threads` | `--slug ` | +| `seeds` | `--seed-id ` | +| `todos` | `--filename ` | +| `quick_tasks` | `--dir ` (the `.planning/quick//` directory name — note this is the ORIGINAL directory name, not the date-stripped `slug` the audit JSON displays) | +| `uat_gaps`, `verification_gaps`, `context_questions` | `--phase --file ` [`--archived-milestone `] | +| `deferred_items` | `--phase --file --text ` [`--archived-milestone `] | + +The marker never overwrites the artifact's own `status:` field for the eight frontmatter-marker categories — only `deferred_items` is the deliberate exception, where the marker IS the entry's `status:` field (there is no other meaning for that field on a `deferred-items.md` bullet). The marker also self-invalidates: it snapshots the artifact's current observed state at acknowledgment time — its `status:` for most categories, a composite of `status:` plus its open-scenario count for `uat_gaps` (a status can stay the same while more scenarios go pending), and a content digest of the full question set (not just a count) for `context_questions` (so replacing every question's text while holding the count steady still invalidates the snapshot) — and the item resurfaces on its own the moment that snapshot no longer matches — an edited, reopened, or otherwise-changed artifact is never silently suppressed forever. `--json` output on `audit-open` (the `run` subcommand, default) now reports an `acknowledged` count per category alongside `counts`, plus an `acknowledged.total`, so a clean audit (`counts.total === 0`) can be told apart from one that is clean only because earlier items are still being suppressed (`acknowledged.total > 0`). > **Note:** the `deferred-items.md` category is the per-phase SCOPE BOUNDARY log a phase agent writes when it finds a defect it should not fix. It is a different artifact from the `## Deferred Items` section `[A]` writes into `STATE.md`, which records what you acknowledged at close. diff --git a/gsd-core/templates/state.md b/gsd-core/templates/state.md index 7eb0b06c8..07e93af87 100644 --- a/gsd-core/templates/state.md +++ b/gsd-core/templates/state.md @@ -79,11 +79,11 @@ None yet. ## Deferred Items -Items acknowledged and carried forward from previous milestone close: +Items acknowledged and deferred at milestone close, most recent first: -| Category | Item | Status | Deferred At | -|----------|------|--------|-------------| -| *(none)* | | | | +| Category | Item | Status | Deferred At | Milestone | +|----------|------|--------|-------------|-----------| +| *(none)* | | | | | ## Session Continuity diff --git a/gsd-core/workflows/complete-milestone.md b/gsd-core/workflows/complete-milestone.md index 6c58d3b4e..0a90e8543 100644 --- a/gsd-core/workflows/complete-milestone.md +++ b/gsd-core/workflows/complete-milestone.md @@ -61,26 +61,135 @@ These items are open. Choose an action: ``` If user chooses [A] (Acknowledge): -1. Re-run `gsd-tools.cjs query audit-open --json` to get structured data -2. Write acknowledged items to STATE.md under `## Deferred Items` section: +1. Re-run `gsd-tools.cjs query audit-open --json` to get structured data. +2. Acknowledge every open item through the `audit-open acknowledge` CLI writer — this is what actually suppresses each item starting at the NEXT `audit-open` scan; the STATE.md table in step 3 is a disclosure record only, it is no longer the suppression mechanism. Every acknowledge call's exit status is accumulated (`ACK_FAILURES`); the step HALTS before closing if any failed — a refusal (`unsupported_heading_shape`, `ambiguous`, `not_found`, missing file, etc.) must never be silently discarded and let the close proceed as if everything were suppressed. `AUDIT_JSON` uses the same `@file:` large-payload sentinel handling `INIT_MANAGER` uses in `verify_readiness` below — `io.output` swaps any JSON payload over 50000 chars for a `@file:` marker, and feeding that literal string to `jq` would silently make every loop body below iterate zero times: + ```bash + AUDIT_JSON=$(gsd_run query audit-open --json) + if [[ "$AUDIT_JSON" == @file:* ]]; then AUDIT_JSON=$(cat "${AUDIT_JSON#@file:}"); fi + MILESTONE_VERSION="v[X.Y]" # already known from ROADMAP.md's active milestone header — the same identifier `milestone.complete` uses in the archive_milestone step + + ACK_FAILURES=0 + ACK_FAILURE_LOG="" + record_ack_failure() { + ACK_FAILURES=$((ACK_FAILURES + 1)) + ACK_FAILURE_LOG="${ACK_FAILURE_LOG} + - $1" + } + + # debug_sessions / threads (--slug) + # NOTE: `< <(...)` process substitution, not `... | while`, so the loop + # runs in THIS shell — a `| while` pipeline puts the loop in a subshell + # and any ACK_FAILURES/ACK_FAILURE_LOG update inside it is lost the + # moment the pipeline exits. + for cat in debug_sessions threads; do + while IFS= read -r slug; do + [ -z "$slug" ] && continue + if ! gsd_run query audit-open acknowledge --category "$cat" --milestone "$MILESTONE_VERSION" --slug "$slug"; then + record_ack_failure "$cat slug=$slug" + fi + done < <(printf '%s' "$AUDIT_JSON" | jq -r --arg cat "$cat" '.items[$cat][] | select(.scan_error | not) | .slug') + done + + # seeds (--seed-id) + while IFS= read -r seed_id; do + [ -z "$seed_id" ] && continue + if ! gsd_run query audit-open acknowledge --category seeds --milestone "$MILESTONE_VERSION" --seed-id "$seed_id"; then + record_ack_failure "seeds seed_id=$seed_id" + fi + done < <(printf '%s' "$AUDIT_JSON" | jq -r '.items.seeds[] | select(.scan_error | not) | .seed_id') + + # todos (--filename) — the scanner caps its list to 5 entries per scan + # (remainder items carry `_remainder_count`, no `filename`, and are skipped) + while IFS= read -r filename; do + [ -z "$filename" ] && continue + if ! gsd_run query audit-open acknowledge --category todos --milestone "$MILESTONE_VERSION" --filename "$filename"; then + record_ack_failure "todos filename=$filename" + fi + done < <(printf '%s' "$AUDIT_JSON" | jq -r '.items.todos[] | select((.scan_error or ._remainder_count) | not) | .filename') + + # quick_tasks (--dir) — the scanner's `slug` strips a leading + # YYYYMMDD-/YYYY-MM-DD- date prefix for display; `--dir` needs the + # ORIGINAL .planning/quick// name, so reconstruct it from `date`+`slug`. + while IFS= read -r dir; do + [ -z "$dir" ] && continue + if ! gsd_run query audit-open acknowledge --category quick_tasks --milestone "$MILESTONE_VERSION" --dir "$dir"; then + record_ack_failure "quick_tasks dir=$dir" + fi + done < <(printf '%s' "$AUDIT_JSON" | jq -r '.items.quick_tasks[] | select(.scan_error | not) | if .date != "" then "\(.date)-\(.slug)" else .slug end') + + # uat_gaps / verification_gaps / context_questions — phase-scoped + # (--phase --file [--archived-milestone] when the item was found in an archived phase) + for cat in uat_gaps verification_gaps context_questions; do + while IFS= read -r item; do + [ -z "$item" ] && continue + phase=$(printf '%s' "$item" | jq -r '.phase') + file=$(printf '%s' "$item" | jq -r '.file') + archived=$(printf '%s' "$item" | jq -r '.archived_milestone // empty') + if [ -n "$archived" ]; then + if ! gsd_run query audit-open acknowledge --category "$cat" --milestone "$MILESTONE_VERSION" --phase "$phase" --file "$file" --archived-milestone "$archived"; then + record_ack_failure "$cat phase=$phase file=$file archived-milestone=$archived" + fi + else + if ! gsd_run query audit-open acknowledge --category "$cat" --milestone "$MILESTONE_VERSION" --phase "$phase" --file "$file"; then + record_ack_failure "$cat phase=$phase file=$file" + fi + fi + done < <(printf '%s' "$AUDIT_JSON" | jq -c --arg cat "$cat" '.items[$cat][] | select(.scan_error | not)') + done + + # deferred_items — same phase-scoped identification, plus --text (the + # exact bullet the audit read, which uniquely identifies the entry) + while IFS= read -r item; do + [ -z "$item" ] && continue + phase=$(printf '%s' "$item" | jq -r '.phase') + file=$(printf '%s' "$item" | jq -r '.file') + text=$(printf '%s' "$item" | jq -r '.text') + archived=$(printf '%s' "$item" | jq -r '.archived_milestone // empty') + if [ -n "$archived" ]; then + if ! gsd_run query audit-open acknowledge --category deferred_items --milestone "$MILESTONE_VERSION" --phase "$phase" --file "$file" --text "$text" --archived-milestone "$archived"; then + record_ack_failure "deferred_items phase=$phase file=$file archived-milestone=$archived" + fi + else + if ! gsd_run query audit-open acknowledge --category deferred_items --milestone "$MILESTONE_VERSION" --phase "$phase" --file "$file" --text "$text"; then + record_ack_failure "deferred_items phase=$phase file=$file" + fi + fi + done < <(printf '%s' "$AUDIT_JSON" | jq -c '.items.deferred_items[] | select(.scan_error | not)') + + if [ "$ACK_FAILURES" -gt 0 ]; then + echo "ERROR: $ACK_FAILURES acknowledge call(s) failed — HALTING before milestone close. Resolve each listed item manually (e.g. edit the file directly for unsupported_heading_shape/ambiguous, or re-run the audit if a --text/--file target has since changed) and re-run /gsd:complete-milestone:" >&2 + printf '%s\n' "$ACK_FAILURE_LOG" >&2 + exit 1 + fi + ``` + `todos` is the only category the scanner caps (5 entries per scan, with a remainder count for the rest). Re-run `gsd-tools.cjs query audit-open --json` (through the same `@file:` handling above) and repeat the `todos` block until it reports no `todos` items — every other category always returns its full open set in one pass. +3. Re-run `gsd-tools.cjs query audit-open --json` once more and write the items just acknowledged as new rows to STATE.md under `## Deferred Items` — append to the existing table (creating the section if absent) rather than overwriting it, preserving rows recorded at earlier milestone closes: ```markdown ## Deferred Items - Items acknowledged and deferred at milestone close on {date}: + Items acknowledged and deferred at milestone close, most recent first: - | Category | Item | Status | - |----------|------|--------| - | debug | {slug} | {status} | - | quick_task | {slug} | {status} | - ... + | Category | Item | Status | Deferred At | Milestone | + |----------|------|--------|-------------|-----------| + | debug_sessions | {slug} | {status} | {date} | {milestone} | + | quick_tasks | {slug} | {status} | {date} | {milestone} | + | threads | {slug} | {status} | {date} | {milestone} | + | seeds | {seed_id} | {status} | {date} | {milestone} | + | todos | {filename} | (presence-only) | {date} | {milestone} | + | uat_gaps | {phase}/{file} | {status} | {date} | {milestone} | + | verification_gaps | {phase}/{file} | {status} | {date} | {milestone} | + | context_questions | {phase}/{file} | {question_count} questions | {date} | {milestone} | + | deferred_items | {phase}/{file}: {text} | acknowledged | {date} | {milestone} | ``` - Sanitize all slug and status values via `sanitizeForDisplay()` before writing. Never inject raw file content into STATE.md. -3. Set `closeout_type=override_closeout` and record `Known verification overrides: {count} (see STATE.md Deferred Items)` in the MILESTONES.md entry. -4. Proceed with milestone close. + One row per item actually acknowledged in step 2 (omit categories with nothing to disclose this close). `{date}` is today's date; `{milestone}` is `MILESTONE_VERSION`. Sanitize all slug/status/text values via `sanitizeForDisplay()` before writing. Never inject raw file content into STATE.md. +4. Set `closeout_type=override_closeout` and record in the MILESTONES.md entry: `Known verification overrides: {N} newly acknowledged, {M} carried forward from a prior close (see STATE.md Deferred Items)` — `{N}` is the count of items acknowledged in step 2 (the pre-acknowledgment audit JSON's `counts.total`) and `{M}` is that same audit JSON's `acknowledged.total` (items a PRIOR close already suppressed and still are). +5. Proceed with milestone close. -If output shows all clear (no open items): set `closeout_type=verified_closeout`, print `All artifact types clear.`, and proceed. +Acknowledging is verdict-preserving and self-invalidating: it never rewrites the artifact's own `status:` field (except `deferred_items`, whose entry has no other meaning for that field), and the suppression it grants lapses automatically the moment the artifact's observed state changes again — a reopened debug session, an edited UAT gap, a re-triggered seed, etc. resurfaces on its own at the next audit and must be acknowledged again. -SECURITY: Audit JSON output is structured data from the `audit-open` query handler (same JSON contract as legacy `gsd-tools.cjs audit-open`) — validated and sanitized at source. When writing to STATE.md, item slugs and descriptions are sanitized via `sanitizeForDisplay()` before inclusion. Never inject raw user-supplied content into STATE.md without sanitization. +If output shows all clear (no open items): set `closeout_type=verified_closeout`. If the audit JSON's `acknowledged.total` is `0`, print `All artifact types clear.` and proceed. Otherwise the close is clean only because `{acknowledged.total}` item(s) acknowledged at an earlier milestone close are still being suppressed, not because everything was fixed this time — print `All artifact types clear ({acknowledged.total} previously acknowledged item(s) still suppressed — see STATE.md Deferred Items).` and record `Known verification overrides: 0 newly acknowledged, {acknowledged.total} carried forward from a prior close (see STATE.md Deferred Items)` in the MILESTONES.md entry before proceeding. + +SECURITY: Audit JSON output is structured data from the `audit-open` query handler (same JSON contract as legacy `gsd-tools.cjs audit-open`) — validated and sanitized at source. The `audit-open acknowledge` writer is the only path that sets the `audit_acknowledged` suppression marker — it snapshots each artifact's current state itself from the identifiers passed on the command line, so this workflow never hand-authors the marker. When writing the STATE.md disclosure table, item identifiers, statuses, and deferred-item text are sanitized via `sanitizeForDisplay()` before inclusion. Never inject raw user-supplied content into STATE.md without sanitization. diff --git a/scripts/lint-phase-enumeration-drift.cjs b/scripts/lint-phase-enumeration-drift.cjs index 0f10a09b4..b8acc8d6c 100644 --- a/scripts/lint-phase-enumeration-drift.cjs +++ b/scripts/lint-phase-enumeration-drift.cjs @@ -168,15 +168,20 @@ * inside it to match — not an enumeration of the phases directory at * all; only shaped like one because `phasesDir` is a substring of the * joined path. - * - `src/audit.cts` `scanUatGaps`, `scanVerificationGaps`, - * `scanContextQuestions`, `scanDeferredItems`: the pre-milestone-close - * audit gate (`gsd-tools.cjs audit-open`, called by `/gsd:complete- - * milestone`'s pre-close gate). Each deliberately SWEEPS EVERY phase - * directory on disk to report open UAT/VERIFICATION/CONTEXT/deferred-item - * gaps — the audit's whole purpose is catching stragglers before a - * milestone closes, so scoping it to the current milestone's window - * would hide exactly the drift (e.g. a still-open item in a phase that - * somehow fell outside the window) it exists to surface. + * - `src/audit.cts` `listAuditPhaseTargets` (#3458): the shared active-root + * enumeration for the pre-milestone-close audit gate (`gsd-tools.cjs + * audit-open`, called by `/gsd:complete-milestone`'s pre-close gate). + * `scanUatGaps`, `scanVerificationGaps`, `scanContextQuestions`, and + * `scanDeferredItems` used to each hand-roll this same readdirSync + * independently (four copies of one re-derivation — the very drift class + * this guard exists to catch); #3458 consolidated all four into this one + * function, so the exemption moved with the call site instead of + * multiplying. It deliberately SWEEPS EVERY phase directory on disk to + * report open UAT/VERIFICATION/CONTEXT/deferred-item gaps — the audit's + * whole purpose is catching stragglers before a milestone closes, so + * scoping it to the current milestone's window would hide exactly the + * drift (e.g. a still-open item in a phase that somehow fell outside the + * window) it exists to surface. * - `src/roadmap-upgrade.cts` `computeMigrationPlan`: a legacy-id-to- * milestone-prefixed-id MIGRATION. It must see and rename EVERY existing * phase directory across every milestone in one pass (a legacy phase @@ -276,7 +281,7 @@ const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'init.cts'), new Set(['detectHasPriorPhases', 'detectUiPhaseActive', 'cmdInitMilestoneOp'])], [path.join('src', 'milestone.cts'), new Set(['archivePhaseDirectories', 'cmdMilestoneComplete', 'cmdPhasesClear'])], [path.join('src', 'phase.cts'), new Set(['cmdPhasesList', 'cmdPhaseNextDecimal', 'cmdPhasePlanIndex', 'cmdPhaseInsert', 'renameDecimalPhases', 'renameIntegerPhases'])], - [path.join('src', 'audit.cts'), new Set(['scanUatGaps', 'scanVerificationGaps', 'scanContextQuestions', 'scanDeferredItems'])], + [path.join('src', 'audit.cts'), new Set(['listAuditPhaseTargets'])], [path.join('src', 'commands.cts'), new Set(['cmdHistoryDigest'])], [path.join('src', 'state.cts'), new Set(['cmdStateValidate', 'cmdStateSync', 'cmdStateRebuild'])], [path.join('src', 'roadmap-upgrade.cts'), new Set(['computeMigrationPlan'])], diff --git a/scripts/lint-plan-count-drift.cjs b/scripts/lint-plan-count-drift.cjs index ad6d02690..06eb287ce 100644 --- a/scripts/lint-plan-count-drift.cjs +++ b/scripts/lint-plan-count-drift.cjs @@ -125,9 +125,13 @@ const CORE_UTILS_EXEMPT_FUNCTIONS = new Set([ // anywhere else in these same files is still caught. Mirrors the // CORE_UTILS_EXEMPT_FUNCTIONS mechanism above, generalized per-file. // -// - audit.cts scanQuickTasks: scans a quick task's OWN directory -// (`.planning/quick//`) for that ONE task's completion record — -// not a phase directory's live-plan/summary counting question. +// - audit.cts resolveQuickTaskSummaryFile: scans a quick task's OWN +// directory (`.planning/quick//`) for that ONE task's completion +// record — not a phase directory's live-plan/summary counting question. +// #3458 follow-up extracted this out of `scanQuickTasks` (the prior +// exemption target) into its own function so `scanQuickTasks` (read) and +// `cmdAuditAcknowledge`'s quick_tasks writer share the ONE discovery +// rule instead of each re-deriving it independently. // - gsd2-import.cts readTasksDir: reads a FOREIGN GSD-2 legacy project's // `tasks/` dir convention during a one-time import, not this project's // `.planning/phases/` layout at all. @@ -162,7 +166,7 @@ const CORE_UTILS_EXEMPT_FUNCTIONS = new Set([ // owner's boolean plan/summary classification cannot answer. const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [CORE_UTILS_FILE, CORE_UTILS_EXEMPT_FUNCTIONS], - [path.join('src', 'audit.cts'), new Set(['scanQuickTasks'])], + [path.join('src', 'audit.cts'), new Set(['resolveQuickTaskSummaryFile'])], [path.join('src', 'gsd2-import.cts'), new Set(['readTasksDir'])], [path.join('src', 'estimate-cli.cts'), new Set(['collectCalibrationSamples'])], [path.join('src', 'roadmap.cts'), new Set(['cmdRoadmapAnnotateDependencies'])], diff --git a/src/audit-command-router.cts b/src/audit-command-router.cts index fb82b7665..ff23bbafd 100644 --- a/src/audit-command-router.cts +++ b/src/audit-command-router.cts @@ -41,6 +41,14 @@ interface UatModule { interface AuditModule { auditOpenArtifacts(cwd: string): unknown; formatAuditReport(result: unknown): string; + /** + * CLI writer for the #3458 follow-up suppression seam — `audit-open + * acknowledge`. Parses its OWN flags out of `args` (mirroring how `run` + * above already owns `--json` parsing for this family) rather than + * widening the Hub handler signature, since this is the only subcommand + * that needs them. + */ + cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void; } interface CoreModule { @@ -116,7 +124,7 @@ function routeAuditOpen({ args, cwd, raw, error, _audit, _core }: RouteAuditOpen routeHubCommandFamily({ family: 'audit-open', args: hubArgs, - subcommands: ['run'], + subcommands: ['run', 'acknowledge'], defaultSubcommand: 'run', handlers: { run: () => { @@ -130,9 +138,16 @@ function routeAuditOpen({ args, cwd, raw, error, _audit, _core }: RouteAuditOpen c.output(null, true, a.formatAuditReport(result)); } }, + // #3458 follow-up (design point A4): `audit-open acknowledge --category + // ... --milestone ...` writes/refreshes an `audit_acknowledged` + // suppression marker. `hubArgs.slice(2)` drops the family token and the + // `acknowledge` subcommand token itself, leaving just this + // subcommand's OWN `--flag value` pairs — the same slice + // `routeHubCommandFamily` itself passes to `hub.dispatch`'s `args`. + acknowledge: () => a.cmdAuditAcknowledge(cwd, hubArgs.slice(2), raw), }, unknownMessage: (subcommand: string) => - `Unknown audit-open subcommand: "${subcommand}". audit-open takes no subcommands (use --json for JSON output).`, + `Unknown audit-open subcommand: "${subcommand}". Available: run (default, use --json for JSON output), acknowledge.`, error, cwd, raw, diff --git a/src/audit.cts b/src/audit.cts index 6404a9ce3..fe51da7c8 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -13,6 +13,7 @@ import fs from 'node:fs'; import path from 'node:path'; +import crypto from 'node:crypto'; import { platformReadSync } from './shell-command-projection.cjs'; import { collectSection } from './markdown-sectionizer.cjs'; import { splitLines } from './text-lines.cjs'; @@ -21,11 +22,19 @@ import planningWorkspace = require('./planning-workspace.cjs'); const { planningDir } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatter = require('./frontmatter.cjs'); -const { extractFrontmatter } = frontmatter; +const { extractFrontmatter, spliceFrontmatter } = frontmatter; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod; -import { requireSafePath, sanitizeForDisplay } from './security.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseLocator = require('./phase-locator.cjs'); +const { getArchivedPhaseDirs } = phaseLocator; +import { requireSafePath, sanitizeForDisplay, sanitizeLabel } from './security.cjs'; +import { platformWriteSync } from './shell-command-projection.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import io = require('./io.cjs'); +const { output, error: ioError } = io; +import { parseNamedArgs } from './command-arg-projection.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -76,6 +85,7 @@ interface UatGapItem { status: string; open_scenario_count: number; scan_error?: boolean; + archived_milestone?: string; } interface VerificationGapItem { @@ -83,6 +93,7 @@ interface VerificationGapItem { file: string; status: string; scan_error?: boolean; + archived_milestone?: string; } interface ContextQuestionItem { @@ -91,6 +102,7 @@ interface ContextQuestionItem { question_count: number; questions: string[]; scan_error?: boolean; + archived_milestone?: string; } interface DeferredItem { @@ -98,6 +110,7 @@ interface DeferredItem { file: string; text: string; scan_error?: boolean; + archived_milestone?: string; } /** @@ -107,6 +120,43 @@ interface DeferredItem { */ interface UatDeferredModule { parseDeferredItems(content: string): Array<{ name: string }>; + /** + * #3458 follow-up: like `parseDeferredItems`, but returns EVERY entry + * (including `status: resolved` ones) together with its raw, unlowercased + * `status:` field value (`''` when absent) so a caller can distinguish + * "resolved" (fixed for real — never counted) from the new "acknowledged" + * (suppressed-but-tallied) from anything else (open). `parseDeferredItems` + * itself is defined in terms of this — see uat.cts — so the two can never + * drift on what an entry's text/boundaries are. + */ + parseDeferredItemsWithStatus(content: string): Array<{ name: string; status: string }>; + /** + * Writer half of the #3458 follow-up seam (A4): sets a matched deferred + * entry's `status:` field to `acknowledged` in place, verdict-preserving + * (never touches an entry already `status: resolved`) and scoped to the + * BULLET-only (headless) `## Deferred Items` shape — see the doc comment on + * the implementation in uat.cts for why the heading-delimited (#3457) shape + * is refused rather than attempted. + */ + acknowledgeDeferredItem(content: string, targetText: string): AcknowledgeDeferredItemResult; +} + +/** Result of `UatDeferredModule.acknowledgeDeferredItem`. */ +interface AcknowledgeDeferredItemResult { + content: string; + status: 'ok' | 'not_found' | 'ambiguous' | 'unsupported_heading_shape' | 'already_resolved' | 'match_verification_failed'; +} + +/** + * A scanner's items PLUS how many otherwise-open items it suppressed via a + * current `audit_acknowledged` marker (#3458 follow-up, A5). Every + * phase-scoped and flat scanner returns this shape now instead of a bare + * array, so `auditOpenArtifacts` can report both halves without a second + * scan pass. + */ +interface ScanOutcome { + items: T[]; + acknowledged: number; } interface AuditCounts { @@ -126,6 +176,14 @@ interface AuditResult { scanned_at: string; has_open_items: boolean; counts: AuditCounts; + /** + * Per-category count of items SUPPRESSED by a current (non-stale) + * `audit_acknowledged` marker (#3458 follow-up, design point A5) — mirrors + * `counts`'s shape exactly. A suppressed item never appears in `counts` or + * `items`; this is the only place it is still observable, so a reviewer can + * tell "clean because fixed" apart from "clean because silenced". + */ + acknowledged: AuditCounts; items: { debug_sessions: DebugSessionItem[]; quick_tasks: QuickTaskItem[]; @@ -148,6 +206,216 @@ const DEFERRED_ITEMS_FILENAME = 'deferred-items.md'; // not recreated on each loop iteration. const TERMINAL_UAT_STATUSES = new Set(['complete', 'resolved']); +// ─── Acknowledgment marker (suppression) ────────────────────────────────────── +// +// #3458 follow-up: `query audit-open` now scans archived milestone phase dirs, +// so an item still unresolved when a milestone closed resurfaces at EVERY +// later close, forever — `[A] Acknowledge all` documented that decision to +// STATE.md but never suppressed it. This section is the suppression seam. +// +// The marker lives INSIDE the artifact it suppresses, as an +// `audit_acknowledged` frontmatter map (no ledger, no id minting — see +// `uat.cts:891-897`'s `deferred-items.md` in-place `status: resolved` +// convention, which this generalizes): +// +// audit_acknowledged: +// milestone: v1.0 # which milestone close acknowledged it +// at: 2026-08-15 # ISO date +// status: gaps_found # snapshot of the artifact's state AT acknowledgment +// # (named `gap_snapshot` — status + open-scenario +// # count — for `uat_gaps`, and `questions_digest` +// # — a content hash of the question set, not just +// # its count — for `context_questions`; see +// # `isAuditItemAcknowledged`'s `snapshotKey` param +// # and each category's `deriveXxx` snapshot +// # helper for why a bare status/count was not +// # enough for those two — #3458 follow-up review) +// +// It is VERDICT-PRESERVING (this section never writes `status:` itself — see +// `cmdAuditAcknowledge` below) and SELF-INVALIDATING: it suppresses ONLY while +// `snapshotKey`'s recorded value still equals the artifact's CURRENT +// effective value. Edit the artifact after acknowledging it and the item +// resurfaces automatically — no separate revive/carry-forward state, and a +// stale acknowledgment can never hide a NEW problem, PROVIDED the category's +// snapshot actually captures the dimension that changed — `uat_gaps` and +// `context_questions` snapshot more than their status/count for exactly this +// reason (see above); every other category's only tracked dimension IS its +// `status:` (or, for `todos`, presence), so a bare status/presence snapshot +// is already complete for those. +// +// `isAuditItemAcknowledged` is the ONE shared predicate every scanner below +// routes through — this file has already been through the "hand-rolled the +// same check nine times" defect family twice this PR; a tenth hand-roll here +// is exactly that class. `deferred_items` is the deliberate exception: its +// suppression key lives PER-ENTRY inside `deferred-items.md`'s own +// `status:` field (see `uat.cts`'s `parseDeferredItemsWithStatus`), not in a +// file-level `audit_acknowledged` map, because a single deferred-items.md can +// carry many independently-acknowledgeable entries. + +/** + * Parse and validate an artifact's `audit_acknowledged` frontmatter marker, + * then decide whether it suppresses the item given the artifact's CURRENT + * effective state. + * + * `snapshotKey` names which sub-field of the marker map carries the snapshot + * comparison value (`'status'` for every category except CONTEXT files, which + * use `'question_count'`). `presenceOnly: true` (used only for `todos`, which + * has no natural status field to snapshot) skips the snapshot comparison + * entirely — marker PRESENCE alone suppresses. + * + * A marker that is not a plain object/map, or is missing a non-empty string + * `milestone`/`at`, or — when a snapshot comparison applies — missing a + * string at `snapshotKey`, is MALFORMED and treated as ABSENT: this function + * returns `false` and the item surfaces. A bad marker must never suppress. + */ +function isAuditItemAcknowledged( + fm: Record, + opts: { snapshotKey: string; currentValue: string; presenceOnly?: boolean }, +): boolean { + const raw = fm.audit_acknowledged; + if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return false; + const marker = raw as Record; + if (typeof marker.milestone !== 'string' || !marker.milestone) return false; + if (typeof marker.at !== 'string' || !marker.at) return false; + if (opts.presenceOnly) return true; + const snapshot = marker[opts.snapshotKey]; + if (typeof snapshot !== 'string') return false; + return snapshot === opts.currentValue; +} + +/** + * Derive a THREAD file's effective status: frontmatter `status:` when + * present, else the `## Status: OPEN|IN PROGRESS` body fallback — the same + * two-step derivation `scanThreads` already performed inline. Extracted so + * `cmdAuditAcknowledge` computes the CURRENT snapshot value with the exact + * same logic the scanner used to produce the marker's recorded value, + * instead of a second hand-derivation that could silently drift from it. + */ +function deriveThreadStatus(fm: Record, content: string): string { + let status = ((fm.status as string) || '').toLowerCase().trim(); + if (!status) { + const bodyStatusMatch = content.match(/##\s*Status:\s*(OPEN|IN PROGRESS|IN_PROGRESS)/i); + if (bodyStatusMatch) { + status = bodyStatusMatch[1].toLowerCase().replace(/ /g, '_'); + } + } + return status; +} + +/** + * Count a UAT file's still-open (`result: pending`/`[pending]`) scenarios. + * Extracted from `scanUatGaps`'s inline logic for the same reason as + * `deriveThreadStatus` — one derivation, shared by the scanner and + * `cmdAuditAcknowledge`'s `deriveUatGapSnapshotValue` below. + */ +function deriveUatGapOpenScenarioCount(content: string): number { + return (content.match(/result:\s*(?:pending|\[pending\])/gi) || []).length; +} + +/** + * Stable snapshot value for a `uat_gaps` item (WARNING 2, #3458 follow-up + * review). `status` alone is COUNT-blind the other direction: a UAT file can + * stay in the SAME open status (`gaps_found`) while gaining MORE pending + * scenarios (measured: 1→6 pending, status unchanged, item stayed + * suppressed under the old status-only scheme). Composing `status` with the + * open-scenario count means either dimension changing invalidates the + * snapshot. + */ +function deriveUatGapSnapshotValue(status: string, content: string): string { + return `${status}::scenarios=${deriveUatGapOpenScenarioCount(content)}`; +} + +/** + * Derive a CONTEXT file's FULL, UNTRUNCATED open-questions list: the + * structured `open_questions` frontmatter array when present and + * non-empty, else EVERY qualifying line of the `## Open Questions` body + * section. Extracted from `scanContextQuestions`'s inline logic for the same + * reason as `deriveThreadStatus` — one derivation, shared by the scanner and + * `cmdAuditAcknowledge`, so the acknowledged `question_count`/digest snapshot + * can never diverge from what the scanner counts. + * + * F2 (#3458 follow-up review, sibling of the deferred_items span-carrying + * fix): this used to `slice(0, 3)` the body-section list AND clamp each + * question to 200 chars BEFORE returning — a value meant for DISPLAY reused + * for the IDENTITY snapshot `deriveOpenQuestionsDigest` hashes. A 4th+ + * question, or anything past char 200 of an earlier one, was invisible to + * the digest: an attacker could ship 3 innocuous questions first, then add + * real blockers afterward with zero effect on the recorded snapshot. Every + * caller that wants a bounded list for DISPLAY (`scanContextQuestions`'s + * `questions` field) truncates its OWN copy at the call site; this function + * always returns the complete, unclamped set. + */ +function deriveOpenQuestions(content: string, fm: Record): string[] { + let questions: string[] = []; + if (fm.open_questions) { + if (Array.isArray(fm.open_questions) && fm.open_questions.length > 0) { + questions = (fm.open_questions as unknown[]).map(q => sanitizeForDisplay(String(q))); + } + } + + if (questions.length === 0) { + const oqSection = collectSection(content, (h) => h.level === 2 && h.text.trim().toLowerCase().startsWith('open questions'), { levelBounded: true }); + if (oqSection) { + const oqBody = oqSection.body.trim(); + if (oqBody && oqBody.length > 0 && !/^\s*none\s*$/i.test(oqBody)) { + const items = oqBody.split('\n') + .map((l: string) => l.trim()) + .filter((l: string) => l && l !== '-' && l !== '*') + .filter((l: string) => /^[-*\d]/.test(l) || l.includes('?')); + questions = items.map((q: string) => sanitizeForDisplay(q)); + } + } + } + + return questions; +} + +/** Bound a question's DISPLAY text (never fed into the identity digest — see `deriveOpenQuestions`'s doc comment). */ +function truncateQuestionForDisplay(question: string): string { + return question.slice(0, 200); +} + +/** + * Stable normalized digest of a CONTEXT file's open-questions set (WARNING 2, + * #3458 follow-up review). `question_count` alone is COUNT-only: replacing + * every question's TEXT with brand-new ones while holding the count steady + * left an acknowledged item permanently suppressed (measured: 2 questions + * acknowledged, then both replaced with unrelated new blockers — still + * `counts:0`). Hashing the full, ordered question text means ANY edit — + * add, remove, reword, or reorder — changes the digest and the item + * resurfaces. sha256 (not the raw joined string) keeps the marker's stored + * value bounded regardless of question length/count. + * + * `questions` MUST be the untruncated, unclamped set `deriveOpenQuestions` + * returns — never a display-sliced/-clamped copy (F2, #3458 follow-up + * review); a truncated input reintroduces exactly the blind spot this digest + * exists to close. + * + * Length-prefixed, separator-free encoding (SWEEP finding, #3458 follow-up + * review) — NOT a plain join (the prior revision joined on a literal + * embedded NUL byte, `questions.join('\\0')` written as a raw control + * character in the SOURCE FILE itself — invisible in a normal diff/editor + * and still forgeable: attacker-controlled markdown CAN contain a literal + * NUL codepoint, since the file is read as UTF-8 text, so that scheme never + * actually closed the boundary-collision gap it was reaching for). A bare + * separator-joined string has no reliably unambiguous element boundary: two + * DIFFERENT question arrays can render the identical joined string and + * collide on the same digest — e.g. `['- Is X ready?', '- Y done?']` and + * `['- Is X ready? - Y', 'done?']` both join to + * `'- Is X ready? - Y done?'` under a space-join, and both could be forced to + * collide under a NUL-join too by an attacker who embeds the separator + * itself. Prefixing each element with its own CHARACTER LENGTH + * (`:`, concatenated with no separator at all) makes the encoding + * self-delimiting instead: decoding always consumes exactly `` + * characters after each `:` before reading the next length prefix, so no two + * distinct arrays can ever encode to the same string — regardless of what + * characters the questions themselves contain. + */ +function deriveOpenQuestionsDigest(questions: string[]): string { + const encoded = questions.map((q) => `${q.length}:${q}`).join(''); + return crypto.createHash('sha256').update(encoded).digest('hex'); +} + // ─── scanDebugSessions ──────────────────────────────────────────────────────── /** @@ -155,16 +423,17 @@ const TERMINAL_UAT_STATUSES = new Set(['complete', 'resolved']); * Open = status NOT in ['resolved', 'complete']. * Ignores the resolved/ subdirectory. */ -function scanDebugSessions(planDir: string): DebugSessionItem[] { +function scanDebugSessions(planDir: string): ScanOutcome { const debugDir = path.join(planDir, 'debug'); - if (!fs.existsSync(debugDir)) return []; + if (!fs.existsSync(debugDir)) return { items: [], acknowledged: 0 }; const results: DebugSessionItem[] = []; + let acknowledged = 0; let files: fs.Dirent[]; try { files = fs.readdirSync(debugDir, { withFileTypes: true }); } catch { - return [{ scan_error: true, slug: '', status: '', updated: '', hypothesis: '' }]; + return { items: [{ scan_error: true, slug: '', status: '', updated: '', hypothesis: '' }], acknowledged: 0 }; } for (const entry of files) { @@ -187,6 +456,11 @@ function scanDebugSessions(planDir: string): DebugSessionItem[] { const status = ((fm.status as string) || 'unknown').toLowerCase(); if (status === 'resolved' || status === 'complete') continue; + if (isAuditItemAcknowledged(fm, { snapshotKey: 'status', currentValue: status })) { + acknowledged++; + continue; + } + // Extract hypothesis from "Current Focus" block if parseable let hypothesis = ''; const focusSection = collectSection(content, (h) => h.level === 2 && h.text.trim().toLowerCase().startsWith('current focus'), { levelBounded: true }); @@ -197,14 +471,56 @@ function scanDebugSessions(planDir: string): DebugSessionItem[] { const slug = path.basename(entry.name, '.md'); results.push({ - slug: sanitizeForDisplay(slug), + slug: sanitizeLabel(slug), status: sanitizeForDisplay(status), updated: sanitizeForDisplay(fm.updated || fm.date || ''), hypothesis, }); } - return results; + return { items: results, acknowledged }; +} + +// ─── resolveQuickTaskSummaryFile ─────────────────────────────────────────────── + +/** + * Resolve a quick task's SUMMARY file, if any exists, under its own + * directory (`taskDir`). workflows/quick.md mandates `${quick_id}-SUMMARY.md`; + * older flows used bare `SUMMARY.md` — accept either to avoid a + * false-positive "missing", preferring the per-task `${dirName}-SUMMARY.md` + * form when more than one candidate exists. + * + * #3183 (ADR-3180 Decision 4(a) — bucket B, out of scope for the + * scanPhasePlans migration): this scans a quick task's OWN directory + * (`.planning/quick//`) for THAT task's single completion record — + * "does this one quick task have a SUMMARY.md" — not a phase directory's + * live-plan/summary counting question. scanPhasePlans is the wrong tool + * here; there is no plan/summary PAIRING to derive, only a single filename + * presence check local to a non-phase directory. + * + * Extracted (#3458 follow-up) so `scanQuickTasks` (read) and + * `cmdAuditAcknowledge`'s quick_tasks writer share the ONE discovery rule — + * previously the writer would have had to hand-roll this exact filter a + * second time, which is exactly the re-derivation-drift class + * `scripts/lint-plan-count-drift.cjs` exists to catch (see its + * `FUNCTION_SCOPED_EXEMPTIONS` entry for this function). + * + * Returns `null` (never throws) on an unreadable `taskDir` or when no + * SUMMARY-shaped file exists. + */ +function resolveQuickTaskSummaryFile(taskDir: string, dirName: string): string | null { + let summaryFiles: fs.Dirent[]; + try { + summaryFiles = fs.readdirSync(taskDir, { withFileTypes: true }) + .filter(e => e.isFile() && (e.name === 'SUMMARY.md' || e.name.endsWith('-SUMMARY.md'))); + } catch { + return null; + } + if (summaryFiles.length === 0) return null; + const preferred = summaryFiles.find(e => e.name === `${dirName}-SUMMARY.md`) + || summaryFiles.find(e => e.name.endsWith('-SUMMARY.md')) + || summaryFiles[0]; + return path.join(taskDir, preferred.name); } // ─── scanQuickTasks ─────────────────────────────────────────────────────────── @@ -213,18 +529,19 @@ function scanDebugSessions(planDir: string): DebugSessionItem[] { * Scan .planning/quick/ for incomplete tasks. * Incomplete if SUMMARY.md missing or status !== 'complete'. */ -function scanQuickTasks(planDir: string): QuickTaskItem[] { +function scanQuickTasks(planDir: string): ScanOutcome { const quickDir = path.join(planDir, 'quick'); - if (!fs.existsSync(quickDir)) return []; + if (!fs.existsSync(quickDir)) return { items: [], acknowledged: 0 }; let entries: fs.Dirent[]; try { entries = fs.readdirSync(quickDir, { withFileTypes: true }); } catch { - return [{ scan_error: true, slug: '', date: '', status: '', description: '' }]; + return { items: [{ scan_error: true, slug: '', date: '', status: '', description: '' }], acknowledged: 0 }; } const results: QuickTaskItem[] = []; + let acknowledged = 0; for (const entry of entries) { if (!entry.isDirectory()) continue; @@ -238,33 +555,11 @@ function scanQuickTasks(planDir: string): QuickTaskItem[] { continue; } - // workflows/quick.md mandates `${quick_id}-SUMMARY.md`; older flows used - // bare `SUMMARY.md`. Accept either to avoid false-positive "missing". - // - // #3183 (ADR-3180 Decision 4(a) — bucket B, out of scope for the - // scanPhasePlans migration): this scans a quick task's OWN directory - // (`.planning/quick//`) for THAT task's single completion record — - // "does this one quick task have a SUMMARY.md" — not a phase directory's - // live-plan/summary counting question. scanPhasePlans is the wrong tool - // here; there is no plan/summary PAIRING to derive, only a single - // filename presence check local to a non-phase directory. - let summaryPath: string | null = null; - try { - const summaryFiles = fs.readdirSync(safeTaskDir, { withFileTypes: true }) - .filter(e => e.isFile() && (e.name === 'SUMMARY.md' || e.name.endsWith('-SUMMARY.md'))); - if (summaryFiles.length > 0) { - // Prefer the per-task `${quick_id}-SUMMARY.md` form when present. - const preferred = summaryFiles.find(e => e.name === `${dirName}-SUMMARY.md`) - || summaryFiles.find(e => e.name.endsWith('-SUMMARY.md')) - || summaryFiles[0]; - summaryPath = path.join(safeTaskDir, preferred.name); - } - } catch { - // fall through with summaryPath = null → status: missing - } + const summaryPath = resolveQuickTaskSummaryFile(safeTaskDir, dirName); let status = 'missing'; const description = ''; + let fm: Record | null = null; if (summaryPath && fs.existsSync(summaryPath)) { let safeSum: string; @@ -277,20 +572,33 @@ function scanQuickTasks(planDir: string): QuickTaskItem[] { if (content === null) { status = 'unreadable'; } else { - const fm = extractFrontmatter(content, safeSum); + fm = extractFrontmatter(content, safeSum); status = ((fm.status as string) || 'unknown').toLowerCase(); } } if (status === 'complete') continue; + // Acknowledgment marker only ever lives in the SUMMARY file's own + // frontmatter — a task with no summary (status: 'missing') has nowhere to + // carry one, so `fm` is null and this is skipped (never suppressed). + if (fm && isAuditItemAcknowledged(fm, { snapshotKey: 'status', currentValue: status })) { + acknowledged++; + continue; + } + // Parse date and slug from directory name: YYYYMMDD-slug or YYYY-MM-DD-slug let date = ''; - let slug = sanitizeForDisplay(dirName); + let slug = sanitizeLabel(dirName); const dateMatch = dirName.match(/^(\d{4}-?\d{2}-?\d{2})-(.+)$/); if (dateMatch) { - date = dateMatch[1]; - slug = sanitizeForDisplay(dateMatch[2]); + // dateMatch[1] is regex-constrained to `\d{4}-?\d{2}-?\d{2}` (digits and + // literal hyphens only — the same "constrained at the source" shape as + // `archived_milestone`), so it cannot itself carry a control byte. + // Still routed through sanitizeLabel as defense-in-depth for + // consistency with every other directory-name-derived field here. + date = sanitizeLabel(dateMatch[1]); + slug = sanitizeLabel(dateMatch[2]); } results.push({ @@ -301,7 +609,7 @@ function scanQuickTasks(planDir: string): QuickTaskItem[] { }); } - return results; + return { items: results, acknowledged }; } // ─── scanThreads ────────────────────────────────────────────────────────────── @@ -310,19 +618,20 @@ function scanQuickTasks(planDir: string): QuickTaskItem[] { * Scan .planning/threads/ for open threads. * Open if status in ['open', 'in_progress', 'in progress'] (case-insensitive). */ -function scanThreads(planDir: string): ThreadItem[] { +function scanThreads(planDir: string): ScanOutcome { const threadsDir = path.join(planDir, 'threads'); - if (!fs.existsSync(threadsDir)) return []; + if (!fs.existsSync(threadsDir)) return { items: [], acknowledged: 0 }; let files: fs.Dirent[]; try { files = fs.readdirSync(threadsDir, { withFileTypes: true }); } catch { - return [{ scan_error: true, slug: '', status: '', updated: '', title: '' }]; + return { items: [{ scan_error: true, slug: '', status: '', updated: '', title: '' }], acknowledged: 0 }; } const openStatuses = new Set(['open', 'in_progress', 'in progress']); const results: ThreadItem[] = []; + let acknowledged = 0; for (const entry of files) { if (!entry.isFile()) continue; @@ -341,18 +650,15 @@ function scanThreads(planDir: string): ThreadItem[] { if (content === null) continue; const fm = extractFrontmatter(content, safeFilePath); - let status = ((fm.status as string) || '').toLowerCase().trim(); - - // Fall back to scanning body for ## Status: OPEN / IN PROGRESS - if (!status) { - const bodyStatusMatch = content.match(/##\s*Status:\s*(OPEN|IN PROGRESS|IN_PROGRESS)/i); - if (bodyStatusMatch) { - status = bodyStatusMatch[1].toLowerCase().replace(/ /g, '_'); - } - } + const status = deriveThreadStatus(fm, content); if (!openStatuses.has(status)) continue; + if (isAuditItemAcknowledged(fm, { snapshotKey: 'status', currentValue: status })) { + acknowledged++; + continue; + } + // Extract title from # Thread: heading or frontmatter title let title = sanitizeForDisplay(fm.title || ''); if (!title) { @@ -364,14 +670,14 @@ function scanThreads(planDir: string): ThreadItem[] { const slug = path.basename(entry.name, '.md'); results.push({ - slug: sanitizeForDisplay(slug), + slug: sanitizeLabel(slug), status: sanitizeForDisplay(status), updated: sanitizeForDisplay(fm.updated || fm.date || ''), title, }); } - return results; + return { items: results, acknowledged }; } // ─── scanTodos ──────────────────────────────────────────────────────────────── @@ -381,22 +687,31 @@ function scanThreads(planDir: string): ThreadItem[] { * Returns array of { filename, priority, area, summary }. * Display limited to first 5 + count of remainder. */ -function scanTodos(planDir: string): TodoItem[] { +function scanTodos(planDir: string): ScanOutcome { const pendingDir = path.join(planDir, 'todos', 'pending'); - if (!fs.existsSync(pendingDir)) return []; + if (!fs.existsSync(pendingDir)) return { items: [], acknowledged: 0 }; let files: fs.Dirent[]; try { files = fs.readdirSync(pendingDir, { withFileTypes: true }); } catch { - return [{ scan_error: true, filename: '', priority: '', area: '', summary: '' }]; + return { items: [{ scan_error: true, filename: '', priority: '', area: '', summary: '' }], acknowledged: 0 }; } const mdFiles = files.filter(e => e.isFile() && e.name.endsWith('.md')); const results: TodoItem[] = []; + let acknowledged = 0; - const displayFiles = mdFiles.slice(0, 5); - for (const entry of displayFiles) { + // BLOCKER 2 (#3458 follow-up review): filter acknowledged items BEFORE + // the display cap. Capping the RAW file list to 5 first meant an + // acknowledge of one of those 5 files simply revealed the 6th on the next + // scan — files 6/7/... (never shown, never acknowledgeable via the CLI's + // own remedy) permanently vanished from every later scan once 5+ items + // existed, because `mdFiles.length` (not the post-filter open count) drove + // both the cap and the remainder count. Read every file's acknowledgment + // state first, THEN cap the OPEN (unacknowledged) set for display. + const openFiles: { entry: fs.Dirent; content: string; fm: Record }[] = []; + for (const entry of mdFiles) { const filePath = path.join(pendingDir, entry.name); let safeFilePath: string; @@ -411,24 +726,38 @@ function scanTodos(planDir: string): TodoItem[] { const fm = extractFrontmatter(content, safeFilePath); + // Todos carry no natural status field — presence in pending/ IS "open" by + // definition (a resolved todo is moved out, not status-flagged). So the + // acknowledgment check here is PRESENCE-ONLY: no snapshot to go stale, no + // self-invalidation on edit — see `isAuditItemAcknowledged`'s doc comment. + if (isAuditItemAcknowledged(fm, { snapshotKey: 'status', currentValue: '', presenceOnly: true })) { + acknowledged++; + continue; + } + + openFiles.push({ entry, content, fm }); + } + + const displayFiles = openFiles.slice(0, 5); + for (const { entry, content, fm } of displayFiles) { // Extract first line of body after frontmatter const bodyMatch = content.replace(/^---[\s\S]*?---\r?\n?/, ''); const firstLine = splitLines(bodyMatch.trim())[0] || ''; const summary = sanitizeForDisplay(firstLine.slice(0, 100)); results.push({ - filename: sanitizeForDisplay(entry.name), + filename: sanitizeLabel(entry.name), priority: sanitizeForDisplay(fm.priority || ''), area: sanitizeForDisplay(fm.area || ''), summary, }); } - if (mdFiles.length > 5) { - results.push({ _remainder_count: mdFiles.length - 5, filename: '', priority: '', area: '', summary: '' }); + if (openFiles.length > 5) { + results.push({ _remainder_count: openFiles.length - 5, filename: '', priority: '', area: '', summary: '' }); } - return results; + return { items: results, acknowledged }; } // ─── scanSeeds ──────────────────────────────────────────────────────────────── @@ -437,19 +766,20 @@ function scanTodos(planDir: string): TodoItem[] { * Scan .planning/seeds/SEED-*.md for unimplemented seeds. * Unimplemented if status in ['dormant', 'active', 'triggered']. */ -function scanSeeds(planDir: string): SeedItem[] { +function scanSeeds(planDir: string): ScanOutcome { const seedsDir = path.join(planDir, 'seeds'); - if (!fs.existsSync(seedsDir)) return []; + if (!fs.existsSync(seedsDir)) return { items: [], acknowledged: 0 }; let files: fs.Dirent[]; try { files = fs.readdirSync(seedsDir, { withFileTypes: true }); } catch { - return [{ scan_error: true, seed_id: '', slug: '', status: '', title: '' }]; + return { items: [{ scan_error: true, seed_id: '', slug: '', status: '', title: '' }], acknowledged: 0 }; } const unimplementedStatuses = new Set(['dormant', 'active', 'triggered']); const results: SeedItem[] = []; + let acknowledged = 0; for (const entry of files) { if (!entry.isFile()) continue; @@ -472,10 +802,21 @@ function scanSeeds(planDir: string): SeedItem[] { if (!unimplementedStatuses.has(status)) continue; - // Extract seed_id from filename or frontmatter + if (isAuditItemAcknowledged(fm, { snapshotKey: 'status', currentValue: status })) { + acknowledged++; + continue; + } + + // Extract seed_id from filename or frontmatter. The regex match is + // `\w`/hyphen-constrained (safe by construction, like `archived_milestone`) + // but the fallback taken when a filename doesn't fully match — e.g. a + // `SEED-`-prefixed, `.md`-suffixed name with a control byte SOMEWHERE in + // the middle, which still passes the `startsWith`/`endsWith` filter above + // — is the raw, unconstrained basename. Both branches are routed through + // sanitizeLabel below. const seedIdMatch = entry.name.match(/^(SEED-[\w-]+)\.md$/); const seed_id = seedIdMatch ? seedIdMatch[1] : path.basename(entry.name, '.md'); - const slug = sanitizeForDisplay(seed_id.replace(/^SEED-/, '')); + const slug = sanitizeLabel(seed_id.replace(/^SEED-/, '')); let title = sanitizeForDisplay(fm.title || ''); if (!title) { @@ -484,45 +825,129 @@ function scanSeeds(planDir: string): SeedItem[] { } results.push({ - seed_id: sanitizeForDisplay(seed_id), + seed_id: sanitizeLabel(seed_id), slug, status: sanitizeForDisplay(status), title, }); } - return results; + return { items: results, acknowledged }; +} + +// ─── listAuditPhaseTargets ──────────────────────────────────────────────────── + +interface AuditPhaseTarget { + dir: string; + fullPath: string; + milestone?: string; +} + +/** + * Enumerate phase directories across BOTH the active `.planning/phases/` root + * and every archived `.planning/milestones/vX.Y-phases/` root. Shared by the + * four phase-scoped scanners below (#3458 — epic #3473 B2/F2). Previously each + * scanner hand-rolled its own active-only `readdirSync(phasesDir)` walk and + * bailed out entirely when the active root was missing, so items still + * unresolved when a milestone closed and its phase dirs archived became + * permanently invisible to every later audit. + * + * ACTIVE dirs: raw readdirSync + isDirectory filter + sort. The enumeration + * walk itself (readdirSync + isDirectory filter + sort) is UNCHANGED from the + * scanners' prior inline behavior; what IS new is that a failed read here no + * longer aborts the whole scan the way each scanner's own inline + * `if (!fs.existsSync) return []` / try-readdirSync-catch-return-sentinel + * pair used to — see `activeUnreadable` below, which is how that signal is + * now surfaced to callers instead. Deliberately NOT routed through + * listMilestonePhaseDirs: these scanners are deliberately not + * milestone-filtered today, and switching would silently add window/sentinel + * filtering — a behavior change belonging to #3372, not here. + * + * A missing/unreadable active root does NOT short-circuit the archive walk — + * the old `if (!fs.existsSync(phasesDir)) return []` was the whole bug in a + * fully-archived project; it degrades to "skip the active half" only. An + * UNREADABLE (as opposed to merely absent) active root is reported back via + * `activeUnreadable: true` so each of the four callers can still emit the + * `scan_error` sentinel they emitted pre-#3458 for this exact case (a real + * I/O failure, not "verified clean") — see each scanner's own use of it. + * + * ARCHIVED dirs: sourced from `getArchivedPhaseDirs` (phase-locator.cjs), the + * canonical archive-enumeration seam `uat.cts`'s `cmdAuditUat` already uses. + * Archived dirs are deliberately NOT milestone-filtered either — see the + * comment at src/uat.cts:107-111: listMilestonePhaseDirs derives the CURRENT + * milestone's phase dirs (window + sentinel filtered) from ROADMAP.md, and + * archived phases belong to past milestones by definition, so filtering them + * discards every one and silently reinstates the bug this function fixes. + * + * An unreadable/unresolvable ARCHIVE root does NOT get its own sentinel. + * Pre-#3458 there was no archived read at all, so — unlike the active root — + * there is no prior `scan_error` contract to preserve here, and no existing + * consumer can regress. It also degrades the same way `listArchiveVersionDirs` + * (phase-locator.cts) already treats an absent `milestones/` dir: a real + * empty, not a failure, matching this function's existing "skip that root" + * idiom for the missing-active-dir case above. Adding a second sentinel path + * would let a machine consumer conflate "no milestones archived yet" (the + * overwhelmingly common case for an active project) with an actual read + * failure, which is a worse signal-to-noise trade than the one this + * function's own fix removes for the active root. + * + * Same-named dirs in both roots (e.g. "01-alpha" active AND archived) are + * DISTINCT targets — no dedupe. + */ +function listAuditPhaseTargets(planDir: string, cwd: string): { targets: AuditPhaseTarget[]; activeUnreadable: boolean } { + const targets: AuditPhaseTarget[] = []; + let activeUnreadable = false; + + const phasesDir = path.join(planDir, 'phases'); + if (fs.existsSync(phasesDir)) { + try { + const dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) + .filter(e => e.isDirectory()) + .map(e => e.name) + .sort(); + for (const dir of dirs) { + targets.push({ dir, fullPath: path.join(phasesDir, dir) }); + } + } catch { + // Unreadable active root: skip it, do not abort the archive walk, but + // report it so callers can emit their pre-#3458 scan_error sentinel. + activeUnreadable = true; + } + } + + try { + for (const archived of getArchivedPhaseDirs(cwd)) { + targets.push({ dir: archived.name, fullPath: archived.fullPath, milestone: archived.milestone }); + } + } catch { + // Unreadable/unresolvable archive root: skip it, keep whatever active + // targets were already collected. No sentinel — see docstring above. + } + + return { targets, activeUnreadable }; } // ─── scanUatGaps ────────────────────────────────────────────────────────────── /** - * Scan .planning/phases for UAT gaps (UAT files with status != 'complete'). + * Scan .planning/phases (active) and .planning/milestones/vX.Y-phases (archived) + * for UAT gaps (UAT files with status != 'complete'/'resolved'). */ -function scanUatGaps(planDir: string): UatGapItem[] { - const phasesDir = path.join(planDir, 'phases'); - if (!fs.existsSync(phasesDir)) return []; - - let dirs: string[]; - try { - dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()) - .map(e => e.name) - .sort(); - } catch { - return [{ scan_error: true, phase: '', file: '', status: '', open_scenario_count: 0 }]; +function scanUatGaps(planDir: string, cwd: string): ScanOutcome { + const results: UatGapItem[] = []; + let acknowledged = 0; + const { targets, activeUnreadable } = listAuditPhaseTargets(planDir, cwd); + if (activeUnreadable) { + results.push({ scan_error: true, phase: '', file: '', status: '', open_scenario_count: 0 }); } - const results: UatGapItem[] = []; - - for (const dir of dirs) { - const phaseDir = path.join(phasesDir, dir); - const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); - const phaseNum = phaseMatch ? phaseMatch[1] : dir; + for (const target of targets) { + const phaseMatch = target.dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); + const phaseNum = phaseMatch ? phaseMatch[1] : target.dir; let files: string[]; try { - files = fs.readdirSync(phaseDir); + files = fs.readdirSync(target.fullPath); } catch { continue; } @@ -532,9 +957,9 @@ function scanUatGaps(planDir: string): UatGapItem[] { // scanVerificationGaps below. for (const file of scopeToPhase( files.filter(f => f.includes('-UAT') && f.endsWith('.md')), - dir, + target.dir, )) { - const filePath = path.join(phaseDir, file); + const filePath = path.join(target.fullPath, file); let safeFilePath: string; try { @@ -555,50 +980,50 @@ function scanUatGaps(planDir: string): UatGapItem[] { if (TERMINAL_UAT_STATUSES.has(status)) continue; if (status === 'unknown' && result === 'all_pass') continue; - // Count open scenarios - const pendingMatches = (content.match(/result:\s*(?:pending|\[pending\])/gi) || []).length; + // Count open scenarios — computed BEFORE the acknowledged check + // (WARNING 2) so the snapshot comparison sees it too, not just status. + const pendingMatches = deriveUatGapOpenScenarioCount(content); - results.push({ - phase: sanitizeForDisplay(phaseNum), - file: sanitizeForDisplay(file), + if (isAuditItemAcknowledged(fm, { snapshotKey: 'gap_snapshot', currentValue: deriveUatGapSnapshotValue(status, content) })) { + acknowledged++; + continue; + } + + const item: UatGapItem = { + phase: sanitizeLabel(phaseNum), + file: sanitizeLabel(file), status: sanitizeForDisplay(status), open_scenario_count: pendingMatches, - }); + }; + if (target.milestone !== undefined) item.archived_milestone = sanitizeLabel(target.milestone); + results.push(item); } } - return results; + return { items: results, acknowledged }; } // ─── scanVerificationGaps ───────────────────────────────────────────────────── /** - * Scan .planning/phases for VERIFICATION gaps. + * Scan .planning/phases (active) and .planning/milestones/vX.Y-phases (archived) + * for VERIFICATION gaps. */ -function scanVerificationGaps(planDir: string): VerificationGapItem[] { - const phasesDir = path.join(planDir, 'phases'); - if (!fs.existsSync(phasesDir)) return []; - - let dirs: string[]; - try { - dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()) - .map(e => e.name) - .sort(); - } catch { - return [{ scan_error: true, phase: '', file: '', status: '' }]; +function scanVerificationGaps(planDir: string, cwd: string): ScanOutcome { + const results: VerificationGapItem[] = []; + let acknowledged = 0; + const { targets, activeUnreadable } = listAuditPhaseTargets(planDir, cwd); + if (activeUnreadable) { + results.push({ scan_error: true, phase: '', file: '', status: '' }); } - const results: VerificationGapItem[] = []; - - for (const dir of dirs) { - const phaseDir = path.join(phasesDir, dir); - const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); - const phaseNum = phaseMatch ? phaseMatch[1] : dir; + for (const target of targets) { + const phaseMatch = target.dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); + const phaseNum = phaseMatch ? phaseMatch[1] : target.dir; let files: string[]; try { - files = fs.readdirSync(phaseDir); + files = fs.readdirSync(target.fullPath); } catch { continue; } @@ -607,9 +1032,9 @@ function scanVerificationGaps(planDir: string): VerificationGapItem[] { // ad-hoc VERIFICATION file cannot surface as this phase's gap. for (const file of scopeToPhase( files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md')), - dir, + target.dir, )) { - const filePath = path.join(phaseDir, file); + const filePath = path.join(target.fullPath, file); let safeFilePath: string; try { @@ -626,52 +1051,51 @@ function scanVerificationGaps(planDir: string): VerificationGapItem[] { if (status !== 'gaps_found' && status !== 'human_needed') continue; - results.push({ - phase: sanitizeForDisplay(phaseNum), - file: sanitizeForDisplay(file), + if (isAuditItemAcknowledged(fm, { snapshotKey: 'status', currentValue: status })) { + acknowledged++; + continue; + } + + const item: VerificationGapItem = { + phase: sanitizeLabel(phaseNum), + file: sanitizeLabel(file), status: sanitizeForDisplay(status), - }); + }; + if (target.milestone !== undefined) item.archived_milestone = sanitizeLabel(target.milestone); + results.push(item); } } - return results; + return { items: results, acknowledged }; } // ─── scanContextQuestions ───────────────────────────────────────────────────── /** - * Scan .planning/phases for CONTEXT files with open_questions. + * Scan .planning/phases (active) and .planning/milestones/vX.Y-phases (archived) + * for CONTEXT files with open_questions. */ -function scanContextQuestions(planDir: string): ContextQuestionItem[] { - const phasesDir = path.join(planDir, 'phases'); - if (!fs.existsSync(phasesDir)) return []; - - let dirs: string[]; - try { - dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()) - .map(e => e.name) - .sort(); - } catch { - return [{ scan_error: true, phase: '', file: '', question_count: 0, questions: [] }]; +function scanContextQuestions(planDir: string, cwd: string): ScanOutcome { + const results: ContextQuestionItem[] = []; + let acknowledged = 0; + const { targets, activeUnreadable } = listAuditPhaseTargets(planDir, cwd); + if (activeUnreadable) { + results.push({ scan_error: true, phase: '', file: '', question_count: 0, questions: [] }); } - const results: ContextQuestionItem[] = []; - - for (const dir of dirs) { - const phaseDir = path.join(phasesDir, dir); - const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); - const phaseNum = phaseMatch ? phaseMatch[1] : dir; + for (const target of targets) { + const phaseMatch = target.dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); + const phaseNum = phaseMatch ? phaseMatch[1] : target.dir; let files: string[]; try { - files = fs.readdirSync(phaseDir); + files = fs.readdirSync(target.fullPath); } catch { continue; } for (const file of files.filter(f => f.includes('-CONTEXT') && f.endsWith('.md'))) { - const filePath = path.join(phaseDir, file); + const filePath = path.join(target.fullPath, file); let safeFilePath: string; try { @@ -684,48 +1108,40 @@ function scanContextQuestions(planDir: string): ContextQuestionItem[] { if (content === null) continue; const fm = extractFrontmatter(content, safeFilePath); - - // Check frontmatter open_questions field - let questions: string[] = []; - if (fm.open_questions) { - if (Array.isArray(fm.open_questions) && fm.open_questions.length > 0) { - questions = (fm.open_questions as unknown[]).map(q => sanitizeForDisplay(String(q).slice(0, 200))); - } - } - - // Also check for ## Open Questions section in body - if (questions.length === 0) { - const oqSection = collectSection(content, (h) => h.level === 2 && h.text.trim().toLowerCase().startsWith('open questions'), { levelBounded: true }); - if (oqSection) { - const oqBody = oqSection.body.trim(); - if (oqBody && oqBody.length > 0 && !/^\s*none\s*$/i.test(oqBody)) { - const items = oqBody.split('\n') - .map((l: string) => l.trim()) - .filter((l: string) => l && l !== '-' && l !== '*') - .filter((l: string) => /^[-*\d]/.test(l) || l.includes('?')); - questions = items.slice(0, 3).map((q: string) => sanitizeForDisplay(q.slice(0, 200))); - } - } - } + const questions = deriveOpenQuestions(content, fm); if (questions.length === 0) continue; - results.push({ - phase: sanitizeForDisplay(phaseNum), - file: sanitizeForDisplay(file), + // WARNING 2 (#3458 follow-up review): snapshot the QUESTIONS + // THEMSELVES (a content digest), not just their count — a count-only + // snapshot cannot see the same-count REPLACEMENT of every question + // with brand-new ones (measured: 2 acknowledged, then both swapped for + // unrelated new blockers, still suppressed under the old scheme). + if (isAuditItemAcknowledged(fm, { snapshotKey: 'questions_digest', currentValue: deriveOpenQuestionsDigest(questions) })) { + acknowledged++; + continue; + } + + const item: ContextQuestionItem = { + phase: sanitizeLabel(phaseNum), + file: sanitizeLabel(file), question_count: questions.length, - questions: questions.slice(0, 3), - }); + questions: questions.slice(0, 3).map(truncateQuestionForDisplay), + }; + if (target.milestone !== undefined) item.archived_milestone = sanitizeLabel(target.milestone); + results.push(item); } } - return results; + return { items: results, acknowledged }; } // ─── scanDeferredItems ──────────────────────────────────────────────────────── /** - * Scan phase directories for UNRESOLVED entries in `deferred-items.md` (#2646). + * Scan phase directories for UNRESOLVED entries in `deferred-items.md` (#2646), + * across both .planning/phases (active) and .planning/milestones/vX.Y-phases + * (archived). * * The SCOPE BOUNDARY convention (`agents/gsd-executor.md`) has a phase agent * log an out-of-scope discovery here rather than fix it. #2287 made that file @@ -737,39 +1153,36 @@ function scanContextQuestions(planDir: string): ContextQuestionItem[] { * and the entry leaves the live tree having never been triaged. * * The resolved/unresolved predicate is NOT reimplemented here: `uat.cjs` - * already exports `parseDeferredItems`, which owns the parsing rule (entries - * under a `## Deferred Items` level-2 heading, else the whole file fail-safe; - * RESOLVED only on an explicit case-insensitive `status: resolved` field). - * Duplicating that inequality is how two readers of the same file drift into - * disagreeing about what "open" means. The require is deliberately LAZY, - * inside the scan, to preserve `audit-command-router.cts`'s property that a - * route never loads the module it does not need. + * already exports `parseDeferredItemsWithStatus`, which owns the parsing rule + * (entries under a `## Deferred Items` level-2 heading, else the whole file + * fail-safe) and — unlike `parseDeferredItems` — surfaces each entry's raw + * `status:` field instead of filtering `resolved` internally, so THIS scanner + * can apply the three-way split (#3458 follow-up): `resolved` (fixed for + * real — dropped, never counted, matching pre-existing behavior exactly), + * `acknowledged` (suppressed AND tallied — the new deferred_items marker; + * see the module doc comment above `isAuditItemAcknowledged`), else open. + * Duplicating either inequality is how two readers of the same file drift + * into disagreeing about what "open" means. The require is deliberately + * LAZY, inside the scan, to preserve `audit-command-router.cts`'s property + * that a route never loads the module it does not need. */ -function scanDeferredItems(planDir: string): DeferredItem[] { - const phasesDir = path.join(planDir, 'phases'); - if (!fs.existsSync(phasesDir)) return []; - - let dirs: string[]; - try { - dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()) - .map(e => e.name) - .sort(); - } catch { - return [{ scan_error: true, phase: '', file: '', text: '' }]; - } +function scanDeferredItems(planDir: string, cwd: string): ScanOutcome { + const { targets, activeUnreadable } = listAuditPhaseTargets(planDir, cwd); // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment const uat: UatDeferredModule = require('./uat.cjs'); const results: DeferredItem[] = []; + let acknowledged = 0; + if (activeUnreadable) { + results.push({ scan_error: true, phase: '', file: '', text: '' }); + } - for (const dir of dirs) { - const phaseDir = path.join(phasesDir, dir); - const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); - const phaseNum = phaseMatch ? phaseMatch[1] : dir; + for (const target of targets) { + const phaseMatch = target.dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); + const phaseNum = phaseMatch ? phaseMatch[1] : target.dir; - const filePath = path.join(phaseDir, DEFERRED_ITEMS_FILENAME); + const filePath = path.join(target.fullPath, DEFERRED_ITEMS_FILENAME); if (!fs.existsSync(filePath)) continue; let safeFilePath: string; @@ -782,16 +1195,25 @@ function scanDeferredItems(planDir: string): DeferredItem[] { const content = platformReadSync(safeFilePath); if (content === null) continue; - for (const item of uat.parseDeferredItems(content)) { - results.push({ - phase: sanitizeForDisplay(phaseNum), - file: DEFERRED_ITEMS_FILENAME, + for (const item of uat.parseDeferredItemsWithStatus(content)) { + const rawStatus = (item.status || '').toLowerCase(); + if (rawStatus === 'resolved') continue; // fixed for real — never counted + if (rawStatus === 'acknowledged') { + acknowledged++; + continue; + } + + const resultItem: DeferredItem = { + phase: sanitizeLabel(phaseNum), + file: sanitizeLabel(DEFERRED_ITEMS_FILENAME), text: sanitizeForDisplay(item.name), - }); + }; + if (target.milestone !== undefined) resultItem.archived_milestone = sanitizeLabel(target.milestone); + results.push(resultItem); } } - return results; + return { items: results, acknowledged }; } // ─── auditOpenArtifacts ─────────────────────────────────────────────────────── @@ -806,39 +1228,39 @@ function auditOpenArtifacts(cwd: string): AuditResult { const planDir = planningDir(cwd); const debugSessions = (() => { - try { return scanDebugSessions(planDir); } catch { return [{ scan_error: true, slug: '', status: '', updated: '', hypothesis: '' }]; } + try { return scanDebugSessions(planDir); } catch { return { items: [{ scan_error: true, slug: '', status: '', updated: '', hypothesis: '' }], acknowledged: 0 }; } })(); const quickTasks = (() => { - try { return scanQuickTasks(planDir); } catch { return [{ scan_error: true, slug: '', date: '', status: '', description: '' }]; } + try { return scanQuickTasks(planDir); } catch { return { items: [{ scan_error: true, slug: '', date: '', status: '', description: '' }], acknowledged: 0 }; } })(); const threads = (() => { - try { return scanThreads(planDir); } catch { return [{ scan_error: true, slug: '', status: '', updated: '', title: '' }]; } + try { return scanThreads(planDir); } catch { return { items: [{ scan_error: true, slug: '', status: '', updated: '', title: '' }], acknowledged: 0 }; } })(); const todos = (() => { - try { return scanTodos(planDir); } catch { return [{ scan_error: true, filename: '', priority: '', area: '', summary: '' }]; } + try { return scanTodos(planDir); } catch { return { items: [{ scan_error: true, filename: '', priority: '', area: '', summary: '' }], acknowledged: 0 }; } })(); const seeds = (() => { - try { return scanSeeds(planDir); } catch { return [{ scan_error: true, seed_id: '', slug: '', status: '', title: '' }]; } + try { return scanSeeds(planDir); } catch { return { items: [{ scan_error: true, seed_id: '', slug: '', status: '', title: '' }], acknowledged: 0 }; } })(); const uatGaps = (() => { - try { return scanUatGaps(planDir); } catch { return [{ scan_error: true, phase: '', file: '', status: '', open_scenario_count: 0 }]; } + try { return scanUatGaps(planDir, cwd); } catch { return { items: [{ scan_error: true, phase: '', file: '', status: '', open_scenario_count: 0 }], acknowledged: 0 }; } })(); const verificationGaps = (() => { - try { return scanVerificationGaps(planDir); } catch { return [{ scan_error: true, phase: '', file: '', status: '' }]; } + try { return scanVerificationGaps(planDir, cwd); } catch { return { items: [{ scan_error: true, phase: '', file: '', status: '' }], acknowledged: 0 }; } })(); const contextQuestions = (() => { - try { return scanContextQuestions(planDir); } catch { return [{ scan_error: true, phase: '', file: '', question_count: 0, questions: [] }]; } + try { return scanContextQuestions(planDir, cwd); } catch { return { items: [{ scan_error: true, phase: '', file: '', question_count: 0, questions: [] }], acknowledged: 0 }; } })(); const deferredItems = (() => { - try { return scanDeferredItems(planDir); } catch { return [{ scan_error: true, phase: '', file: '', text: '' }]; } + try { return scanDeferredItems(planDir, cwd); } catch { return { items: [{ scan_error: true, phase: '', file: '', text: '' }], acknowledged: 0 }; } })(); // Count real items (not scan_error sentinels) @@ -846,33 +1268,51 @@ function auditOpenArtifacts(cwd: string): AuditResult { arr.filter(i => !i.scan_error && !i._remainder_count).length; const counts: AuditCounts = { - debug_sessions: countReal(debugSessions), - quick_tasks: countReal(quickTasks), - threads: countReal(threads), - todos: countReal(todos), - seeds: countReal(seeds), - uat_gaps: countReal(uatGaps), - verification_gaps: countReal(verificationGaps), - context_questions: countReal(contextQuestions), - deferred_items: countReal(deferredItems), + debug_sessions: countReal(debugSessions.items), + quick_tasks: countReal(quickTasks.items), + threads: countReal(threads.items), + todos: countReal(todos.items), + seeds: countReal(seeds.items), + uat_gaps: countReal(uatGaps.items), + verification_gaps: countReal(verificationGaps.items), + context_questions: countReal(contextQuestions.items), + deferred_items: countReal(deferredItems.items), total: 0, }; counts.total = counts.debug_sessions + counts.quick_tasks + counts.threads + counts.todos + counts.seeds + counts.uat_gaps + counts.verification_gaps + counts.context_questions + counts.deferred_items; + // #3458 follow-up (A5): mirrors `counts`'s shape exactly, so a reviewer can + // tell "clean because fixed" apart from "clean because silenced" without a + // second output contract to learn. + const acknowledged: AuditCounts = { + debug_sessions: debugSessions.acknowledged, + quick_tasks: quickTasks.acknowledged, + threads: threads.acknowledged, + todos: todos.acknowledged, + seeds: seeds.acknowledged, + uat_gaps: uatGaps.acknowledged, + verification_gaps: verificationGaps.acknowledged, + context_questions: contextQuestions.acknowledged, + deferred_items: deferredItems.acknowledged, + total: 0, + }; + acknowledged.total = acknowledged.debug_sessions + acknowledged.quick_tasks + acknowledged.threads + acknowledged.todos + acknowledged.seeds + acknowledged.uat_gaps + acknowledged.verification_gaps + acknowledged.context_questions + acknowledged.deferred_items; + return { scanned_at: new Date().toISOString(), has_open_items: counts.total > 0, counts, + acknowledged, items: { - debug_sessions: debugSessions, - quick_tasks: quickTasks, - threads, - todos, - seeds, - uat_gaps: uatGaps, - verification_gaps: verificationGaps, - context_questions: contextQuestions, - deferred_items: deferredItems, + debug_sessions: debugSessions.items, + quick_tasks: quickTasks.items, + threads: threads.items, + todos: todos.items, + seeds: seeds.items, + uat_gaps: uatGaps.items, + verification_gaps: verificationGaps.items, + context_questions: contextQuestions.items, + deferred_items: deferredItems.items, }, }; } @@ -886,7 +1326,7 @@ function auditOpenArtifacts(cwd: string): AuditResult { * @returns Formatted report */ function formatAuditReport(auditResult: AuditResult): string { - const { counts, items, has_open_items } = auditResult; + const { counts, items, has_open_items, acknowledged } = auditResult; const lines: string[] = []; const hr = '━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━'; @@ -894,18 +1334,31 @@ function formatAuditReport(auditResult: AuditResult): string { lines.push(' Milestone Close: Open Artifact Audit'); lines.push(hr); + // WARNING 3 (#3458 follow-up review): the acknowledged tally previously + // existed only in `--json` output — the human report could not tell + // "clean because fixed" apart from "clean because silenced", which is the + // exact distinction the acknowledged/counts split exists to preserve. if (!has_open_items) { lines.push(''); - lines.push(' All artifact types clear. Safe to proceed.'); + if (acknowledged.total > 0) { + lines.push(` All artifact types clear (${acknowledged.total} previously acknowledged item${acknowledged.total !== 1 ? 's' : ''} still suppressed).`); + } else { + lines.push(' All artifact types clear. Safe to proceed.'); + } lines.push(''); lines.push(hr); return lines.join('\n'); } + // WARNING 3: per-category "N previously acknowledged" suffix, so the + // human report carries the same "clean vs silenced" signal `--json` + // already did via `acknowledged`. + const ackSuffix = (n: number): string => (n > 0 ? `, ${n} previously acknowledged` : ''); + // Debug sessions (blocking quality — red) if (counts.debug_sessions > 0) { lines.push(''); - lines.push(`🔴 Debug Sessions (${counts.debug_sessions} open)`); + lines.push(`🔴 Debug Sessions (${counts.debug_sessions} open${ackSuffix(acknowledged.debug_sessions)})`); for (const item of items.debug_sessions.filter(i => !i.scan_error)) { const hyp = item.hypothesis ? ` — ${item.hypothesis}` : ''; lines.push(` • ${item.slug} [${item.status}]${hyp}`); @@ -915,25 +1368,27 @@ function formatAuditReport(auditResult: AuditResult): string { // UAT gaps (blocking quality — red) if (counts.uat_gaps > 0) { lines.push(''); - lines.push(`🔴 UAT Gaps (${counts.uat_gaps} phases with incomplete UAT)`); + lines.push(`🔴 UAT Gaps (${counts.uat_gaps} phases with incomplete UAT${ackSuffix(acknowledged.uat_gaps)})`); for (const item of items.uat_gaps.filter(i => !i.scan_error)) { - lines.push(` • Phase ${item.phase}: ${item.file} [${item.status}] — ${item.open_scenario_count} pending scenarios`); + const archived = item.archived_milestone ? ` (archived ${item.archived_milestone})` : ''; + lines.push(` • Phase ${item.phase}${archived}: ${item.file} [${item.status}] — ${item.open_scenario_count} pending scenarios`); } } // Verification gaps (blocking quality — red) if (counts.verification_gaps > 0) { lines.push(''); - lines.push(`🔴 Verification Gaps (${counts.verification_gaps} unresolved)`); + lines.push(`🔴 Verification Gaps (${counts.verification_gaps} unresolved${ackSuffix(acknowledged.verification_gaps)})`); for (const item of items.verification_gaps.filter(i => !i.scan_error)) { - lines.push(` • Phase ${item.phase}: ${item.file} [${item.status}]`); + const archived = item.archived_milestone ? ` (archived ${item.archived_milestone})` : ''; + lines.push(` • Phase ${item.phase}${archived}: ${item.file} [${item.status}]`); } } // Quick tasks (incomplete work — yellow) if (counts.quick_tasks > 0) { lines.push(''); - lines.push(`🟡 Quick Tasks (${counts.quick_tasks} incomplete)`); + lines.push(`🟡 Quick Tasks (${counts.quick_tasks} incomplete${ackSuffix(acknowledged.quick_tasks)})`); for (const item of items.quick_tasks.filter(i => !i.scan_error)) { const d = item.date ? ` (${item.date})` : ''; lines.push(` • ${item.slug}${d} [${item.status}]`); @@ -945,7 +1400,7 @@ function formatAuditReport(auditResult: AuditResult): string { const realTodos = items.todos.filter(i => !i.scan_error && !i._remainder_count); const remainder = items.todos.find(i => i._remainder_count); lines.push(''); - lines.push(`🟡 Pending Todos (${counts.todos} pending)`); + lines.push(`🟡 Pending Todos (${counts.todos} pending${ackSuffix(acknowledged.todos)})`); for (const item of realTodos) { const area = item.area ? ` [${item.area}]` : ''; const pri = item.priority ? ` (${item.priority})` : ''; @@ -960,7 +1415,7 @@ function formatAuditReport(auditResult: AuditResult): string { // Threads (deferred decisions — blue) if (counts.threads > 0) { lines.push(''); - lines.push(`🔵 Open Threads (${counts.threads} active)`); + lines.push(`🔵 Open Threads (${counts.threads} active${ackSuffix(acknowledged.threads)})`); for (const item of items.threads.filter(i => !i.scan_error)) { const title = item.title ? ` — ${item.title}` : ''; lines.push(` • ${item.slug} [${item.status}]${title}`); @@ -970,7 +1425,7 @@ function formatAuditReport(auditResult: AuditResult): string { // Seeds (deferred decisions — blue) if (counts.seeds > 0) { lines.push(''); - lines.push(`🔵 Unimplemented Seeds (${counts.seeds} pending)`); + lines.push(`🔵 Unimplemented Seeds (${counts.seeds} pending${ackSuffix(acknowledged.seeds)})`); for (const item of items.seeds.filter(i => !i.scan_error)) { const title = item.title ? ` — ${item.title}` : ''; lines.push(` • ${item.seed_id} [${item.status}]${title}`); @@ -980,9 +1435,10 @@ function formatAuditReport(auditResult: AuditResult): string { // Context questions (deferred decisions — blue) if (counts.context_questions > 0) { lines.push(''); - lines.push(`🔵 CONTEXT Open Questions (${counts.context_questions} phases with open questions)`); + lines.push(`🔵 CONTEXT Open Questions (${counts.context_questions} phases with open questions${ackSuffix(acknowledged.context_questions)})`); for (const item of items.context_questions.filter(i => !i.scan_error)) { - lines.push(` • Phase ${item.phase}: ${item.file} (${item.question_count} question${item.question_count !== 1 ? 's' : ''})`); + const archived = item.archived_milestone ? ` (archived ${item.archived_milestone})` : ''; + lines.push(` • Phase ${item.phase}${archived}: ${item.file} (${item.question_count} question${item.question_count !== 1 ? 's' : ''})`); for (const q of item.questions) { lines.push(` - ${q}`); } @@ -993,18 +1449,237 @@ function formatAuditReport(auditResult: AuditResult): string { // phase agent recorded rather than fixed, still unresolved at close (#2646). if (counts.deferred_items > 0) { lines.push(''); - lines.push(`🔵 Deferred Items (${counts.deferred_items} unresolved)`); + lines.push(`🔵 Deferred Items (${counts.deferred_items} unresolved${ackSuffix(acknowledged.deferred_items)})`); for (const item of items.deferred_items.filter(i => !i.scan_error)) { - lines.push(` • Phase ${item.phase}: ${item.text}`); + const archived = item.archived_milestone ? ` (archived ${item.archived_milestone})` : ''; + lines.push(` • Phase ${item.phase}${archived}: ${item.text}`); } } lines.push(''); lines.push(hr); lines.push(` ${counts.total} item${counts.total !== 1 ? 's' : ''} require decisions before close.`); + if (acknowledged.total > 0) { + lines.push(` ${acknowledged.total} previously acknowledged item${acknowledged.total !== 1 ? 's' : ''} also suppressed above the ${counts.total} open item${counts.total !== 1 ? 's' : ''}.`); + } lines.push(hr); return lines.join('\n'); } -export = { auditOpenArtifacts, formatAuditReport }; +// ─── resolvePhaseTargetDir ───────────────────────────────────────────────────── + +/** + * Resolve ONE phase directory (active or archived) by its phase token, for + * `cmdAuditAcknowledge`'s `--phase [--archived-milestone]` identification of + * a uat_gaps/verification_gaps/context_questions/deferred_items item. Built + * on `listAuditPhaseTargets` — the same enumeration the four phase-scoped + * scanners use — so the writer can never resolve a DIFFERENT directory than + * the one the audit actually scanned. + * + * `archivedMilestone` absent → matches the ACTIVE `.planning/phases/` + * (a target with no `milestone`). Present → matches the archived target + * whose `milestone` equals it exactly — the same disambiguator the audit + * output's `archived_milestone` field carries. + */ +function resolvePhaseTargetDir(planDir: string, cwd: string, phase: string, archivedMilestone: string | null): string | null { + const { targets } = listAuditPhaseTargets(planDir, cwd); + const phaseTokenRe = new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i'); + for (const target of targets) { + const phaseMatch = target.dir.match(phaseTokenRe); + const phaseNum = phaseMatch ? phaseMatch[1] : target.dir; + if (phaseNum !== phase) continue; + if (archivedMilestone) { + if (target.milestone === archivedMilestone) return target.fullPath; + } else if (target.milestone === undefined) { + return target.fullPath; + } + } + return null; +} + +// ─── cmdAuditAcknowledge ──────────────────────────────────────────────────────── + +/** + * CLI writer for the #3458 follow-up suppression seam (design point A4). Sets + * (or refreshes) the `audit_acknowledged` marker on ONE identified artifact, + * snapshotting its CURRENT effective state itself so the marker is never + * hand-authored and can never drift from what the scanners actually compute. + * + * `--category` selects which of the nine audit categories is being + * acknowledged, and which OTHER flags are required to identify the artifact — + * mirroring the fields the audit's OWN JSON output already carries per + * category (phase/file/archived_milestone for the four phase-scoped + * categories; slug/seed-id/dir/filename for the five flat ones), the same + * convention `frontmatter get/set/merge/validate` uses for `--file`/`--field`. + * + * VERDICT-PRESERVING: this function never writes to the artifact's own + * `status:` field (the audit's real verdict) for the 8 frontmatter-marker + * categories — only the sibling `audit_acknowledged` map. `deferred_items` is + * the sole, deliberate exception (see `uat.cts`'s `acknowledgeDeferredItem`): + * there, the marker IS the entry's own `status:` field, because a + * deferred-items.md entry carries no OTHER meaning for that field. + * + * Every path this function writes is routed through `requireSafePath`, so an + * artifact identifier that resolves outside the project is refused before + * any read or write is attempted. + */ +function cmdAuditAcknowledge(cwd: string, args: string[], raw: boolean): void { + const { + category, milestone, at: atFlag, + phase, file, 'archived-milestone': archivedMilestone, + slug, 'seed-id': seedId, dir: quickDir, filename, text, + } = parseNamedArgs(args, [ + 'category', 'milestone', 'at', + 'phase', 'file', 'archived-milestone', + 'slug', 'seed-id', 'dir', 'filename', 'text', + ]) as Record; + + if (!category) ioError('--category is required'); + if (!milestone) ioError('--milestone is required'); + const at = atFlag || new Date().toISOString().slice(0, 10); + + const planDir = planningDir(cwd); + const markerBase = { milestone: milestone as string, at }; + + // ── The four phase-scoped categories: --phase --file [--archived-milestone] ── + const PHASE_SCOPED = new Set(['uat_gaps', 'verification_gaps', 'context_questions', 'deferred_items']); + if (PHASE_SCOPED.has(category as string)) { + if (!phase) ioError('--phase is required for this --category'); + if (!file) ioError('--file is required for this --category'); + const targetDir = resolvePhaseTargetDir(planDir, cwd, phase as string, archivedMilestone); + if (!targetDir) { + ioError(`no phase directory found for phase "${phase as string}"${archivedMilestone ? ` (archived-milestone "${archivedMilestone}")` : ''}`); + } + const filePath = path.join(targetDir as string, file as string); + const safeFilePath = requireSafePath(filePath, planDir, 'audit acknowledge target', { allowAbsolute: true }); + if (!fs.existsSync(safeFilePath)) ioError(`file not found: ${file as string}`); + + if (category === 'deferred_items') { + if (!text) ioError('--text is required for --category deferred_items'); + // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment + const uat: UatDeferredModule = require('./uat.cjs'); + const content = fs.readFileSync(safeFilePath, 'utf-8'); + const result = uat.acknowledgeDeferredItem(content, text as string); + if (result.status === 'not_found') ioError(`no deferred item matched --text "${text as string}"`); + if (result.status === 'ambiguous') ioError(`--text "${text as string}" matches more than one deferred item — text must be unique`); + if (result.status === 'already_resolved') ioError(`deferred item is already "status: resolved" — acknowledging a resolved item is a no-op`); + if (result.status === 'unsupported_heading_shape') { + ioError('this deferred-items.md uses the heading-delimited (#3457) entry shape, which the CLI writer does not yet support — edit the file directly'); + } + if (result.status === 'match_verification_failed') { + ioError(`internal error: matched span for --text "${text as string}" did not re-verify before write — refused rather than risk writing the wrong entry`); + } + platformWriteSync(safeFilePath, result.content); + output({ acknowledged: true, category, phase, file, text }, raw, 'true'); + return; + } + + const content = fs.readFileSync(safeFilePath, 'utf-8'); + const fm = extractFrontmatter(content, safeFilePath); + let snapshotKey: string; + let currentValue: string; + if (category === 'uat_gaps') { + // WARNING 2 (#3458 follow-up review): status alone can't see MORE + // pending scenarios added under the same status — snapshot the + // composite `deriveUatGapSnapshotValue` instead (see its doc comment). + snapshotKey = 'gap_snapshot'; + currentValue = deriveUatGapSnapshotValue(((fm.status as string) || 'unknown').toLowerCase(), content); + } else if (category === 'verification_gaps') { + snapshotKey = 'status'; + currentValue = ((fm.status as string) || 'unknown').toLowerCase(); + } else { + // context_questions — WARNING 2: snapshot a content digest of the + // question set, not just its count (see `deriveOpenQuestionsDigest`'s + // doc comment). + snapshotKey = 'questions_digest'; + currentValue = deriveOpenQuestionsDigest(deriveOpenQuestions(content, fm)); + } + fm.audit_acknowledged = { ...markerBase, [snapshotKey]: currentValue }; + const newContent = spliceFrontmatter(content, fm); + platformWriteSync(safeFilePath, newContent); + output({ acknowledged: true, category, phase, file, [snapshotKey]: currentValue }, raw, 'true'); + return; + } + + // ── The five flat categories: category-specific identifier flag ── + // `status` for all five per the architecture's per-category table (`todos` + // is presence-only and never reads `snapshotKey`, so it stays a constant). + const snapshotKey = 'status'; + let safeFilePath: string; + let currentValue: string; + let createIfMissing = false; + // Same value shape `Frontmatter`/`extractFrontmatter` use (frontmatter.cts + // does not export the `Frontmatter` type name itself, so it is spelled out + // structurally here) — keeps this and `extractFrontmatter`'s return type + // unifying to the SAME type below instead of a lossy `Record` that `spliceFrontmatter`'s `Frontmatter` parameter would reject. + let fmForCreate: Record> = {}; + + if (category === 'debug_sessions') { + if (!slug) ioError('--slug is required for --category debug_sessions'); + safeFilePath = requireSafePath(path.join(planDir, 'debug', `${slug as string}.md`), planDir, 'audit acknowledge target', { allowAbsolute: true }); + if (!fs.existsSync(safeFilePath)) ioError(`file not found: debug/${slug as string}.md`); + const content = fs.readFileSync(safeFilePath, 'utf-8'); + currentValue = ((extractFrontmatter(content, safeFilePath).status as string) || 'unknown').toLowerCase(); + } else if (category === 'threads') { + if (!slug) ioError('--slug is required for --category threads'); + safeFilePath = requireSafePath(path.join(planDir, 'threads', `${slug as string}.md`), planDir, 'audit acknowledge target', { allowAbsolute: true }); + if (!fs.existsSync(safeFilePath)) ioError(`file not found: threads/${slug as string}.md`); + const content = fs.readFileSync(safeFilePath, 'utf-8'); + currentValue = deriveThreadStatus(extractFrontmatter(content, safeFilePath), content); + } else if (category === 'seeds') { + if (!seedId) ioError('--seed-id is required for --category seeds'); + safeFilePath = requireSafePath(path.join(planDir, 'seeds', `${seedId as string}.md`), planDir, 'audit acknowledge target', { allowAbsolute: true }); + if (!fs.existsSync(safeFilePath)) ioError(`file not found: seeds/${seedId as string}.md`); + const content = fs.readFileSync(safeFilePath, 'utf-8'); + currentValue = ((extractFrontmatter(content, safeFilePath).status as string) || 'dormant').toLowerCase(); + } else if (category === 'todos') { + if (!filename) ioError('--filename is required for --category todos'); + safeFilePath = requireSafePath(path.join(planDir, 'todos', 'pending', filename as string), planDir, 'audit acknowledge target', { allowAbsolute: true }); + if (!fs.existsSync(safeFilePath)) ioError(`file not found: todos/pending/${filename as string}`); + currentValue = ''; // presence-only — see scanTodos + } else if (category === 'quick_tasks') { + if (!quickDir) ioError('--dir is required for --category quick_tasks'); + const taskDir = requireSafePath(path.join(planDir, 'quick', quickDir as string), planDir, 'audit acknowledge target dir', { allowAbsolute: true }); + if (!fs.existsSync(taskDir)) ioError(`directory not found: quick/${quickDir as string}`); + // Shared with scanQuickTasks (#3458 follow-up) so the reader and this + // writer can never disagree about which file is the task's record. + const resolvedSummaryPath = resolveQuickTaskSummaryFile(taskDir, quickDir as string); + if (resolvedSummaryPath) { + safeFilePath = requireSafePath(resolvedSummaryPath, planDir, 'audit acknowledge target', { allowAbsolute: true }); + const content = fs.readFileSync(safeFilePath, 'utf-8'); + currentValue = ((extractFrontmatter(content, safeFilePath).status as string) || 'unknown').toLowerCase(); + } else { + // No SUMMARY.md at all — the audit's own observed status is 'missing'. + // There is nowhere to carry the marker, so create the canonical + // `${dir}-SUMMARY.md` with ONLY `status: missing` + the marker — the + // acknowledgment's own snapshot of "no summary exists yet", which + // self-invalidates the moment a real SUMMARY.md is written (the + // scanner then reads THAT file's own status instead). + safeFilePath = requireSafePath(path.join(taskDir, `${quickDir as string}-SUMMARY.md`), planDir, 'audit acknowledge target', { allowAbsolute: true }); + currentValue = 'missing'; + createIfMissing = true; + fmForCreate = { status: 'missing' }; + } + } else { + ioError(`unknown --category "${category as string}". Available: debug_sessions, quick_tasks, threads, todos, seeds, uat_gaps, verification_gaps, context_questions, deferred_items`); + return; // unreachable — ioError throws — satisfies TS control-flow analysis + } + + const presenceOnly = category === 'todos'; + const fm = createIfMissing ? fmForCreate : extractFrontmatter(fs.readFileSync(safeFilePath, 'utf-8'), safeFilePath); + fm.audit_acknowledged = presenceOnly ? { ...markerBase } : { ...markerBase, [snapshotKey]: currentValue }; + const newContent = createIfMissing + ? spliceFrontmatter('', fm) + : spliceFrontmatter(fs.readFileSync(safeFilePath, 'utf-8'), fm); + platformWriteSync(safeFilePath, newContent); + output({ acknowledged: true, category, ...(presenceOnly ? {} : { [snapshotKey]: currentValue }) }, raw, 'true'); +} + +export = { + auditOpenArtifacts, + formatAuditReport, + listAuditPhaseTargets, + cmdAuditAcknowledge, +}; diff --git a/src/phase-locator.cts b/src/phase-locator.cts index b2ff2fb97..126eb2b81 100644 --- a/src/phase-locator.cts +++ b/src/phase-locator.cts @@ -121,9 +121,27 @@ interface ArchiveVersionDir { * exactly the shape that let the original #2855 bug (hardcoded root path) * exist in one copy and not the other. Sharing this seam means a future * change to how the archive tree is located only needs to happen once. - * Most-recent-milestone-first order (reverse-sorted directory names). + * Most-recent-milestone-first order, compared numerically segment-by-segment + * on the version (e.g. `v1.10` before `v1.9`) — NOT lexicographically. A + * lexicographic `.sort().reverse()` (the prior implementation) ranks `v1.9` + * ahead of `v1.10` because the string `"1.9"` sorts after `"1.10"`; that is + * deterministic but wrong for every double-digit-or-higher minor/patch + * version, and #3458 is what first surfaces archived phases in audit output + * where the misordering becomes user-visible. * Never throws: an absent/unreadable milestones/ dir yields []. */ +function compareArchiveVersionDesc(aName: string, bName: string): number { + const aParts = (aName.match(/^v([\d.]+)-phases$/)?.[1] ?? '').split('.').map(Number); + const bParts = (bName.match(/^v([\d.]+)-phases$/)?.[1] ?? '').split('.').map(Number); + const len = Math.max(aParts.length, bParts.length); + for (let i = 0; i < len; i++) { + const a = aParts[i] ?? 0; + const b = bParts[i] ?? 0; + if (a !== b) return b - a; // descending: newest (numerically largest) first + } + return 0; +} + function listArchiveVersionDirs(cwd: string): ArchiveVersionDir[] { const milestonesDir = path.join(planningDir(cwd), 'milestones'); if (!fs.existsSync(milestonesDir)) return []; @@ -133,8 +151,7 @@ function listArchiveVersionDirs(cwd: string): ArchiveVersionDir[] { return milestoneEntries .filter(e => e.isDirectory() && /^v[\d.]+-phases$/.test(e.name)) .map(e => e.name) - .sort() - .reverse() + .sort(compareArchiveVersionDesc) .map(archiveName => ({ version: archiveName.match(/^(v[\d.]+)-phases$/)![1], archivePath: path.join(milestonesDir, archiveName), diff --git a/src/security.cts b/src/security.cts index 9f609e39c..90a1f8fe0 100644 --- a/src/security.cts +++ b/src/security.cts @@ -413,6 +413,66 @@ export function sanitizeForDisplay(text: unknown): string { return sanitized; } +/** + * Sanitize a value that must render as a SINGLE LINE and is derived from a + * filesystem name (a phase directory's number/name token, an archived + * milestone label, a bare filename) — not from file/frontmatter CONTENT. + * + * Why this is NOT `sanitizeForDisplay`: that helper's job is multi-line + * prose — it strips whole protocol-leak LINES while deliberately preserving + * `\n` between legitimate ones (see its docstring and + * `tests/security.test.cjs`'s neighbouring describe). A filesystem name is + * the opposite shape: it is supposed to be one line, so a `\n`/`\r` inside + * one is never legitimate content to preserve — it is an attacker (or a + * doctored checkout) using the directory NAME itself as the injection + * vector. #3458's reproduction: a phase directory literally named + * `zz\n0 open items require decisions.\n\x1b[2K\x1b[1G FORGED` + * flows verbatim into `audit-open`'s human report (the phase-number + * fallback taken when the name doesn't match `PHASE_NUMBER_TOKEN_SOURCE`). + * `sanitizeForDisplay` would pass every one of those bytes straight through + * — by design, since it never touches control characters — so the embedded + * `\n` becomes a real newline in the report, printing a forged + * "0 open items require decisions." as its own line, and the raw ESC bytes + * reach the terminal. + * + * This helper closes that hole by ESCAPING (never silently stripping) the + * C0 control range (0x00–0x1F, including ESC 0x1B, CR, LF), DEL (0x7F), and + * the C1 range (0x80–0x9F) into a visible representation (`\n`, `\x1b`, + * ...). Escaping rather than stripping is deliberate: a reviewer reading the + * report should be able to SEE that a name was doctored, not have it quietly + * normalized away as if nothing happened. Every other character — including + * all ordinary printable and non-ASCII text — passes through byte-identical. + */ +export function sanitizeLabel(text: unknown): string { + if (!text || typeof text !== 'string') return text as string; + + const NAMED_ESCAPES: Record = { + 0x00: '\\0', + 0x07: '\\a', + 0x08: '\\b', + 0x09: '\\t', + 0x0a: '\\n', + 0x0b: '\\v', + 0x0c: '\\f', + 0x0d: '\\r', + 0x1b: '\\x1b', + }; + + let out = ''; + for (const ch of text) { + const code = ch.codePointAt(0) as number; + const isC0 = code <= 0x1f; + const isDel = code === 0x7f; + const isC1 = code >= 0x80 && code <= 0x9f; + if (isC0 || isDel || isC1) { + out += NAMED_ESCAPES[code] ?? `\\x${code.toString(16).padStart(2, '0')}`; + } else { + out += ch; + } + } + return out; +} + // ─── Shell Safety ─────────────────────────────────────────────────────────────────────── /** diff --git a/src/uat.cts b/src/uat.cts index dc3430a4f..6d1657cb7 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -903,7 +903,18 @@ function parseGapsTableItems(sectionBody: string): UatItem[] { * one item PER BULLET. A body with no headings keeps the original * one-bullet-per-item split unchanged. */ -function parseDeferredItems(content: string): UatItem[] { +/** + * One `deferred-items.md` entry with its RAW (un-lowercased) `status:` field + * value (`''` when the entry carries no parseable status). #3458 follow-up: + * `parseDeferredItems` (below) is now DEFINED IN TERMS OF this — it filters + * to `status !== 'resolved'` — and `audit.cts`'s `scanDeferredItems` also + * consumes this directly so it can tell `resolved` (fixed for real, never + * counted), the newer `acknowledged` (suppressed-but-tallied, #3458 + * follow-up), and everything else (open) apart WITHOUT a second, + * independent entry-boundary/field-extraction pass that could drift from + * this one. + */ +function parseDeferredItemsWithStatus(content: string): Array<{ name: string; status: string }> { const deferredSection = collectSection( content, (h) => /^deferred\s+items$/i.test(h.text) && h.level === 2, @@ -911,7 +922,7 @@ function parseDeferredItems(content: string): UatItem[] { ); const sectionBody = deferredSection ? deferredSection.body : content; - const items: UatItem[] = []; + const items: Array<{ name: string; status: string }> = []; // #3457: heading-delimited shape — an entry's fields live in sibling bullets // (`- **Status:** resolved`), so the bullet marker is stripped on EVERY line @@ -930,27 +941,192 @@ function parseDeferredItems(content: string): UatItem[] { })); for (const { lines: entryLines, fields } of entries) { - const rawStatus = fields.status; - if (rawStatus && rawStatus.toLowerCase() === 'resolved') continue; - const text = rawGapEntryText(entryLines); if (!text) continue; - items.push({ - name: text, - result: 'unresolved', - category: 'deferred', - }); + items.push({ name: text, status: fields.status || '' }); } // #2766: union with the table form — see parseDeferredTableItems. Executors // write this file by hand with no mandated shape, and a GFM table is a natural // choice for the common "test → failing seeds" case, which produced ZERO items. - items.push(...parseDeferredTableItems(sectionBody)); + // Table rows carry no independently-parseable status column in general — + // `parseDeferredTableItems` already excludes resolved/done/pass rows at its + // own layer (any cell reading exactly one of those three) — so anything it + // returns here is inherently open; `acknowledge` (#3458 follow-up) has no + // representable field to write for a table row, so those are reported with + // status `''` (never `resolved`/`acknowledged`) and remain permanently + // un-acknowledgeable via the CLI writer — a known, deliberate limitation + // (see `acknowledgeDeferredItem`'s doc comment). + items.push(...parseDeferredTableItems(sectionBody).map((item) => ({ name: item.name, status: '' }))); return items; } +function parseDeferredItems(content: string): UatItem[] { + return parseDeferredItemsWithStatus(content) + .filter((entry) => !(entry.status && entry.status.toLowerCase() === 'resolved')) + .map((entry) => ({ + name: entry.name, + result: 'unresolved', + category: 'deferred', + })); +} + +// ─── acknowledgeDeferredItem ─────────────────────────────────────────────────── + +/** Result of `acknowledgeDeferredItem`. */ +interface AcknowledgeDeferredItemResult { + content: string; + status: 'ok' | 'not_found' | 'ambiguous' | 'unsupported_heading_shape' | 'already_resolved' | 'match_verification_failed'; +} + +/** + * CLI-writer half of the #3458 follow-up deferred_items suppression seam. + * Sets the ONE deferred entry whose rendered text (`rawGapEntryText`, the + * same value `parseDeferredItemsWithStatus`/the audit's JSON output surface + * as `name`/`text`) exactly equals `targetText` to `status: acknowledged` — + * a NEW terminal value, distinct from the existing `resolved` (which keeps + * meaning "actually fixed"). This is the marker for this category: unlike + * every other audit category (a sibling `audit_acknowledged` frontmatter map + * that never touches the artifact's own `status:`), a deferred-items.md + * entry's `status:` field carries no OTHER meaning, so the field itself + * doubles as the marker — self-invalidating for free: edit the entry's + * `status:` away from `acknowledged` (or delete the field) and it resurfaces + * with no separate cleanup step, exactly like every other category's marker. + * + * Deliberately refuses (`unsupported_heading_shape`) rather than guess when + * the section uses the heading-delimited (#3457) entry shape: reliably + * mapping a `splitDeferredHeadingEntries` entry back to its EXACT source line + * span is not safely derivable without re-deriving that function's + * leaf/container walk against a document that may also mix in headless + * (`splitGapsEntries`-derived) entries between headings — attempting it risks + * writing into the WRONG entry. The bullet-only (headless) shape below is the + * primary, documented SCOPE BOUNDARY convention and is handled precisely. + * + * Also refuses `ambiguous` (2+ entries share the exact same text — status must + * be unique to identify one) and `not_found`, and is a no-op + * (`already_resolved`) on an entry already carrying `status: resolved` — the + * verdict-preserving direction: acknowledging a genuinely-fixed item would + * silently downgrade its terminal state. + * + * SPAN-CARRIED, not re-searched (F1, #3458 follow-up review — see + * `splitGapsEntriesWithSpans`'s doc comment): the target entry's location + * within `sectionBody` is the (start, end) character span recorded by + * `splitGapsEntriesWithSpans` in the SAME pass that produced `entryLines` / + * `targetText` above — never re-derived afterwards by searching. The + * previous implementation re-found the entry with a regex anchored on its + * own (escaped) exact text; that regex necessarily matches the FIRST + * occurrence of that text within `sectionBody`, which is not always the + * entry that was actually selected (a continuation/quoted line inside an + * EARLIER or LATER entry can carry byte-identical text) — and because the + * mis-targeted span is byte-identical to `targetText`, no downstream check + * on the WRITTEN text could ever distinguish a wrong-entry write from a + * correct one. Carrying the span removes the re-derivation step entirely: + * there is no second search to mis-target. + * + * Section-anchored (BLOCKER 1, #3458 follow-up review): the span is + * `sectionBody`-relative — the SAME string `matches`/the `ambiguous` guard + * were computed over — not `content`-relative, so an identical bullet living + * outside `## Deferred Items` (e.g. in an unrelated `# Notes` or a + * UAT/VERIFICATION body) can never steal the write. The span is translated + * into `content`-relative offsets via `deferredSection.bodyStart` (the + * section's own start offset, an invariant `collectSection` guarantees: + * `content.slice(bodyStart, bodyEnd) === body`). Before writing, the + * spanned text's own raw entry is re-derived and compared against + * `targetText` one more time — this is now a GENUINE invariant check (the + * span was computed by `splitGapsEntriesCore`'s independent offset + * bookkeeping, a different code path than the `entryLines`/`targetText` + * comparison above), not a no-op — if it does not match, the write is + * refused with `match_verification_failed` rather than risk touching the + * wrong span. + */ +function acknowledgeDeferredItem(content: string, targetText: string): AcknowledgeDeferredItemResult { + const deferredSection = collectSection( + content, + (h) => /^deferred\s+items$/i.test(h.text) && h.level === 2, + { levelBounded: true }, + ); + const sectionBody = deferredSection ? deferredSection.body : content; + + if (splitDeferredHeadingEntries(sectionBody) !== null) { + return { content, status: 'unsupported_heading_shape' }; + } + + const entries = splitGapsEntriesWithSpans(sectionBody); + const matches = entries + .map((entry) => ({ entry, text: rawGapEntryText(entry.lines) })) + .filter((e) => e.text === targetText); + + if (matches.length === 0) return { content, status: 'not_found' }; + if (matches.length > 1) return { content, status: 'ambiguous' }; + + const { entry } = matches[0]; + const { lines: entryLines, start, end } = entry; + const fields = extractGapEntryFields(entryLines); + if (fields.status && fields.status.toLowerCase() === 'resolved') { + return { content, status: 'already_resolved' }; + } + + // Anchor to the SAME section body `matches`/the `ambiguous` guard above + // were computed over (BLOCKER 1) — never the whole `content`, which could + // contain an identical bullet elsewhere. `start`/`end` are the entry's own + // span, carried directly from `splitGapsEntriesWithSpans` — no re-search. + const sectionOffset = deferredSection ? deferredSection.bodyStart : 0; + const matchedLines = sectionBody.slice(start, end).split('\n'); + + // Genuine invariant re-verification (see doc comment above): the span was + // computed by a code path independent of the `entryLines`/`targetText` + // comparison that selected this entry — this catches real drift between + // the two rather than a regex trivially guaranteed to agree with itself. + const strippedForVerify = matchedLines.map((l) => l.replace(/\r$/, '')); + if (rawGapEntryText(strippedForVerify) !== targetText) { + return { content, status: 'match_verification_failed' }; + } + + const matchIndexInContent = sectionOffset + start; + const statusFieldRe = /^\s*(?:-\s+)?(\*+status:\*+|status:)/i; + const statusLineIdx = matchedLines.findIndex((rawLine) => statusFieldRe.test(rawLine.replace(/\r$/, ''))); + + // No CRLF-preservation branch here (WARNING 1, #3458 follow-up review): + // every write goes through `platformWriteSync` → `normalizeContent`, which + // for a `.md` path unconditionally runs `_normalizeMd` — whole-file + // `\r\n` → `\n`, plus blank-line normalization around headings/lists — on + // EVERY write, not just this one. That is this codebase's single, + // deliberate OS-facing I/O seam (`shell-command-projection.cts`), applied + // uniformly to every `.md` writer; carving out one exception here would + // fight it rather than follow it, for a guarantee (byte-identical CRLF on + // disk) the seam already makes impossible. A marker write on a CRLF + // `deferred-items.md` normalizes the WHOLE file to LF, same as any other + // `.md` write in this codebase — expected, not a regression to guard + // against. Where a source line still carries a trailing `\r` (read from an + // on-disk CRLF document before normalization), `String.prototype.replace` + // consumes it as part of `.*$` and the replacement text does not + // reproduce it, so it is dropped here too — consistent with the eventual + // whole-file normalization rather than duplicating it. + let newMatchedLines: string[]; + if (statusLineIdx === -1) { + const bulletIndentMatch = matchedLines[0].match(/^(\s*)-\s+/); + const continuationIndent = ' '.repeat((bulletIndentMatch ? bulletIndentMatch[1].length : 0) + 2); + newMatchedLines = [ + matchedLines[0], + `${continuationIndent}status: acknowledged`, + ...matchedLines.slice(1), + ]; + } else { + const original = matchedLines[statusLineIdx]; + const replaced = original.replace( + /^(\s*(?:-\s+)?)(\*+status:\*+|status:)(\s*).*$/i, + (_m, indent: string, key: string, ws: string) => `${indent}${key}${ws}acknowledged`, + ); + newMatchedLines = matchedLines.slice(); + newMatchedLines[statusLineIdx] = replaced; + } + + const newContent = content.slice(0, matchIndexInContent) + newMatchedLines.join('\n') + content.slice(matchIndexInContent + (end - start)); + return { content: newContent, status: 'ok' }; +} + /** * Strip one leading `- ` bullet marker (#3457). Heading-delimited deferred * entries carry their fields as sibling bullets; `extractGapEntryFields` only @@ -1096,6 +1272,79 @@ function parseDeferredTableItems(sectionBody: string): UatItem[] { return items; } +/** + * One `splitGapsEntries` entry together with the exact character SPAN it + * occupies within the `sectionBody` it was derived from — + * `sectionBody.slice(start, end)` is the entry's own original text, + * byte-for-byte (CRLF preserved, unlike `lines`, which strips a trailing + * `\r` off every line). See `splitGapsEntriesWithSpans`'s doc comment for why + * a caller would want this over the plain `lines` shape. + */ +interface GapsEntrySpan { + lines: string[]; + start: number; + end: number; +} + +/** + * Shared walk behind `splitGapsEntries` and `splitGapsEntriesWithSpans` — ONE + * pass over `sectionBody` that both groups its lines into entries (see + * `splitGapsEntries`'s doc comment for the grouping rule) AND records each + * entry's (start, end) character offset within `sectionBody`. Extracted so + * the two public shapes can never drift apart on what counts as an entry + * boundary — a second, independently-written grouping pass is exactly how a + * span-carrying sibling could disagree with the plain-lines version it is + * supposed to be span-annotating. + */ +function splitGapsEntriesCore(sectionBody: string): GapsEntrySpan[] { + const rawLines = sectionBody.split('\n'); + const lineStarts: number[] = []; + const lineEnds: number[] = []; + let cursor = 0; + for (const rawLine of rawLines) { + lineStarts.push(cursor); + cursor += rawLine.length; + lineEnds.push(cursor); + cursor += 1; // the '\n' separator — absent after the final line, but nothing reads past it + } + + const entries: GapsEntrySpan[] = []; + let current: string[] | null = null; + let currentStartLine = -1; + let currentEndLine = -1; + let baseIndent: number | null = null; + + const flush = (): void => { + if (current !== null) { + entries.push({ lines: current, start: lineStarts[currentStartLine], end: lineEnds[currentEndLine] }); + } + }; + + rawLines.forEach((rawLine, idx) => { + const line = rawLine.replace(/\r$/, ''); + const bulletMatch = line.match(/^(\s*)-\s/); + if (bulletMatch) { + const indent = bulletMatch[1].length; + if (baseIndent === null) baseIndent = indent; + if (indent <= baseIndent) { + flush(); + current = [line]; + currentStartLine = idx; + currentEndLine = idx; + return; + } + } + if (current !== null) { + current.push(line); + currentEndLine = idx; + } + // else: pre-first-bullet content (e.g. the template's HTML comment) — discarded. + }); + flush(); + + return entries; +} + /** * Split a `## Gaps` section body into per-entry line groups on TOP-LEVEL * `- ` bullet openers. @@ -1114,29 +1363,27 @@ function parseDeferredTableItems(sectionBody: string): UatItem[] { * (heading present, no bullets) returns `[]`. */ function splitGapsEntries(sectionBody: string): string[][] { - const lines = sectionBody.split('\n'); - const entries: string[][] = []; - let current: string[] | null = null; - let baseIndent: number | null = null; + return splitGapsEntriesCore(sectionBody).map((entry) => entry.lines); +} - for (const rawLine of lines) { - const line = rawLine.replace(/\r$/, ''); - const bulletMatch = line.match(/^(\s*)-\s/); - if (bulletMatch) { - const indent = bulletMatch[1].length; - if (baseIndent === null) baseIndent = indent; - if (indent <= baseIndent) { - if (current) entries.push(current); - current = [line]; - continue; - } - } - if (current) current.push(line); - // else: pre-first-bullet content (e.g. the template's HTML comment) — discarded. - } - if (current) entries.push(current); - - return entries; +/** + * Sibling of `splitGapsEntries` (F1, #3458 follow-up review) that ADDITIVELY + * carries each entry's character span — every existing `splitGapsEntries` + * caller (`parseGapsItems`, `parseDeferredItemsWithStatus`, + * `splitDeferredHeadingEntries`'s `flushPending`) is unaffected and keeps + * using the plain `lines`-only shape. `acknowledgeDeferredItem` is the one + * caller that needs a span: it used to select an entry via `splitGapsEntries` + * and then RE-FIND that entry's location with a fresh regex search over + * `sectionBody` — matching the FIRST occurrence of the entry's exact text, + * not necessarily the entry actually selected (a continuation/quoted line + * inside a DIFFERENT entry can carry byte-identical text). Because the + * mis-targeted span is byte-identical to the target text, no check on the + * WRITTEN result could ever tell a wrong-entry write apart from a correct + * one. Carrying the span out of THIS same pass — the one that already knows + * exactly where the entry lives — removes the re-derivation step entirely. + */ +function splitGapsEntriesWithSpans(sectionBody: string): GapsEntrySpan[] { + return splitGapsEntriesCore(sectionBody); } /** @@ -1416,4 +1663,6 @@ export = { resolveCheckpointFrame, checkpointBoxLine, parseDeferredItems, + parseDeferredItemsWithStatus, + acknowledgeDeferredItem, }; diff --git a/tests/audit-command-cutover.test.cjs b/tests/audit-command-cutover.test.cjs index a7e330709..92c6cc37c 100644 --- a/tests/audit-command-cutover.test.cjs +++ b/tests/audit-command-cutover.test.cjs @@ -498,9 +498,128 @@ describe('audit-open — does not crash with ReferenceError (#2659)', () => { const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +// ─── #3458 fixtures: phase-scoped scanners must also see archived phases ───── +// +// scanUatGaps, scanVerificationGaps, scanContextQuestions, and scanDeferredItems +// resolve ONLY the active `.planning/phases/` root. Once a milestone closes and +// its phase dirs move to `.planning/milestones/v-phases/`, items still +// unresolved at that moment become invisible to every later audit — these +// fixtures carry one item of each of the four kinds, in both an "unresolved" +// (must be counted) and a "resolved" (must NOT be counted) shape, verified +// against the real scanner formats before being repurposed for the archived +// layout below. + +const UAT_GAP_UNRESOLVED = [ + '# UAT', + '', + '## Gaps', + '', + '- truth: an unresolved UAT gap that survived milestone close', + ' status: open', + '', +].join('\n'); + +const UAT_GAP_RESOLVED = [ + '---', + 'status: resolved', + '---', + '', + '# UAT', + '', + '## Gaps', + '', + '- truth: a gap that was resolved', + '', +].join('\n'); + +const VERIFICATION_GAP_UNRESOLVED = [ + '---', + 'status: gaps_found', + '---', + '', + '# Verification', + '', + 'Gaps found during verification.', + '', +].join('\n'); + +const VERIFICATION_GAP_RESOLVED = [ + '---', + 'status: passed', + '---', + '', + '# Verification', + '', + 'All checks passed.', + '', +].join('\n'); + +const CONTEXT_QUESTION_OPEN = [ + '# Context', + '', + '## Open Questions', + '', + '- Should this default to strict mode?', + '', +].join('\n'); + +const CONTEXT_QUESTION_RESOLVED = [ + '# Context', + '', + '## Open Questions', + '', + 'None', + '', +].join('\n'); + +const DEFERRED_ITEM_UNRESOLVED = [ + '# Deferred Items', + '', + '- **STILL-OPEN:** an unresolved deferred item that survived milestone close', + '', +].join('\n'); + +const DEFERRED_ITEM_RESOLVED = [ + '# Deferred Items', + '', + '- **RESOLVED-ITEM:** an item that was resolved', + ' status: resolved', + '', +].join('\n'); + +/** + * Write one phase's worth of UAT/VERIFICATION/CONTEXT/deferred-items files + * (one of each of the four scanner-recognized kinds) into `phaseDir`, using + * `phaseNumberPrefix` (e.g. '01') as the file-token so `scopeToPhase` (#3511) + * accepts them for a dir named `-`. + */ +function writePhaseArtifacts(phaseDir, phaseNumberPrefix, { uat, verification, context, deferred }) { + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, `${phaseNumberPrefix}-UAT.md`), uat); + fs.writeFileSync(path.join(phaseDir, `${phaseNumberPrefix}-VERIFICATION.md`), verification); + fs.writeFileSync(path.join(phaseDir, `${phaseNumberPrefix}-CONTEXT.md`), context); + fs.writeFileSync(path.join(phaseDir, 'deferred-items.md'), deferred); +} + +const UNRESOLVED_ARTIFACTS = { + uat: UAT_GAP_UNRESOLVED, + verification: VERIFICATION_GAP_UNRESOLVED, + context: CONTEXT_QUESTION_OPEN, + deferred: DEFERRED_ITEM_UNRESOLVED, +}; + +const RESOLVED_ARTIFACTS = { + uat: UAT_GAP_RESOLVED, + verification: VERIFICATION_GAP_RESOLVED, + context: CONTEXT_QUESTION_RESOLVED, + deferred: DEFERRED_ITEM_RESOLVED, +}; + describe('audit-open — output shape (#2911)', () => { let tmpDir; @@ -591,6 +710,303 @@ describe('audit-open — output shape (#2911)', () => { ); } }); + + // ── #3458: phase-scoped scanners must also see archived phases ──────────── + // + // scanUatGaps, scanVerificationGaps, scanContextQuestions, and + // scanDeferredItems resolve ONLY the active `.planning/phases/` root. Once a + // milestone closes and its phase dirs move to + // `.planning/milestones/v-phases/`, items still unresolved at that + // moment become invisible to every later audit. + + test('#3458 archived-only project: unresolved items in .planning/milestones/vX.Y-phases/ are counted', () => { + // The active phases root is scaffolded empty by createTempProject; the bug + // report's own repro has it ABSENT entirely in a fully-archived project — + // remove it so this fixture matches that exactly, not just "empty". + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- removing only the .planning/phases subdir within a still-live fixture (INFO-7 fix for #3458 review: fs.rmdirSync threw on a non-empty dir); helpers.cleanup() tears down the whole tmpDir, not a subdirectory, so it cannot substitute here. + fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); + + const archivedPhaseDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '01-alpha'); + writePhaseArtifacts(archivedPhaseDir, '01', UNRESOLVED_ARTIFACTS); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + assert.equal(parsed.counts.uat_gaps, 1, `uat_gaps: expected 1, got ${parsed.counts.uat_gaps}`); + assert.equal(parsed.counts.verification_gaps, 1, `verification_gaps: expected 1, got ${parsed.counts.verification_gaps}`); + assert.equal(parsed.counts.context_questions, 1, `context_questions: expected 1, got ${parsed.counts.context_questions}`); + assert.equal(parsed.counts.deferred_items, 1, `deferred_items: expected 1, got ${parsed.counts.deferred_items}`); + assert.equal(parsed.has_open_items, true, 'has_open_items must be true when an archived phase carries unresolved items'); + }); + + test('#3458 mixed project: active AND archived phases are both scanned and summed (not one replacing the other)', () => { + const activePhaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + writePhaseArtifacts(activePhaseDir, '01', UNRESOLVED_ARTIFACTS); + + const archivedPhaseDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '02-beta'); + writePhaseArtifacts(archivedPhaseDir, '02', UNRESOLVED_ARTIFACTS); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + assert.equal(parsed.counts.uat_gaps, 2, `uat_gaps: expected 2 (1 active + 1 archived), got ${parsed.counts.uat_gaps}`); + assert.equal(parsed.counts.verification_gaps, 2, `verification_gaps: expected 2, got ${parsed.counts.verification_gaps}`); + assert.equal(parsed.counts.context_questions, 2, `context_questions: expected 2, got ${parsed.counts.context_questions}`); + assert.equal(parsed.counts.deferred_items, 2, `deferred_items: expected 2, got ${parsed.counts.deferred_items}`); + assert.equal(parsed.has_open_items, true, 'has_open_items must be true'); + }); + + test('#3458 active-only project: unchanged behavior, unresolved items still counted (guards the pre-existing path)', () => { + const activePhaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + writePhaseArtifacts(activePhaseDir, '01', UNRESOLVED_ARTIFACTS); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + assert.equal(parsed.counts.uat_gaps, 1, `uat_gaps: expected 1, got ${parsed.counts.uat_gaps}`); + assert.equal(parsed.counts.verification_gaps, 1, `verification_gaps: expected 1, got ${parsed.counts.verification_gaps}`); + assert.equal(parsed.counts.context_questions, 1, `context_questions: expected 1, got ${parsed.counts.context_questions}`); + assert.equal(parsed.counts.deferred_items, 1, `deferred_items: expected 1, got ${parsed.counts.deferred_items}`); + assert.equal(parsed.has_open_items, true, 'has_open_items must be true'); + }); + + test('#3458 archived-only project with all-RESOLVED items: contributes 0 (fix must not blindly count archived files)', () => { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- removing only the .planning/phases subdir within a still-live fixture (INFO-7 fix for #3458 review: fs.rmdirSync threw on a non-empty dir); helpers.cleanup() tears down the whole tmpDir, not a subdirectory, so it cannot substitute here. + fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); + + const archivedPhaseDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '01-alpha'); + writePhaseArtifacts(archivedPhaseDir, '01', RESOLVED_ARTIFACTS); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + assert.equal(parsed.counts.uat_gaps, 0, `uat_gaps: expected 0 (resolved), got ${parsed.counts.uat_gaps}`); + assert.equal(parsed.counts.verification_gaps, 0, `verification_gaps: expected 0 (resolved), got ${parsed.counts.verification_gaps}`); + assert.equal(parsed.counts.context_questions, 0, `context_questions: expected 0 (resolved), got ${parsed.counts.context_questions}`); + assert.equal(parsed.counts.deferred_items, 0, `deferred_items: expected 0 (resolved), got ${parsed.counts.deferred_items}`); + assert.equal(parsed.has_open_items, false, 'has_open_items must be false when the only archived phase is fully resolved'); + }); + + // ── phase-directory-name forgery (sanitizeLabel) ─────────────────────────── + // + // A phase directory NAME (not file content) is filesystem-controlled, not + // frontmatter-controlled — a doctored checkout can name a directory + // anything. `phaseNum` falls back to the raw directory name verbatim when + // it doesn't match PHASE_NUMBER_TOKEN_SOURCE, so an embedded `\n`/ESC in + // the NAME itself used to reach the human report unescaped + // (`sanitizeForDisplay` deliberately preserves newlines — it is not the + // right tool for a single-line label). `sanitizeLabel` (src/security.cts) + // closes this by escaping control bytes rather than stripping them. + const FORGED_PHASE_DIR_NAME = 'zz\n0 open items require decisions.\n\x1b[2K\x1b[1G FORGED'; + + /** + * Windows/NTFS forbids control characters (including \n and ESC/0x1B) in + * path components, so `mkdirSync` throws ENOENT there rather than creating + * the doctored directory — the directory-name forgery vector these tests + * exercise does not exist on that platform. `t.skip()` degrades cleanly + * (same convention as trySymlink() in tests/adr-index-gate.test.cjs, which + * skips on EPERM for the analogous symlink-creation gap); a bare `return` + * would silently report a PASS in node:test and hide the gap. The + * sanitizer itself (`sanitizeLabel`) remains fully covered on every + * platform by the platform-independent, filesystem-free string tests in + * tests/security.test.cjs (describe('sanitizeLabel', ...)). + */ + function tryMkdirForgedName(t, dirPath) { + try { + fs.mkdirSync(dirPath, { recursive: true }); + return true; + } catch (err) { + if (err && (err.code === 'ENOENT' || err.code === 'EINVAL')) { + t.skip(`cannot create a directory name with control characters on this platform (${err.code})`); + return false; + } + throw err; + } + } + + test('a phase directory name containing a newline cannot forge a new report line', (t) => { + const forgedPhaseDir = path.join(tmpDir, '.planning', 'phases', FORGED_PHASE_DIR_NAME); + if (!tryMkdirForgedName(t, forgedPhaseDir)) return; + fs.writeFileSync(path.join(forgedPhaseDir, 'deferred-items.md'), DEFERRED_ITEM_UNRESOLVED); + + const result = runGsdTools('audit-open', tmpDir); + assert.ok(result.success, `audit-open must not crash. stderr: ${result.error}`); + + const lines = result.output.split('\n').map(l => l.trim()); + assert.ok( + !lines.includes('0 open items require decisions.'), + `the doctored directory name must not inject its own report line; got lines: ${JSON.stringify(lines)}` + ); + }); + + test('a phase directory name with an ESC/ANSI payload never reaches raw output', (t) => { + const forgedPhaseDir = path.join(tmpDir, '.planning', 'phases', FORGED_PHASE_DIR_NAME); + if (!tryMkdirForgedName(t, forgedPhaseDir)) return; + fs.writeFileSync(path.join(forgedPhaseDir, 'deferred-items.md'), DEFERRED_ITEM_UNRESOLVED); + + const result = runGsdTools('audit-open', tmpDir); + assert.ok(result.success, `audit-open must not crash. stderr: ${result.error}`); + + assert.ok( + !result.output.includes('\x1b'), + 'no raw ESC byte from the doctored directory name may reach the report output' + ); + }); + + // ── adversarial-review follow-ups on #3458 ───────────────────────────────── + + test('WARNING-4a: archived_milestone is present (correct value) on archived items and absent (no key at all) on active items', () => { + const activePhaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + writePhaseArtifacts(activePhaseDir, '01', UNRESOLVED_ARTIFACTS); + + const archivedPhaseDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '02-beta'); + writePhaseArtifacts(archivedPhaseDir, '02', UNRESOLVED_ARTIFACTS); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + for (const category of ['uat_gaps', 'verification_gaps', 'context_questions', 'deferred_items']) { + const items = parsed.items[category].filter(i => !i.scan_error); + const active = items.find(i => i.phase === '01'); + const archived = items.find(i => i.phase === '02'); + assert.ok(active, `${category}: expected an active-phase item; got: ${JSON.stringify(items)}`); + assert.ok(archived, `${category}: expected an archived-phase item; got: ${JSON.stringify(items)}`); + assert.strictEqual('archived_milestone' in active, false, + `${category}: active item must not carry the archived_milestone key at all`); + assert.strictEqual(archived.archived_milestone, 'v1.0', + `${category}: archived item must carry archived_milestone: 'v1.0'`); + } + }); + + test('BLOCKER-1 regression: an unreadable active root (a FILE at .planning/phases) still yields a scan_error sentinel in all four phase-scoped categories', () => { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- removing only the .planning/phases subdir within a still-live fixture (INFO-7 fix for #3458 review: fs.rmdirSync threw on a non-empty dir); helpers.cleanup() tears down the whole tmpDir, not a subdirectory, so it cannot substitute here. + fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'phases'), 'not a directory'); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + for (const category of ['uat_gaps', 'verification_gaps', 'context_questions', 'deferred_items']) { + const sentinel = parsed.items[category].find(i => i.scan_error === true); + assert.ok(sentinel, + `${category}: expected a scan_error sentinel when the active root is unreadable (ENOTDIR); ` + + `got: ${JSON.stringify(parsed.items[category])}`); + } + }); + + test('an unreadable archived root does not prevent the active half from being scanned (no sentinel for the archive half — see docstring)', () => { + const activePhaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + writePhaseArtifacts(activePhaseDir, '01', UNRESOLVED_ARTIFACTS); + + // Make `.planning/milestones` an unreadable FILE instead of a directory so + // getArchivedPhaseDirs's readdirSync throws (ENOTDIR). + fs.writeFileSync(path.join(tmpDir, '.planning', 'milestones'), 'not a directory'); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + // Active half still scanned — the four counts include the active item. + assert.equal(parsed.counts.uat_gaps, 1, `uat_gaps: expected 1 (active only), got ${parsed.counts.uat_gaps}`); + assert.equal(parsed.counts.verification_gaps, 1, `verification_gaps: expected 1, got ${parsed.counts.verification_gaps}`); + assert.equal(parsed.counts.context_questions, 1, `context_questions: expected 1, got ${parsed.counts.context_questions}`); + assert.equal(parsed.counts.deferred_items, 1, `deferred_items: expected 1, got ${parsed.counts.deferred_items}`); + + // Pinned decision: an unreadable archive root does NOT get a scan_error + // sentinel (no pre-#3458 contract to preserve for it — see the + // listAuditPhaseTargets docstring). + for (const category of ['uat_gaps', 'verification_gaps', 'context_questions', 'deferred_items']) { + const hasSentinel = parsed.items[category].some(i => i.scan_error === true); + assert.strictEqual(hasSentinel, false, + `${category}: an unreadable archive root must not produce a scan_error sentinel; got: ${JSON.stringify(parsed.items[category])}`); + } + }); + + test('WARNING-3: duplicate phase name across active and archived roots produces two distinct entries, and the human report distinguishes them', () => { + const activePhaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + writePhaseArtifacts(activePhaseDir, '01', UNRESOLVED_ARTIFACTS); + + const archivedPhaseDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '01-alpha'); + writePhaseArtifacts(archivedPhaseDir, '01', UNRESOLVED_ARTIFACTS); + + const jsonResult = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(jsonResult.success, `audit-open --json must not crash. stderr: ${jsonResult.error}`); + const parsed = JSON.parse(jsonResult.output); + + const uatEntries = parsed.items.uat_gaps.filter(i => !i.scan_error); + assert.equal(uatEntries.length, 2, + `same-named active + archived phase must produce two distinct uat_gaps entries; got: ${JSON.stringify(uatEntries)}`); + const archivedFlags = uatEntries.map(i => Boolean(i.archived_milestone)).sort(); + assert.deepEqual(archivedFlags, [false, true], + 'exactly one of the two duplicate-named entries must carry archived_milestone'); + + const textResult = runGsdTools(['audit-open'], tmpDir); + assert.ok(textResult.success, `audit-open (text) must not crash. stderr: ${textResult.error}`); + const uatLines = textResult.output.split('\n').filter(l => l.includes('01-UAT.md')); + assert.equal(uatLines.length, 2, + `expected two UAT-gap report lines (one active, one archived); got: ${JSON.stringify(uatLines)}`); + assert.notStrictEqual(uatLines[0], uatLines[1], + 'the active and archived duplicate-named entries must render as distinguishable lines, not byte-identical duplicates'); + assert.ok(uatLines.some(l => l.includes('archived v1.0')), + `expected one report line to be labeled with its archived milestone; got: ${JSON.stringify(uatLines)}`); + }); + + test('INFO-5: archived milestones v1.0, v1.9, v1.10 sort newest-first, numerically (v1.10 before v1.9, not lexicographically)', () => { + writePhaseArtifacts(path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '01-a'), '01', UNRESOLVED_ARTIFACTS); + writePhaseArtifacts(path.join(tmpDir, '.planning', 'milestones', 'v1.9-phases', '01-b'), '01', UNRESOLVED_ARTIFACTS); + writePhaseArtifacts(path.join(tmpDir, '.planning', 'milestones', 'v1.10-phases', '01-c'), '01', UNRESOLVED_ARTIFACTS); + + const result = runGsdTools(['audit-open', '--json'], tmpDir); + assert.ok(result.success, `audit-open --json must not crash. stderr: ${result.error}`); + const parsed = JSON.parse(result.output); + + const milestoneOrder = parsed.items.uat_gaps.filter(i => !i.scan_error).map(i => i.archived_milestone); + assert.deepEqual(milestoneOrder, ['v1.10', 'v1.9', 'v1.0'], + `expected numeric-aware newest-first ordering (v1.10, v1.9, v1.0); got: ${JSON.stringify(milestoneOrder)}`); + }); + + // ── quick-task directory-name forgery (sanitizeLabel) ────────────────────── + // + // scanQuickTasks derives `slug` from the `.planning/quick/` + // directory NAME (filesystem-controlled, same shape as the phase-directory + // case above), not from file content. Before sanitizeLabel was applied + // here, an embedded `\n`/ESC byte in the directory name reached the human + // report unescaped via `sanitizeForDisplay` (which deliberately preserves + // newlines — it is not the right tool for a single-line label). + const FORGED_QUICK_TASK_DIR_NAME = 'zz\n0 open items require decisions.\n\x1b[2K\x1b[1G FORGED'; + + test('a quick-task directory name containing a newline cannot forge a new report line', (t) => { + const forgedQuickDir = path.join(tmpDir, '.planning', 'quick', FORGED_QUICK_TASK_DIR_NAME); + if (!tryMkdirForgedName(t, forgedQuickDir)) return; + + const result = runGsdTools('audit-open', tmpDir); + assert.ok(result.success, `audit-open must not crash. stderr: ${result.error}`); + + const lines = result.output.split('\n').map(l => l.trim()); + assert.ok( + !lines.includes('0 open items require decisions.'), + `the doctored directory name must not inject its own report line; got lines: ${JSON.stringify(lines)}` + ); + }); + + test('a quick-task directory name with an ESC/ANSI payload never reaches raw output', (t) => { + const forgedQuickDir = path.join(tmpDir, '.planning', 'quick', FORGED_QUICK_TASK_DIR_NAME); + if (!tryMkdirForgedName(t, forgedQuickDir)) return; + + const result = runGsdTools('audit-open', tmpDir); + assert.ok(result.success, `audit-open must not crash. stderr: ${result.error}`); + + assert.ok( + !result.output.includes('\x1b'), + 'no raw ESC byte from the doctored directory name may reach the report output' + ); + }); }); }); } @@ -1192,3 +1608,806 @@ describe('bug #950: quick-task SUMMARY must carry status: complete', () => { }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// #3458 follow-up: `audit_acknowledged` suppression seam + the +// `audit-open acknowledge` CLI writer. +// +// BACKGROUND: #3458 made `audit-open` scan archived milestone phase dirs too, +// so an item still unresolved when a milestone closed now resurfaces at +// EVERY later close, forever — `[A] Acknowledge all` documented that +// decision to STATE.md but never suppressed it. This suite verifies the +// suppression seam: `audit_acknowledged` is VERDICT-PRESERVING (never +// touches the artifact's own `status:` verdict) and SELF-INVALIDATING (a +// stale acknowledgment resurfaces automatically the moment the artifact's +// current state stops matching the marker's recorded snapshot). +// ──────────────────────────────────────────────────────────────────────── +{ + const fs = require('node:fs'); + const path = require('node:path'); + const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); + + function readJson(result) { + assert.ok(result.success, `command must succeed. stdout: ${result.output}\nstderr: ${result.error}`); + return JSON.parse(result.output); + } + + function ack(tmpDir, args) { + return runGsdTools(['audit-open', 'acknowledge', ...args, '--json'], tmpDir); + } + + function audit(tmpDir) { + return readJson(runGsdTools(['audit-open', '--json'], tmpDir)); + } + + describe('audit-open acknowledge — suppression seam (#3458 follow-up)', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject('gsd-3458-ack-'); }); + afterEach(() => { cleanup(tmpDir); }); + + function planningPath(...segs) { + return path.join(tmpDir, '.planning', ...segs); + } + + // ── per-category suppression + verdict preservation ─────────────────── + + test('debug_sessions: acknowledged item drops out of counts/has_open_items; status: field unchanged', () => { + const debugDir = planningPath('debug'); + fs.mkdirSync(debugDir, { recursive: true }); + const filePath = path.join(debugDir, 'investigate.md'); + fs.writeFileSync(filePath, '---\nstatus: open\n---\n## Current Focus\ndigging\n'); + + assert.equal(audit(tmpDir).counts.debug_sessions, 1); + + const result = ack(tmpDir, ['--category', 'debug_sessions', '--slug', 'investigate', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.debug_sessions, 0); + assert.equal(after.acknowledged.debug_sessions, 1); + assert.equal(after.has_open_items, false); + assert.match(fs.readFileSync(filePath, 'utf-8'), /^status: open$/m, 'verdict-preserving: status: must be unchanged'); + }); + + test('quick_tasks: acknowledged item drops out of counts; status: field unchanged', () => { + const taskDir = planningPath('quick', '20260810-fixthing'); + fs.mkdirSync(taskDir, { recursive: true }); + const filePath = path.join(taskDir, '20260810-fixthing-SUMMARY.md'); + fs.writeFileSync(filePath, '---\nstatus: needs_review\n---\nbody\n'); + + assert.equal(audit(tmpDir).counts.quick_tasks, 1); + + const result = ack(tmpDir, ['--category', 'quick_tasks', '--dir', '20260810-fixthing', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.quick_tasks, 0); + assert.equal(after.acknowledged.quick_tasks, 1); + assert.match(fs.readFileSync(filePath, 'utf-8'), /^status: needs_review$/m, 'verdict-preserving: status: must be unchanged'); + }); + + test('quick_tasks: task with NO SUMMARY.md at all can still be acknowledged (writer creates the marker file)', () => { + const taskDir = planningPath('quick', '20260811-nosummary'); + fs.mkdirSync(taskDir, { recursive: true }); + + assert.equal(audit(tmpDir).counts.quick_tasks, 1); + + const result = ack(tmpDir, ['--category', 'quick_tasks', '--dir', '20260811-nosummary', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.quick_tasks, 0); + assert.equal(after.acknowledged.quick_tasks, 1); + }); + + test('threads: acknowledged item drops out of counts; status: field unchanged', () => { + const threadsDir = planningPath('threads'); + fs.mkdirSync(threadsDir, { recursive: true }); + const filePath = path.join(threadsDir, 'design-debate.md'); + fs.writeFileSync(filePath, '---\nstatus: open\n---\n# Thread: design debate\n'); + + assert.equal(audit(tmpDir).counts.threads, 1); + + const result = ack(tmpDir, ['--category', 'threads', '--slug', 'design-debate', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.threads, 0); + assert.equal(after.acknowledged.threads, 1); + assert.match(fs.readFileSync(filePath, 'utf-8'), /^status: open$/m, 'verdict-preserving: status: must be unchanged'); + }); + + test('seeds: acknowledged item drops out of counts; status: field unchanged', () => { + const seedsDir = planningPath('seeds'); + fs.mkdirSync(seedsDir, { recursive: true }); + const filePath = path.join(seedsDir, 'SEED-idea.md'); + fs.writeFileSync(filePath, '---\nstatus: dormant\n---\n# An idea\n'); + + assert.equal(audit(tmpDir).counts.seeds, 1); + + const result = ack(tmpDir, ['--category', 'seeds', '--seed-id', 'SEED-idea', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.seeds, 0); + assert.equal(after.acknowledged.seeds, 1); + assert.match(fs.readFileSync(filePath, 'utf-8'), /^status: dormant$/m, 'verdict-preserving: status: must be unchanged'); + }); + + test('todos: acknowledged item drops out of counts (presence-only — no snapshot field)', () => { + const pendingDir = planningPath('todos', 'pending'); + fs.mkdirSync(pendingDir, { recursive: true }); + const filePath = path.join(pendingDir, 'fix-thing.md'); + fs.writeFileSync(filePath, '---\npriority: low\narea: docs\n---\nFix the thing\n'); + + assert.equal(audit(tmpDir).counts.todos, 1); + + const result = ack(tmpDir, ['--category', 'todos', '--filename', 'fix-thing.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.todos, 0); + assert.equal(after.acknowledged.todos, 1); + }); + + test('uat_gaps: acknowledged item drops out of counts; status: field unchanged (CLI writer round-trip)', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-UAT.md'); + fs.writeFileSync(filePath, '---\nstatus: gaps_found\n---\n# UAT\n\n## Gaps\n\n- truth: "something broke"\n status: open\n'); + + assert.equal(audit(tmpDir).counts.uat_gaps, 1); + + const result = ack(tmpDir, ['--category', 'uat_gaps', '--phase', '01', '--file', '01-UAT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.uat_gaps, 0); + assert.equal(after.acknowledged.uat_gaps, 1); + assert.equal(after.has_open_items, false); + assert.match( + fs.readFileSync(filePath, 'utf-8'), /^status: gaps_found$/m, + 'CLI writer round-trip: the artifact\'s own status: must be UNCHANGED after acknowledge (verdict-preserving)', + ); + }); + + test('verification_gaps: acknowledged item drops out of counts; status: field unchanged', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-VERIFICATION.md'); + fs.writeFileSync(filePath, '---\nstatus: gaps_found\n---\n# Verification\n\nGaps found.\n'); + + assert.equal(audit(tmpDir).counts.verification_gaps, 1); + + const result = ack(tmpDir, ['--category', 'verification_gaps', '--phase', '01', '--file', '01-VERIFICATION.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.verification_gaps, 0); + assert.equal(after.acknowledged.verification_gaps, 1); + assert.match(fs.readFileSync(filePath, 'utf-8'), /^status: gaps_found$/m, 'verdict-preserving: status: must be unchanged'); + }); + + test('context_questions: acknowledged item drops out of counts; question_count snapshot recorded', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-CONTEXT.md'); + fs.writeFileSync(filePath, '# Context\n\n## Open Questions\n\n- Which backend?\n- What about auth?\n'); + + assert.equal(audit(tmpDir).counts.context_questions, 1); + + const result = ack(tmpDir, ['--category', 'context_questions', '--phase', '01', '--file', '01-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.context_questions, 0); + assert.equal(after.acknowledged.context_questions, 1); + // WARNING 2 (#3458 follow-up review): the marker snapshots a content + // digest of the FULL question set, not a bare count — a count-only + // snapshot cannot see a same-count REPLACEMENT of every question (see + // the WARNING-2 disproof tests below). + assert.match(fs.readFileSync(filePath, 'utf-8'), /questions_digest: [0-9a-f]{64}/, 'marker records the questions_digest snapshot'); + }); + + test('deferred_items: acknowledged entry drops out of counts; entry text otherwise unchanged', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + fs.writeFileSync(filePath, '## Deferred Items\n\n- an out of scope thing\n severity: low\n'); + + assert.equal(audit(tmpDir).counts.deferred_items, 1); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'an out of scope thing severity: low', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const after = audit(tmpDir); + assert.equal(after.counts.deferred_items, 0); + assert.equal(after.acknowledged.deferred_items, 1); + const content = fs.readFileSync(filePath, 'utf-8'); + assert.match(content, /an out of scope thing/, 'entry text is preserved'); + assert.match(content, /status: acknowledged/, 'entry now carries status: acknowledged'); + assert.match(content, /severity: low/, 'sibling field is preserved'); + }); + + // ── SELF-INVALIDATION: edit the artifact after acknowledging → resurfaces ── + + test('SELF-INVALIDATION debug_sessions: status changes after acknowledge → item resurfaces', () => { + const debugDir = planningPath('debug'); + fs.mkdirSync(debugDir, { recursive: true }); + const filePath = path.join(debugDir, 'investigate.md'); + fs.writeFileSync(filePath, '---\nstatus: open\n---\n## Current Focus\ndigging\n'); + + assert.ok(ack(tmpDir, ['--category', 'debug_sessions', '--slug', 'investigate', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.debug_sessions, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: open', 'status: in_progress'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.debug_sessions, 1, 'AFTER edit: resurfaces — stale acknowledgment no longer applies'); + }); + + test('SELF-INVALIDATION quick_tasks: status changes after acknowledge → item resurfaces', () => { + const taskDir = planningPath('quick', '20260810-fixthing'); + fs.mkdirSync(taskDir, { recursive: true }); + const filePath = path.join(taskDir, '20260810-fixthing-SUMMARY.md'); + fs.writeFileSync(filePath, '---\nstatus: needs_review\n---\nbody\n'); + + assert.ok(ack(tmpDir, ['--category', 'quick_tasks', '--dir', '20260810-fixthing', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.quick_tasks, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: needs_review', 'status: in_progress'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.quick_tasks, 1, 'AFTER edit: resurfaces'); + }); + + test('SELF-INVALIDATION threads: status changes after acknowledge → item resurfaces', () => { + const threadsDir = planningPath('threads'); + fs.mkdirSync(threadsDir, { recursive: true }); + const filePath = path.join(threadsDir, 'design-debate.md'); + fs.writeFileSync(filePath, '---\nstatus: open\n---\n# Thread: design debate\n'); + + assert.ok(ack(tmpDir, ['--category', 'threads', '--slug', 'design-debate', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.threads, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: open', 'status: in_progress'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.threads, 1, 'AFTER edit: resurfaces (still open, but a DIFFERENT open status than the snapshot)'); + }); + + test('SELF-INVALIDATION seeds: status changes after acknowledge → item resurfaces', () => { + const seedsDir = planningPath('seeds'); + fs.mkdirSync(seedsDir, { recursive: true }); + const filePath = path.join(seedsDir, 'SEED-idea.md'); + fs.writeFileSync(filePath, '---\nstatus: dormant\n---\n# An idea\n'); + + assert.ok(ack(tmpDir, ['--category', 'seeds', '--seed-id', 'SEED-idea', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.seeds, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: dormant', 'status: active'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.seeds, 1, 'AFTER edit: resurfaces'); + }); + + test('SELF-INVALIDATION uat_gaps: status changes after acknowledge → item resurfaces', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-UAT.md'); + fs.writeFileSync(filePath, '---\nstatus: gaps_found\n---\n# UAT\n\n## Gaps\n\n- truth: "something broke"\n status: open\n'); + + assert.ok(ack(tmpDir, ['--category', 'uat_gaps', '--phase', '01', '--file', '01-UAT.md', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.uat_gaps, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: gaps_found', 'status: human_needed'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.uat_gaps, 1, 'AFTER edit: resurfaces'); + }); + + test('SELF-INVALIDATION verification_gaps: status changes after acknowledge → item resurfaces', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-VERIFICATION.md'); + fs.writeFileSync(filePath, '---\nstatus: gaps_found\n---\n# Verification\n\nGaps found.\n'); + + assert.ok(ack(tmpDir, ['--category', 'verification_gaps', '--phase', '01', '--file', '01-VERIFICATION.md', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.verification_gaps, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: gaps_found', 'status: human_needed'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.verification_gaps, 1, 'AFTER edit: resurfaces'); + }); + + test('SELF-INVALIDATION context_questions: question_count changes after acknowledge → item resurfaces', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-CONTEXT.md'); + fs.writeFileSync(filePath, '# Context\n\n## Open Questions\n\n- Which backend?\n- What about auth?\n'); + + assert.ok(ack(tmpDir, ['--category', 'context_questions', '--phase', '01', '--file', '01-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.context_questions, 0, 'BEFORE edit: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8') + '- What about the third thing?\n'; + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.context_questions, 1, 'AFTER a new open question is added: resurfaces'); + }); + + test('SELF-INVALIDATION deferred_items: reopening the entry (status changed away from acknowledged) → item resurfaces', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + fs.writeFileSync(filePath, '## Deferred Items\n\n- an out of scope thing\n severity: low\n'); + + assert.ok(ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'an out of scope thing severity: low', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.deferred_items, 0, 'BEFORE reopen: suppressed'); + + const content = fs.readFileSync(filePath, 'utf-8').replace('status: acknowledged', 'status: reopened'); + fs.writeFileSync(filePath, content); + + assert.equal(audit(tmpDir).counts.deferred_items, 1, 'AFTER reopen: resurfaces'); + }); + + // ── malformed marker never suppresses ────────────────────────────────── + + test('malformed audit_acknowledged (not a map) does NOT suppress — item still surfaces', () => { + const debugDir = planningPath('debug'); + fs.mkdirSync(debugDir, { recursive: true }); + const filePath = path.join(debugDir, 'investigate.md'); + // Hand-authored, deliberately malformed: audit_acknowledged is a bare + // scalar, not a map — must be treated as ABSENT, never suppress. + fs.writeFileSync(filePath, '---\nstatus: open\naudit_acknowledged: not-a-map\n---\nstill open\n'); + + const parsed = audit(tmpDir); + assert.equal(parsed.counts.debug_sessions, 1, 'a malformed marker must never suppress'); + assert.equal(parsed.acknowledged.debug_sessions, 0); + }); + + test('malformed audit_acknowledged (missing milestone/at) does NOT suppress — item still surfaces', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-UAT.md'); + fs.writeFileSync( + filePath, + '---\nstatus: gaps_found\naudit_acknowledged:\n status: gaps_found\n---\n# UAT\n\n## Gaps\n\n- truth: "x"\n status: open\n', + ); + + const parsed = audit(tmpDir); + assert.equal(parsed.counts.uat_gaps, 1, 'a marker missing milestone/at must never suppress'); + }); + + // ── deferred_items status matrix ─────────────────────────────────────── + + test('deferred_items status matrix: acknowledged suppresses, resolved still suppresses, no status still surfaces', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + fs.writeFileSync( + filePath, + [ + '## Deferred Items', + '', + '- an acknowledged item', + ' status: acknowledged', + '- a resolved item', + ' status: resolved', + '- a plain open item with no status field', + '', + ].join('\n'), + ); + + const parsed = audit(tmpDir); + assert.equal(parsed.counts.deferred_items, 1, 'only the no-status entry is open'); + assert.equal(parsed.acknowledged.deferred_items, 1, 'the acknowledged entry is tallied, not silenced'); + assert.deepEqual( + parsed.items.deferred_items.map((i) => i.text), + ['a plain open item with no status field'], + ); + }); + + // ── BLOCKER 1 (#3458 follow-up review): deferred_items writer must be + // section-anchored, never write into the wrong span, and refuse rather + // than guess on every shape it cannot safely handle ──────────────────── + + test('BLOCKER 1: an identical bullet OUTSIDE `## Deferred Items` is never targeted — mixed-section fixture', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + // The SAME bullet text appears once under an unrelated `# Notes` + // section and once under `## Deferred Items`. Before the fix, the + // unanchored regex matched the FIRST occurrence anywhere in the file — + // i.e. the one under `# Notes` — not the one `matches`/`ambiguous` + // were computed over. + fs.writeFileSync( + filePath, + [ + '# Notes', + '', + '- Fix the parser', + '', + '## Deferred Items', + '', + '- Fix the parser', + '', + ].join('\n'), + ); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'Fix the parser', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const content = fs.readFileSync(filePath, 'utf-8'); + const notesSection = content.slice(content.indexOf('# Notes'), content.indexOf('## Deferred Items')); + const deferredSection = content.slice(content.indexOf('## Deferred Items')); + assert.doesNotMatch(notesSection, /status: acknowledged/, 'the UNRELATED # Notes bullet must never be touched'); + assert.match(deferredSection, /status: acknowledged/, 'the actual Deferred Items entry must carry the marker'); + + const after = audit(tmpDir); + assert.equal(after.counts.deferred_items, 0, 're-audit: the correct entry is suppressed'); + assert.equal(after.acknowledged.deferred_items, 1); + }); + + test('BLOCKER 1: --text matching 2+ deferred entries is refused as ambiguous, nothing written', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + const before = ['## Deferred Items', '', '- duplicated text', '- duplicated text', ''].join('\n'); + fs.writeFileSync(filePath, before); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'duplicated text', '--milestone', 'v1.0']); + assert.equal(result.success, false, 'ambiguous --text must be refused'); + assert.match(result.error, /matches more than one/i); + assert.equal(fs.readFileSync(filePath, 'utf-8'), before, 'file must be byte-identical — nothing written on refusal'); + }); + + test('BLOCKER 1: --text matching no deferred entry is refused as not_found', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + fs.writeFileSync(filePath, '## Deferred Items\n\n- a real entry\n'); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'no such entry', '--milestone', 'v1.0']); + assert.equal(result.success, false, 'unmatched --text must be refused'); + assert.match(result.error, /no deferred item matched/i); + }); + + test('BLOCKER 1: heading-delimited (#3457) deferred-items shape is refused as unsupported_heading_shape, not guessed at', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + const before = ['## Deferred Items', '', '### Something out of scope', '', 'Some detail line.', ''].join('\n'); + fs.writeFileSync(filePath, before); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'Something out of scope', '--milestone', 'v1.0']); + assert.equal(result.success, false, 'heading-delimited shape must be refused'); + assert.match(result.error, /heading-delimited/i); + assert.equal(fs.readFileSync(filePath, 'utf-8'), before, 'file must be byte-identical — nothing written on refusal'); + }); + + // ── F1 (#3458 follow-up review, HIGH): the writer must splice by the + // SELECTED entry's own carried span, never re-find it by searching — + // otherwise a byte-identical substring living inside an EARLIER entry + // (a continuation/quoted line) can steal the write ───────────────────── + + test('F1: a target entry text appearing as a continuation line INSIDE an earlier entry is never targeted — the earlier (CRITICAL) entry is untouched', () => { + const phaseDir = planningPath('phases', '03-x'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + const before = [ + '## Deferred Items', + '', + '- CRITICAL unfixed auth bypass', + ' see also: - minor typo', + '- minor typo', + '', + ].join('\n'); + fs.writeFileSync(filePath, before); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '03', '--file', 'deferred-items.md', '--text', 'minor typo', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const content = fs.readFileSync(filePath, 'utf-8'); + // Derive the CRITICAL entry's block by LINES, not by `content.indexOf('- minor typo')` + // on the raw string — that substring also occurs INSIDE the CRITICAL entry's own + // continuation line (" see also: - minor typo"), so an indexOf-based slice truncates + // before the continuation line is fully captured. Walk lines from the CRITICAL bullet + // up to (not including) the next TOP-LEVEL bullet (a line starting with "- ", no + // leading indentation) to get the entry's own span, continuation lines included. + const lines = splitLines(content); + const criticalIdx = lines.findIndex((l) => l.startsWith('- CRITICAL')); + let criticalEndIdx = lines.length; + for (let i = criticalIdx + 1; i < lines.length; i++) { + if (lines[i].startsWith('- ')) { criticalEndIdx = i; break; } + } + const criticalBlock = lines.slice(criticalIdx, criticalEndIdx).join('\n'); + assert.doesNotMatch(criticalBlock, /status: acknowledged/, 'the CRITICAL entry (and its continuation line) must NEVER be touched'); + assert.match(criticalBlock, /see also: - minor typo/, 'the CRITICAL entry continuation line is preserved verbatim'); + // Measured: the write seam's `_normalizeMd` (src/shell-command-projection.cts:837) + // inserts a blank line before a list item whose predecessor is a non-blank, non-list + // line — so a blank line appears between the CRITICAL continuation line and the + // "- minor typo" bullet after this write. That is repo-wide `.md`-write normalization + // (50 callers through the single write seam), not something specific to this feature. + assert.match(content, /- minor typo\n {2}status: acknowledged/, 'the standalone "minor typo" entry (its OWN span) now carries the marker'); + + const after = audit(tmpDir); + assert.equal(after.counts.deferred_items, 1, 're-audit: the CRITICAL entry is still open'); + assert.equal(after.acknowledged.deferred_items, 1, 're-audit: only the typo entry is acknowledged'); + assert.deepEqual( + after.items.deferred_items.filter((i) => !i.scan_error).map((i) => i.text), + ['CRITICAL unfixed auth bypass see also: - minor typo'], + 'the still-open item must be the CRITICAL one, not suppressed', + ); + }); + + test('F1 (weaker/prose variant): the target text also appears as a decoy substring INLINE inside an earlier entry\'s prose — the decoy prose must never be corrupted, and the one real matching entry is acknowledged (pre-fix: the decoy prose line was split mid-sentence, the real entry was never touched, and the CLI still exited 0)', () => { + const phaseDir = planningPath('phases', '03-x'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + const before = [ + '## Deferred Items', + '', + '- Note: reference - minor typo elsewhere, ignore', + '- minor typo', + '', + ].join('\n'); + fs.writeFileSync(filePath, before); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '03', '--file', 'deferred-items.md', '--text', 'minor typo', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed — the real "minor typo" entry unambiguously matches. stderr: ${result.error}`); + + const content = fs.readFileSync(filePath, 'utf-8'); + assert.match( + content, + /- Note: reference - minor typo elsewhere, ignore\n/, + 'the decoy prose line must be preserved VERBATIM, never split mid-sentence by an inserted status: field', + ); + assert.match(content, /- minor typo\n {2}status: acknowledged/, 'the real, standalone "minor typo" entry (its OWN carried span) is the one acknowledged'); + + const after = audit(tmpDir); + // The decoy `- Note: reference - minor typo elsewhere, ignore` line is ITSELF a + // separate, un-acknowledged deferred entry — it was never targeted or written to, + // so it remains open. Only the real "minor typo" entry was suppressed. + assert.equal(after.counts.deferred_items, 1, 're-audit: the decoy Note entry remains open — it was never acknowledged'); + assert.equal(after.acknowledged.deferred_items, 1, 're-audit: only the real "minor typo" entry is acknowledged'); + assert.deepEqual( + after.items.deferred_items.filter((i) => !i.scan_error).map((i) => i.text), + ['Note: reference - minor typo elsewhere, ignore'], + 'the one remaining open item is the decoy Note entry — proving the REAL entry (not the decoy) was the one suppressed', + ); + }); + + test('F1: --text matching only a SUBSTRING of a prose entry (no entry\'s OWN text equals it) is refused as not_found, not silently corrupted', () => { + const phaseDir = planningPath('phases', '03-x'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + const before = [ + '## Deferred Items', + '', + '- Some unrelated note mentioning minor typo inline as commentary', + '', + ].join('\n'); + fs.writeFileSync(filePath, before); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '03', '--file', 'deferred-items.md', '--text', 'minor typo', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.equal(result.success, false, 'a --text that only matches a SUBSTRING of an entry (not the whole entry) must be refused, never silently split/corrupted'); + assert.match(result.error, /no deferred item matched/i); + assert.equal(fs.readFileSync(filePath, 'utf-8'), before, 'file must be byte-identical — nothing written on refusal'); + + const after = audit(tmpDir); + assert.equal(after.counts.deferred_items, 1, 'the prose entry remains open and intact — not silently acknowledged/corrupted'); + }); + + // ── WARNING 1 (#3458 follow-up review): every `.md` write normalizes to + // LF — a CRLF deferred-items.md is normalized, not byte-preserved, + // matching every other `.md` writer in this codebase ───────────────── + + test('WARNING 1: acknowledging an entry in a CRLF deferred-items.md normalizes the whole file to LF (no dead CRLF preservation)', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, 'deferred-items.md'); + fs.writeFileSync(filePath, '## Deferred Items\r\n\r\n- a crlf entry\r\n severity: low\r\n'); + + const result = ack(tmpDir, ['--category', 'deferred_items', '--phase', '01', '--file', 'deferred-items.md', '--text', 'a crlf entry severity: low', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + + const content = fs.readFileSync(filePath, 'utf-8'); + assert.ok(!content.includes('\r'), 'the whole file normalizes to LF on any .md write — no stray \\r bytes'); + assert.match(content, /status: acknowledged/, 'the entry still carries the marker after normalization'); + assert.match(content, /a crlf entry/, 'entry text is preserved'); + + const after = audit(tmpDir); + assert.equal(after.counts.deferred_items, 0); + assert.equal(after.acknowledged.deferred_items, 1); + }); + + // ── BLOCKER 2 (#3458 follow-up review): todos beyond the display cap + // must not be permanently hidden by acknowledging the displayed 5 ───── + + test('BLOCKER 2: acknowledging the 5 displayed todos surfaces the remaining 2, not zero — filter-before-cap', () => { + const pendingDir = planningPath('todos', 'pending'); + fs.mkdirSync(pendingDir, { recursive: true }); + for (let i = 1; i <= 7; i++) { + fs.writeFileSync(path.join(pendingDir, `t${i}.md`), `---\npriority: low\narea: misc\n---\ntodo ${i}\n`); + } + + const before = audit(tmpDir); + assert.equal(before.counts.todos, 5, 'display cap: 5 of 7 shown in one scan'); + assert.equal(before.has_open_items, true); + + const shown = before.items.todos.filter((i) => !i.scan_error && !i._remainder_count).map((i) => i.filename); + assert.equal(shown.length, 5); + + for (const filename of shown) { + const result = ack(tmpDir, ['--category', 'todos', '--filename', filename, '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed for ${filename}. stderr: ${result.error}`); + } + + const after = audit(tmpDir); + // Pre-fix this was 0 (the 2 unshown files were permanently invisible — + // `mdFiles.length` drove both the cap and the remainder count, so once + // the raw 7 dropped to the still-raw-7-minus-nothing count computation + // never noticed 2 files had never been shown at all). + assert.equal(after.counts.todos, 2, 'the 2 never-displayed todos must still surface'); + assert.equal(after.has_open_items, true, 'must not report clean while 2 todos remain unacknowledged'); + assert.equal(after.acknowledged.todos, 5); + }); + + // ── WARNING 2 (#3458 follow-up review): the snapshot must identify + // CONTENT, not just its size — a same-count/same-status change must + // still resurface ─────────────────────────────────────────────────── + + test('WARNING-2 disproof: replacing every acknowledged open_question with a NEW one (same count) resurfaces the item', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-CONTEXT.md'); + fs.writeFileSync(filePath, '---\nopen_questions:\n - "Which backend?"\n - "What about auth?"\n---\n# Context\n'); + + assert.ok(ack(tmpDir, ['--category', 'context_questions', '--phase', '01', '--file', '01-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + assert.equal(audit(tmpDir).counts.context_questions, 0, 'BEFORE replacement: suppressed'); + + // Same COUNT (2), completely different TEXT. + fs.writeFileSync(filePath, '---\nopen_questions:\n - "BRAND NEW BLOCKER: is data loss possible?"\n - "ANOTHER NEW BLOCKER: auth bypass?"\n---\n# Context\n'); + + const after = audit(tmpDir); + assert.equal(after.counts.context_questions, 1, 'AFTER replacement: must RESURFACE — a count-only snapshot cannot see this'); + assert.deepEqual( + after.items.context_questions.filter((i) => !i.scan_error).map((i) => i.questions), + [['BRAND NEW BLOCKER: is data loss possible?', 'ANOTHER NEW BLOCKER: auth bypass?']], + ); + }); + + // ── F2 (#3458 follow-up review): the digest must see the WHOLE question + // set, not the first-3-display-truncated slice `deriveOpenQuestions` used + // to hash — a 4th+ question was invisible to the snapshot ───────────── + + test('F2: a 4th open question added after acknowledging a 3-question body-section set RESURFACES the item (digest was blind past position 3)', () => { + const phaseDir = planningPath('phases', '02-beta'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '02-CONTEXT.md'); + fs.writeFileSync( + filePath, + '# Context\n\n## Open Questions\n\n- Q1?\n- Q2?\n- Q3?\n- Q4?\n', + ); + + const before = audit(tmpDir); + const beforeItem = before.items.context_questions.find((i) => !i.scan_error && i.file === '02-CONTEXT.md'); + assert.equal(beforeItem.question_count, 4, 'question_count must reflect all 4 questions, not the display cap'); + + const result = ack(tmpDir, ['--category', 'context_questions', '--phase', '02', '--file', '02-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(result.success, `acknowledge must succeed. stderr: ${result.error}`); + assert.equal(audit(tmpDir).items.context_questions.filter((i) => !i.scan_error && i.file === '02-CONTEXT.md').length, 0, 'BEFORE mutation: suppressed'); + + // Q1–Q3 UNCHANGED (still the first 3 lines — a pre-fix digest hashing + // only `slice(0, 3)` would see NO difference at all); Q4 replaced with + // two brand-new unanswered blockers. + fs.writeFileSync( + filePath, + '# Context\n\n## Open Questions\n\n- Q1?\n- Q2?\n- Q3?\n- Brand new unanswered blocker A?\n- Brand new unanswered blocker B?\n', + ); + + const after = audit(tmpDir); + const afterItem = after.items.context_questions.find((i) => !i.scan_error && i.file === '02-CONTEXT.md'); + assert.ok(afterItem, 'AFTER replacing Q4 with new blockers: the item must RESURFACE — a slice(0,3) digest cannot see past position 3'); + assert.equal(afterItem.question_count, 5); + }); + + // ── SWEEP finding (#3458 follow-up review): the digest's element-join + // must be unambiguous — two DIFFERENT question sets must never encode to + // the same joined string and collide on the same digest ─────────────── + + test('SWEEP: two different open-question sets that collide under a naive separator-join must record DIFFERENT digests', () => { + const phase1Dir = planningPath('phases', '03-one'); + const phase2Dir = planningPath('phases', '04-two'); + fs.mkdirSync(phase1Dir, { recursive: true }); + fs.mkdirSync(phase2Dir, { recursive: true }); + const file1 = path.join(phase1Dir, '03-CONTEXT.md'); + const file2 = path.join(phase2Dir, '04-CONTEXT.md'); + + // Both sets embed a literal NUL codepoint (via the YAML `\x00` + // double-quoted hex escape) at the exact position needed to make the + // TWO DIFFERENT arrays below encode to the byte-identical string under + // ANY single-character-separator join (a plain space join, OR the + // separator this seam actually shipped with) — the general proof that + // NO fixed separator closes this class, only a length-prefixed, + // self-delimiting encoding does. + // Set 1: ["foo\0bar", "baz"] → "foo\0bar" + SEP + "baz" + // Set 2: ["foo", "bar\0baz"] → "foo" + SEP + "bar\0baz" + // For SEP = "\0" both concatenate to the identical "foo\0bar\0baz". + fs.writeFileSync(file1, '---\nopen_questions:\n - "foo\\x00bar"\n - "baz"\n---\n# Context\n'); + fs.writeFileSync(file2, '---\nopen_questions:\n - "foo"\n - "bar\\x00baz"\n---\n# Context\n'); + + const r1 = ack(tmpDir, ['--category', 'context_questions', '--phase', '03', '--file', '03-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + const r2 = ack(tmpDir, ['--category', 'context_questions', '--phase', '04', '--file', '04-CONTEXT.md', '--milestone', 'v1.0', '--at', '2026-08-15']); + assert.ok(r1.success, `stderr: ${r1.error}`); + assert.ok(r2.success, `stderr: ${r2.error}`); + + const digest1 = fs.readFileSync(file1, 'utf-8').match(/questions_digest:\s*([0-9a-f]{64})/)[1]; + const digest2 = fs.readFileSync(file2, 'utf-8').match(/questions_digest:\s*([0-9a-f]{64})/)[1]; + assert.notEqual(digest1, digest2, 'two DIFFERENT question sets must never record the same digest, even when they collide under a naive separator-join'); + }); + + test('WARNING-2 disproof: adding more pending scenarios to an acknowledged UAT gap (status unchanged) resurfaces the item', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-UAT.md'); + fs.writeFileSync(filePath, '---\nstatus: gaps_found\n---\n# UAT\n\n## Scenarios\n\n1. result: pending\n'); + + assert.ok(ack(tmpDir, ['--category', 'uat_gaps', '--phase', '01', '--file', '01-UAT.md', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + let after = audit(tmpDir); + assert.equal(after.counts.uat_gaps, 0, 'BEFORE: 1 pending scenario, suppressed'); + + // status: stays `gaps_found` — only the scenario count moves, 1 → 6. + fs.writeFileSync( + filePath, + '---\nstatus: gaps_found\n---\n# UAT\n\n## Scenarios\n\n1. result: pending\n2. result: pending\n3. result: pending\n4. result: pending\n5. result: pending\n6. result: pending\n', + ); + + after = audit(tmpDir); + assert.equal(after.counts.uat_gaps, 1, 'AFTER: same status, MORE pending scenarios — must RESURFACE'); + const item = after.items.uat_gaps.find((i) => !i.scan_error); + assert.equal(item.open_scenario_count, 6); + }); + + // ── WARNING 3 (#3458 follow-up review): the human report must carry the + // same "clean vs silenced" signal --json already did ────────────────── + + test('WARNING 3: human-readable report shows the acknowledged tally, per-category and in the all-clear footer', () => { + const phaseDir = planningPath('phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + const filePath = path.join(phaseDir, '01-UAT.md'); + fs.writeFileSync(filePath, '---\nstatus: gaps_found\n---\n# UAT\n\n## Gaps\n\n- truth: "x"\n status: open\n'); + assert.ok(ack(tmpDir, ['--category', 'uat_gaps', '--phase', '01', '--file', '01-UAT.md', '--milestone', 'v1.0', '--at', '2026-08-15']).success); + + // All-clear case: the only item left is a previously-acknowledged one. + const clearReport = runGsdTools(['audit-open'], tmpDir); + assert.ok(clearReport.success, `stderr: ${clearReport.error}`); + assert.match(clearReport.output, /1 previously acknowledged item/i, 'all-clear footer must disclose the suppressed item'); + + // Now add a genuinely NEW open item so has_open_items is true, and + // confirm the per-category line also discloses the acknowledged one + // still sitting alongside it. + const debugDir = planningPath('debug'); + fs.mkdirSync(debugDir, { recursive: true }); + fs.writeFileSync(path.join(debugDir, 'investigate.md'), '---\nstatus: open\n---\n## Current Focus\ndigging\n'); + + const openReport = runGsdTools(['audit-open'], tmpDir); + assert.ok(openReport.success, `stderr: ${openReport.error}`); + assert.match(openReport.output, /previously acknowledged item/i, 'footer must still disclose the acknowledged item while other items are open'); + }); + + // ── writer refuses a path outside the project ────────────────────────── + + test('writer refuses to acknowledge a path that escapes the project (path traversal)', () => { + const result = ack(tmpDir, ['--category', 'debug_sessions', '--slug', '../../../../etc/passwd', '--milestone', 'v1.0']); + assert.equal(result.success, false, 'a traversal-shaped --slug must be refused, not written'); + }); + + test('writer refuses to acknowledge a category with a required flag missing', () => { + const result = ack(tmpDir, ['--category', 'uat_gaps', '--milestone', 'v1.0']); // no --phase/--file + assert.equal(result.success, false, 'missing --phase/--file must be refused'); + }); + }); +} diff --git a/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json b/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json index 919db6e7d..a41a39949 100644 --- a/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json +++ b/tests/emitted-drift-acks/2962-zsh-nomatch-for-glob-portability.json @@ -3,7 +3,6 @@ "paths": { "gsd-integration-checker.md": "#2962: 1 bash block (line ~98 SUMMARY iteration) gained the nullglob shim for zsh portability of the for-glob loop.", "resume-project.md": "#2962: 1 bash block (line ~66 plans-without-summaries scan) gained the nullglob shim for zsh portability of the for-glob loop.", - "complete-milestone.md": "#2962: 1 bash block (line ~221 summary one-liner extraction) gained the nullglob shim for zsh portability of the for-glob loop.", "audit-milestone.md": "#2962: 1 bash block (line ~126 requirements_completed extraction) gained the nullglob shim for zsh portability of the for-glob loop." } } diff --git a/tests/emitted-drift-acks/3458-audit-open-acknowledge-wiring.json b/tests/emitted-drift-acks/3458-audit-open-acknowledge-wiring.json new file mode 100644 index 000000000..87c904bde --- /dev/null +++ b/tests/emitted-drift-acks/3458-audit-open-acknowledge-wiring.json @@ -0,0 +1,6 @@ +{ + "version": 1, + "paths": { + "complete-milestone.md": "#3458 follow-up: the `pre_close_artifact_audit` step's `[A]` branch previously told the model to hand-author a `## Deferred Items` markdown table with no writer, no schema, and no reader — the acknowledgment never actually suppressed anything at the next close. This wires the real `audit-open acknowledge` CLI writer that landed in src/audit.cts: step 2 now calls it once per open item, per category, using the identifiers the audit JSON already emits (including the quick_tasks `--dir` reconstruction from `date`+`slug`, and the phase-scoped `--archived-milestone` passthrough), before step 3 writes the STATE.md table as a disclosure record only. Also converges the STATE.md `## Deferred Items` table to the template's 5-column shape (adds `Milestone`) and updates the MILESTONES.md disclosure line to distinguish newly-acknowledged items from ones a prior close already suppressed. The growth (+6,764 bytes) is this new per-category acknowledgment loop plus the expanded disclosure/security prose — no unrelated content moved. #3458 follow-up review (round 2, BLOCKER 3): the `[A]` branch's acknowledge loops previously ran as `cmd | while read` pipelines with no failure tracking, so a refused acknowledge (`unsupported_heading_shape`, `ambiguous`, `not_found`, missing file) was silently discarded and the close proceeded anyway; separately, `AUDIT_JSON=$(gsd_run query audit-open --json)` never handled the `@file:` large-payload sentinel `io.output` swaps in past 50000 chars, so every `jq` read against it would silently no-op every loop body. This round switches every `cmd | while read` to `while read; do …; done < <(cmd)` (process substitution, so the loop body runs in the CURRENT shell and can mutate a counter that survives it) plus an `ACK_FAILURES` accumulator that HALTS the step before close if any acknowledge call failed, and adds the same `@file:` sentinel handling `verify_readiness`'s `INIT_MANAGER` already uses. Additional growth: +2,433 bytes for the failure-accumulation wrapper and the `@file:` guard." + } +} diff --git a/tests/security.test.cjs b/tests/security.test.cjs index d7386e963..7c64c66a5 100644 --- a/tests/security.test.cjs +++ b/tests/security.test.cjs @@ -17,6 +17,7 @@ const { scanForInjection, sanitizeForPrompt, sanitizeForDisplay, + sanitizeLabel, safeJsonParse, validatePhaseNumber, validateFieldName, @@ -445,6 +446,51 @@ describe('sanitizeForDisplay', () => { }); }); +describe('sanitizeLabel', () => { + test('escapes CR/LF so a single-line label cannot forge a new report line', () => { + const input = 'zz\n0 open items require decisions.\n\x1b[2K\x1b[1G FORGED'; + const result = sanitizeLabel(input); + assert.ok(!result.includes('\n'), 'no raw newline survives'); + assert.ok(!result.includes('\r'), 'no raw carriage return survives'); + assert.equal( + result, + 'zz\\n0 open items require decisions.\\n\\x1b[2K\\x1b[1G FORGED', + ); + }); + + test('escapes ESC/ANSI control bytes visibly rather than stripping them', () => { + const input = '\x1b[31mred\x1b[0m'; + const result = sanitizeLabel(input); + assert.ok(!result.includes('\x1b'), 'no raw ESC byte survives'); + assert.equal(result, '\\x1b[31mred\\x1b[0m'); + }); + + test('escapes DEL and C1 control range', () => { + assert.equal(sanitizeLabel('a\x7fb'), 'a\\x7fb'); + assert.equal(sanitizeLabel('a\x9fb'), 'a\\x9fb'); + }); + + test('ordinary printable input passes through byte-identical', () => { + const input = '03-alpha-and-omega (v1.2)'; + assert.equal(sanitizeLabel(input), input); + }); + + test('non-string / empty input passes through unchanged', () => { + assert.equal(sanitizeLabel(undefined), undefined); + assert.equal(sanitizeLabel(''), ''); + }); + + test('differs from sanitizeForDisplay on multi-line input — documents why both exist', () => { + // sanitizeForDisplay's job is preserving newlines between legitimate + // prose lines while dropping whole protocol-leak lines; sanitizeLabel's + // job is refusing to let ANY newline survive in a single-line label. + const input = 'Visible line\nAnother line'; + assert.equal(sanitizeForDisplay(input), input); // newline preserved + assert.equal(sanitizeLabel(input), 'Visible line\\nAnother line'); // newline escaped + assert.notEqual(sanitizeForDisplay(input), sanitizeLabel(input)); + }); +}); + // ─── Shell Safety ─────────────────────────────────────────────────────────── describe('validateShellArg', () => {