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', () => {