* 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<X.Y>-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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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: zz<newline>0 open items require decisions.<newline><ESC>[2K<ESC>[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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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:<path>` 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/jolly-voles-rally.md
Normal file
5
.changeset/jolly-voles-rally.md
Normal file
@@ -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)
|
||||
5
.changeset/rapid-tunas-dance.md
Normal file
5
.changeset/rapid-tunas-dance.md
Normal file
@@ -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 <cat> --milestone <ver> [--at <date>] ...` 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)
|
||||
@@ -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 <category> --milestone <version> [--at <date>] <identifier flags…>
|
||||
|
||||
# Reverse-migrate a GSD-2 project into the current structure (backs `/gsd-import --from-gsd2`)
|
||||
node gsd-tools.cjs from-gsd2 [--path <dir>] [--force] [--dry-run]
|
||||
|
||||
|
||||
@@ -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 <category> --milestone <version> [--at <YYYY-MM-DD>] <identifier flags…>
|
||||
```
|
||||
|
||||
`--category` and `--milestone` are always required; `--at` defaults to today. The identifier flags depend on `--category`:
|
||||
|
||||
| `--category` | Identifier flags |
|
||||
|---------------|-------------------|
|
||||
| `debug_sessions` | `--slug <slug>` |
|
||||
| `threads` | `--slug <slug>` |
|
||||
| `seeds` | `--seed-id <id>` |
|
||||
| `todos` | `--filename <file>` |
|
||||
| `quick_tasks` | `--dir <dir>` (the `.planning/quick/<dir>/` 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 <phase> --file <file>` [`--archived-milestone <version>`] |
|
||||
| `deferred_items` | `--phase <phase> --file <file> --text <exact bullet text>` [`--archived-milestone <version>`] |
|
||||
|
||||
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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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:<path>` 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/<dir>/ 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.
|
||||
</step>
|
||||
|
||||
<step name="verify_readiness">
|
||||
|
||||
@@ -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'])],
|
||||
|
||||
@@ -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/<task>/`) 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/<task>/`) 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'])],
|
||||
|
||||
@@ -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,
|
||||
|
||||
1175
src/audit.cts
1175
src/audit.cts
File diff suppressed because it is too large
Load Diff
@@ -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),
|
||||
|
||||
@@ -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<number, string> = {
|
||||
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 ───────────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
|
||||
315
src/uat.cts
315
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,
|
||||
};
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -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."
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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:<path>` 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."
|
||||
}
|
||||
}
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user