From 472f585f7ccdb519fb8728cd41936afca947c403 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Sat, 29 Aug 2026 16:00:45 -0500 Subject: [PATCH] fix(#3726)!: require --confirm before milestone complete mutates (#3774) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3726): require --confirm before milestone complete mutates `milestone complete ` is a one-way door — ROADMAP.md and REQUIREMENTS.md archived, every phase directory in the milestone MOVED, STATE.md rewritten — and ran unconditionally on first invocation through every invocation path, including `query milestone.complete `, whose `query` meta-prefix reads as a read-only namespace but performs no filtering (#167's invocation-compatibility shim + #3243's dotted-form normalization). The gate lives on the destructive command itself, not on the `query` prefix (the prefix is an intentional invocation mechanism, not a permission boundary — restricting it would break dozens of shipped workflow callers). Without --confirm and without --dry-run the command now refuses via error() before reading anything beyond its arg checks, so an unconfirmed invocation is a guaranteed no-op on disk. --dry-run still previews with no confirmation needed and is now documented in the usage block (it was only documented for the sibling archive-quick). --force keeps its narrow meaning — bypassing the TRUNCATED-scope and unstarted-phase guards — and does not double as the mutation opt-in. --confirm follows the existing `phases clear --confirm` idiom in the same module. complete-milestone.md's two invocations pass --confirm (the workflow has gathered explicit user intent by that step). Existing tests get --confirm appended — pre-change behavior is exactly confirmed behavior — and a #3726 regression block covers: refusal + full-tree byte-identity on both invocation forms, --force not satisfying the gate, --dry-run still passing without confirmation, and --confirm proceeding. The refusal tests fail against pre-fix code (negative control run). Fixes #3726 * docs(#3726): document the --confirm requirement in CLI-TOOLS and COMMANDS Cross-AI review of the fix diff (codex, pre-create) caught three shipped doc sites still instructing the now-refused bare invocation: the CLI-TOOLS.md milestone-complete synopsis + flag table, and COMMANDS.md's two guard-override instructions (`--force` alone now refuses without --confirm). Localized CLI-TOOLS copies already lag the English synopsis (no --force/--dry-run either) and follow the translation pipeline, not this fix. * chore(#3726): set changeset fragment pr to 3774 * test(#3726): confirm-gate CI repairs — QA scenario caller + growth ack Two CI reds from the --confirm gate, both this branch's own misses: - tests/qa/scenarios/milestone-rollover.json invoked `milestone complete 1.0 --force` as a JSON arg-array fixture — a caller shape the test sweep (which grepped runGsdTools/runSdkQuery in tests/*.cjs) never enumerated. Adds --confirm; the scenario's boundary-crossing contract is otherwise untouched. - complete-milestone.md's +420-byte --confirm note trips the emitted-attribution growth ratchet. Acknowledged as a #3726 append to the existing complete-milestone.md entry in 3409-unreachable-guard-arms.json (two ack sources may never name the same path, per that fragment's own precedent). Local: lint-emitted-drift-ack ok; loop-walk.qa 115/115 green sandboxed. * docs(#3726): CLI-TOOLS.md guard-override sentences say --force --confirm Review Major 1: the truncated-window and unstarted-phase guard paragraphs still told the reader to "Pass `--force` to override", which now refuses (--force alone does not satisfy the confirmation gate), while the flag table 470 lines later said the opposite. Mirror the docs/COMMANDS.md pair so the file no longer contradicts itself. * docs(#3726): synopsis renders --confirm and --dry-run as alternatives Review Nit 1: `milestone complete --confirm [--dry-run]` read as "a dry run still needs --confirm", the opposite of AC 3. Render the pair as `(--confirm | --dry-run)` in the CLI-TOOLS.md synopsis and the usage docblock, and let the flag rows carry the rule. * test(#3726): pass --confirm in base-added milestone fixtures; re-file the growth ack Rebase onto next (26 commits) surfaced three tests the gate now refuses: the #3685 write-flag contract pair in tests/milestone.test.cjs and the `milestone complete` boundary fixture in tests/state-contract.test.cjs all invoke the command bare. Each now passes --confirm (a mutating run is exactly what they assert on). The +420 byte complete-milestone.md growth ack rode on 3409-unreachable-guard-arms.json, which #3078 swept from next as fully spent — hence the modify/delete conflict. Re-filed under a fresh fragment named for this issue, never resurrecting the swept one. * test(#3726): pin the present-but-falsy arm of the confirmation gate Review Minor 1: the boundary triple covered absent and present but not present-but-falsy. The gate is an exact-token match, so --confirm=false and --confirm=0 refuse today — pinned (canonical + query forms, whole .planning/ tree byte-identical) so a future `=`-aware or prefix-matching parser cannot silently turn --confirm=false into a confirmed run of an irreversible command. * test(#3726): drop --confirm from dry-run-only invocations Review Nit 2: --confirm was mass-appended to 14 pre-existing --dry-run invocations that never needed it, so each stopped standing as incidental proof that a preview needs no confirmation. Reverted to the pre-PR form; the dedicated AC-3 test carries the explicit assertion. * docs(#3726): sync the localized CLI-TOOLS synopsis with the confirm gate REQ-I18N-02 (docs/features/internationalized-documentation.md) requires translations to stay synchronized with the English source. The four localized CLI-TOOLS.md guides still advertised a bare `milestone complete `, which now exits 1. Render the English synopsis verbatim — `(--confirm | --dry-run)` plus the `[--force]` and `[--archive-quick]` flags the translations had also fallen behind on. * test(#3726): drop --confirm from the remaining preview-only invocations Round 2 reverted the --confirm appends on --dry-run-only invocations in tests/milestone.test.cjs, but four more sat in two files the sweep missed: tests/milestone-archive.test.cjs (three) and tests/milestone-window-single-owner.test.cjs (one). Each is a preview run whose whole purpose is to document that a preview mutates nothing, so `--dry-run ... --confirm` contradicted the semantics the test exists to pin. Dropping the token restores each as incidental proof that a preview needs no confirmation; the dedicated AC-3 test keeps the explicit assertion. No assertion added, relaxed, or removed — the change is four tokens. * chore(#3726): migrate the emitted-drift ack from a fragment to a commit trailer #3954 (ADR-3942) moved emitted-drift acknowledgments out of tests/emitted-drift-acks/ and into git commit trailers, and the fragment directory no longer exists on next. The reason this PR's fragment carried moves verbatim into the Emitted-Drift-Ack-Growth trailer on this commit; the fragment file is removed rather than resurrected. Emitted-Drift-Ack-Growth: complete-milestone.md — #3726: +420 bytes (40186 -> 40606). The archive_milestone step's two `milestone complete` invocations now pass the required --confirm flag (the command refuses to mutate without it — the archive is irreversible), with a note explaining the flag and pointing at --dry-run for previews. Deliberate runtime-loaded workflow text for the new gate, not converter drift. * fix(#3726): name --confirm in the version-required refusal The documented arg-discovery path (gsd-tools.cjs top-level usage: invoke the command without args and the error lists what is required) stopped at `version required for milestone complete (e.g., v1.0)` — one required argument short. Discovering --confirm took a second round trip through the gate. The refusal now reads `… — and --confirm to mutate`, pinned by a test that also asserts the version-less invocation leaves .planning/ untouched. * test(#3726): pin the milestone complete docs against a silent regression The changeset is `type: Fixed`, which the docs-required lint exempts, so nothing in CI would notice a later edit that reinstated the bare-`--force` override prose or dropped `--confirm` from the synopsis. Four tests in tests/milestone.test.cjs now pin: the synopsis line in docs/CLI-TOOLS.md and its four localized mirrors; the `--confirm` flag row; both guard-override instructions in docs/CLI-TOOLS.md and docs/COMMANDS.md, by guard name (a substring match on each instruction's `--force --confirm` text); and — as an identity ratchet over the milestone-complete sections — every `--force` sentence or clause that lacks `--confirm`, so a new bare instruction in its own sentence or clause fails whatever its wording. Named residual: a bare instruction spliced into the same clause as a compliant one coalesces with it and passes the ratchet; the by-name pins are what keep the four known instructions from losing the pairing that way. The file is registered in scripts/docs-guard-registry.cjs so the pin runs on the PR that changes those docs, not only after merge. --------- Co-authored-by: CI Rebase Check Co-authored-by: Tom Boucher --- .changeset/calm-otters-refuse.md | 5 + docs/CLI-TOOLS.md | 10 +- docs/COMMANDS.md | 4 +- docs/ja-JP/CLI-TOOLS.md | 2 +- docs/ko-KR/CLI-TOOLS.md | 2 +- docs/pt-BR/CLI-TOOLS.md | 2 +- docs/zh-CN/CLI-TOOLS.md | 2 +- gsd-core/bin/gsd-tools.cjs | 13 +- gsd-core/workflows/complete-milestone.md | 9 +- scripts/docs-guard-registry.cjs | 12 + src/milestone.cts | 32 +- tests/milestone-archive.test.cjs | 50 +-- tests/milestone-window-single-owner.test.cjs | 8 +- tests/milestone.test.cjs | 321 ++++++++++++++++--- tests/new-milestone-clear-phases.test.cjs | 4 +- tests/phase-resolution-parity.test.cjs | 2 +- tests/qa/scenarios/milestone-rollover.json | 2 +- tests/state-contract.test.cjs | 5 +- tests/state.test.cjs | 2 +- 19 files changed, 400 insertions(+), 87 deletions(-) create mode 100644 .changeset/calm-otters-refuse.md diff --git a/.changeset/calm-otters-refuse.md b/.changeset/calm-otters-refuse.md new file mode 100644 index 000000000..b3e0dffeb --- /dev/null +++ b/.changeset/calm-otters-refuse.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3774 +--- +**`milestone complete` now requires an explicit `--confirm` to mutate** — the command irreversibly archives ROADMAP.md/REQUIREMENTS.md, MOVES every phase directory in the milestone, and rewrites STATE.md, yet ran unconditionally on first invocation through every invocation path, including `query milestone.complete `, whose `query` meta-prefix reads as a read-only namespace but performs no filtering. Without `--confirm` (and without `--dry-run`) the command now refuses before touching anything and names the flag that proceeds; `--dry-run` still previews the exact move list with no confirmation needed, and is now documented in the command's own usage block. `--force` keeps its narrow meaning (bypass the TRUNCATED-scope / unstarted-phase guards) and does not double as the mutation opt-in. The `/gsd-complete-milestone` workflow passes `--confirm` at its archive step. (#3726) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 228b037a1..c1eb43893 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -393,8 +393,9 @@ elsewhere — phase scoping cannot be trusted, and the command now refuses rather than falling back to an over-inclusive filter that would archive every phase directory in the project. `unreadable` (no ROADMAP.md at all) and `unscoped` (no section for this version) are pre-existing, legitimately -handled states and are not refused here. Pass `--force` to override, the same -affordance the unstarted-phase guard uses. +handled states and are not refused here. Pass `--force --confirm` to override, the same +affordance the unstarted-phase guard uses (`--confirm` is required for any mutating run — +#3726; `--force` alone does not imply it). --- @@ -881,7 +882,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ```bash # Archive milestone -node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] [--force] [--dry-run] [--archive-quick] +node gsd-tools.cjs milestone complete (--confirm | --dry-run) [--name ] [--no-archive-phases] [--force] [--archive-quick] # Archive .planning/quick/* into milestones/-quick/ WITHOUT the milestone complete close-out (#2142) node gsd-tools.cjs milestone archive-quick [--dry-run] @@ -896,13 +897,14 @@ node gsd-tools.cjs requirements mark-complete | Flag | Description | |------|-------------| | `` | Milestone version label to archive (e.g. `v1.0`). | +| `--confirm` | **Required to mutate (#3726).** The archive is irreversible — ROADMAP.md/REQUIREMENTS.md archived, phase directories MOVED, STATE.md rewritten — so without this flag the command refuses and changes nothing (exit 1, with a message naming both `--confirm` and `--dry-run`). Not implied by `--force`, which only overrides the guards below. | | `--name ` | Display name for the MILESTONES.md entry. Defaults to ``. | | `--no-archive-phases` | Leave phase directories in place instead of moving them into `.planning/milestones/-phases/`. | | `--archive-quick` | Opt-in (default OFF, #2142): also move every directory under `.planning/quick/` into `.planning/milestones/-quick/`, (re)write that archive directory's `README.md` index, and clear STATE.md's `### Quick Tasks Completed` table rows. See "`milestone archive-quick`" below for the narrower standalone form and the full behavior. | | `--force` | Override the unstarted-phase guard (see below). | | `--dry-run` | Print the archive plan (roadmap, requirements, phases, and — when `--archive-quick` is also passed — quick-task dirs to move) without mutating anything. | -**Unstarted-phase guard.** Before archiving, the command scans the ROADMAP scoped for `` and refuses if any `### Phase N:` heading in that slice has no matching phase directory on disk (`disk_status: no_directory`). Phase 0 (pre-milestone) and Phase 999 (backlog) sentinels are excluded. The guard runs whenever `--force` is absent, independent of `STATE.md`'s `milestone:` field — if that field is present but does not match ``, a WARNING naming both values is emitted to stderr and the scan still runs (#2946). Pass `--force` to override. +**Unstarted-phase guard.** Before archiving, the command scans the ROADMAP scoped for `` and refuses if any `### Phase N:` heading in that slice has no matching phase directory on disk (`disk_status: no_directory`). Phase 0 (pre-milestone) and Phase 999 (backlog) sentinels are excluded. The guard runs whenever `--force` is absent, independent of `STATE.md`'s `milestone:` field — if that field is present but does not match ``, a WARNING naming both values is emitted to stderr and the scan still runs (#2946). Pass `--force --confirm` to override (`--confirm` is required for any mutating run — #3726; `--force` alone does not imply it). **Sentinel directories are never archived.** The phase-directory move performed when `--no-archive-phases` is absent is now filtered through the same canonical sentinel predicate as `phases list` and `phases clear`: `999.*` (backlog) and `0-*` (pre-milestone) directories are left in place rather than moved into `.planning/milestones/-phases/`. Previously this path was scoped only by the milestone window, with no sentinel filter, so a sentinel directory sitting inside the window could be archived along with the milestone's real phases. diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 41ddaa3b8..4eeff01db 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -510,9 +510,9 @@ The marker never overwrites the artifact's own `status:` field for the eight fro > **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. -> **Truncated-window guard.** Archiving also refuses when the milestone's ROADMAP window is truncated — `Cannot mark milestone complete: the ROADMAP window for "" is truncated`. This is the case where the milestone's heading is found but its section closes before reaching the roadmap's `### Phase N:` region (typically a closed-milestone heading sitting in between), which previously degraded to an over-inclusive filter and archived *every* phase directory in the project rather than the milestone's own. An unreadable ROADMAP.md or a version with no matching section at all are pre-existing, legitimately-handled states and are not refused here. Same override as below: `gsd-tools milestone complete --force`. A window that is genuinely empty — a freshly-declared milestone with no phases yet — is *not* affected and still completes normally. +> **Truncated-window guard.** Archiving also refuses when the milestone's ROADMAP window is truncated — `Cannot mark milestone complete: the ROADMAP window for "" is truncated`. This is the case where the milestone's heading is found but its section closes before reaching the roadmap's `### Phase N:` region (typically a closed-milestone heading sitting in between), which previously degraded to an over-inclusive filter and archived *every* phase directory in the project rather than the milestone's own. An unreadable ROADMAP.md or a version with no matching section at all are pre-existing, legitimately-handled states and are not refused here. Same override as below: `gsd-tools milestone complete --force --confirm` (#3726: `--confirm` is required for any mutating run; `--force` alone does not imply it). A window that is genuinely empty — a freshly-declared milestone with no phases yet — is *not* affected and still completes normally. -> **Unstarted-phase guard.** Archiving refuses if the milestone's ROADMAP still lists a phase with no phase directory on disk — `Cannot mark milestone complete: ROADMAP lists N unstarted phase(s)`. If a phase was intentionally deferred or merged without a directory, run `gsd-tools milestone complete --force` (the `/gsd-complete-milestone` workflow runs the underlying command without `--force`, so use the CLI directly to override). A `STATE.md` `milestone:` value that does not match `` prints a WARNING and still runs the guard (#2946). +> **Unstarted-phase guard.** Archiving refuses if the milestone's ROADMAP still lists a phase with no phase directory on disk — `Cannot mark milestone complete: ROADMAP lists N unstarted phase(s)`. If a phase was intentionally deferred or merged without a directory, run `gsd-tools milestone complete --force --confirm` (the `/gsd-complete-milestone` workflow runs the underlying command without `--force`, so use the CLI directly to override; `--confirm` is required for any mutating run — #3726). A `STATE.md` `milestone:` value that does not match `` prints a WARNING and still runs the guard (#2946). > **Sentinel directories stay put.** Moving phase directories into the archive (the default, unless `--no-archive-phases` is passed) now excludes `999.*` (backlog) and `0-*` (pre-milestone) directories via the same sentinel predicate the unstarted-phase guard already uses. Previously the archive move was scoped only by the milestone window, so a sentinel directory sitting inside that window could be archived along with the milestone's own phases. diff --git a/docs/ja-JP/CLI-TOOLS.md b/docs/ja-JP/CLI-TOOLS.md index 46504c52f..59d640da2 100644 --- a/docs/ja-JP/CLI-TOOLS.md +++ b/docs/ja-JP/CLI-TOOLS.md @@ -323,7 +323,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ```bash # マイルストーンをアーカイブ -node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] +node gsd-tools.cjs milestone complete (--confirm | --dry-run) [--name ] [--no-archive-phases] [--force] [--archive-quick] # 要件を完了としてマーク node gsd-tools.cjs requirements mark-complete diff --git a/docs/ko-KR/CLI-TOOLS.md b/docs/ko-KR/CLI-TOOLS.md index d272bd4be..d3c57aa96 100644 --- a/docs/ko-KR/CLI-TOOLS.md +++ b/docs/ko-KR/CLI-TOOLS.md @@ -323,7 +323,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ```bash # 마일스톤 아카이브 -node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] +node gsd-tools.cjs milestone complete (--confirm | --dry-run) [--name ] [--no-archive-phases] [--force] [--archive-quick] # 요구사항을 완료로 표시 node gsd-tools.cjs requirements mark-complete diff --git a/docs/pt-BR/CLI-TOOLS.md b/docs/pt-BR/CLI-TOOLS.md index 928e66f8b..bcfaf0192 100644 --- a/docs/pt-BR/CLI-TOOLS.md +++ b/docs/pt-BR/CLI-TOOLS.md @@ -325,7 +325,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ```bash # Arquiva milestone -node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] +node gsd-tools.cjs milestone complete (--confirm | --dry-run) [--name ] [--no-archive-phases] [--force] [--archive-quick] # Marca requisitos como concluídos node gsd-tools.cjs requirements mark-complete diff --git a/docs/zh-CN/CLI-TOOLS.md b/docs/zh-CN/CLI-TOOLS.md index 447a499a0..31903097d 100644 --- a/docs/zh-CN/CLI-TOOLS.md +++ b/docs/zh-CN/CLI-TOOLS.md @@ -323,7 +323,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ```bash # 归档里程碑 -node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] +node gsd-tools.cjs milestone complete (--confirm | --dry-run) [--name ] [--no-archive-phases] [--force] [--archive-quick] # 将需求标记为完成 node gsd-tools.cjs requirements mark-complete diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index fd79997d9..1f7e41f55 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -68,7 +68,12 @@ * gaps_found-only, never call on the pass path * * Milestone Operations: - * milestone complete Archive milestone, create MILESTONES.md + * milestone complete (--confirm | --dry-run) + * Archive milestone, create MILESTONES.md — one of the two is required + * --confirm REQUIRED to mutate (#3726): the archive is irreversible (ROADMAP/ + * REQUIREMENTS archived, phase dirs MOVED, STATE.md rewritten), so + * without this flag the command refuses and mutates nothing + * --dry-run Preview what would move, mutates nothing (no --confirm needed; #2118) * [--name ] * [--no-archive-phases] Skip moving phase dirs to milestones/vX.Y-phases/ (archived by default) * [--archive-quick] Move .planning/quick/* dirs to milestones/vX.Y-quick/ + reset the @@ -2521,7 +2526,11 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load // --no-archive-phases' inverted shape, absence of this flag means // "do nothing" rather than "skip a default-on behavior". const archiveQuick = args.includes('--archive-quick'); - milestone.cmdMilestoneComplete(cwd, args[2], { name: milestoneName, archivePhases, force, dryRun, archiveQuick }, raw); + // #3726: explicit mutation opt-in — without --confirm (and without + // --dry-run) the command refuses before touching anything. Distinct + // from --force, which bypasses the narrow scope guards only. + const confirm = args.includes('--confirm'); + milestone.cmdMilestoneComplete(cwd, args[2], { name: milestoneName, archivePhases, force, dryRun, archiveQuick, confirm }, raw); } else if (subcommand === 'archive-quick') { // #2142 escalation: narrow archival-only entry point (does NOT // touch ROADMAP/REQUIREMENTS/MILESTONES.md, runs no completion diff --git a/gsd-core/workflows/complete-milestone.md b/gsd-core/workflows/complete-milestone.md index d23a29066..03dcfc298 100644 --- a/gsd-core/workflows/complete-milestone.md +++ b/gsd-core/workflows/complete-milestone.md @@ -520,9 +520,14 @@ If "Yes": set `ARCHIVE_QUICK_FLAG="--archive-quick"`. If "Skip" (or `.planning/q **Delegate archival to `gsd_run query milestone.complete`:** ```bash -ARCHIVE=$(gsd_run query milestone.complete "v[X.Y]" --name "[Milestone Name]" $ARCHIVE_QUICK_FLAG) +ARCHIVE=$(gsd_run query milestone.complete "v[X.Y]" --name "[Milestone Name]" --confirm $ARCHIVE_QUICK_FLAG) ``` +`--confirm` is required (#3726): the archive is irreversible (ROADMAP/REQUIREMENTS archived, phase +directories MOVED, STATE.md rewritten), so `milestone complete` refuses to mutate without it. This +workflow has already gathered the user's explicit intent by this step, so passing the flag here is +correct; `--dry-run` previews the exact move list without mutating if a preview is ever needed first. + The CLI handles: - Creating `.planning/milestones/` directory - Archiving ROADMAP.md to `milestones/v[X.Y]-ROADMAP.md` @@ -545,7 +550,7 @@ Verify after `--archive-quick` was passed: `✅ Quick tasks archived to .plannin If the user explicitly wants to keep phase directories in place as raw execution history, invoke `milestone complete` with `--no-archive-phases`: ```bash -gsd_run query milestone complete v[X.Y] --no-archive-phases +gsd_run query milestone complete v[X.Y] --no-archive-phases --confirm ``` Verify after a default (archived) completion: `✅ Phase directories archived to .planning/milestones/v[X.Y]-phases/` diff --git a/scripts/docs-guard-registry.cjs b/scripts/docs-guard-registry.cjs index 2f8d9abb1..c029fa2bc 100644 --- a/scripts/docs-guard-registry.cjs +++ b/scripts/docs-guard-registry.cjs @@ -286,6 +286,18 @@ const DOCS_GUARD_TESTS = { 'tests/inventory-manifest-sync.test.cjs': ['docs/INVENTORY.md', 'docs/INVENTORY-MANIFEST.json'], 'tests/kilo-upgrades.test.cjs': ['docs/how-to/connect-gsd-mcp-server.md'], 'tests/live-config-guard.test.cjs': ['docs/TESTING-SUITES.md'], + // #3726: pins the `milestone complete` synopsis (English + four localized + // mirrors), the `--confirm` flag row and the guard-override instructions — + // `type: Fixed` exempts that PR from the docs-required lint, so this is the + // only gate on that prose. + 'tests/milestone.test.cjs': [ + 'docs/CLI-TOOLS.md', + 'docs/COMMANDS.md', + 'docs/ja-JP/CLI-TOOLS.md', + 'docs/ko-KR/CLI-TOOLS.md', + 'docs/pt-BR/CLI-TOOLS.md', + 'docs/zh-CN/CLI-TOOLS.md', + ], 'tests/model-catalog-runtime-defaults.test.cjs': ['docs/CONFIGURATION.md'], // Scans every git-tracked file in the whole repo via `git ls-files` // (no-pending-3212-markers.test.cjs:37-46), which includes all of docs/ — diff --git a/src/milestone.cts b/src/milestone.cts index ac6cc8d5e..685111744 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -82,6 +82,16 @@ interface MilestoneCompleteOptions { force?: boolean; archivePhases?: boolean; dryRun?: boolean; + // #3726: explicit mutation opt-in. `milestone complete` is a one-way door + // (ROADMAP/REQUIREMENTS archived, every phase directory in the milestone + // MOVED, STATE.md rewritten) that used to run unconditionally on first + // invocation — including through the `query` meta-prefix, whose name reads + // as a read. Without --confirm (and without --dry-run) the command now + // refuses before any mutation. Deliberately NOT folded into `force`: + // --force bypasses the narrow TRUNCATED-scope and unstarted-phase guards + // and must keep exactly that meaning (#3726 AC 4) — intent-to-mutate and + // intent-to-override-a-guard are different declarations. + confirm?: boolean; // #2142: opt-in quick-task archival. Default OFF (unlike archivePhases, // which is default-ON since #1871) — acceptance criterion 1 is explicit // that Skip/absent must preserve today's behavior. Do NOT mirror @@ -554,7 +564,7 @@ function applyQuickTasksReset(content: string): { content: string; warning: { fi function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCompleteOptions, raw: boolean): void { if (!version) { - error('version required for milestone complete (e.g., v1.0)'); + error('version required for milestone complete (e.g., v1.0) — and --confirm to mutate'); } // #2288 security: `version` is a CLI positional that is interpolated into // multiple filesystem sinks below — `path.join(archiveDir, `${version}-ROADMAP.md`)`, @@ -566,6 +576,26 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo error(`milestone complete: version "${version}" is invalid — a milestone version label may contain only letters, digits, '.', '-' and '_', and must not contain path separators or "..".`); } + // #3726: confirmation gate — refuse before ANY read of the tree beyond the + // arg checks above, so an unconfirmed invocation is a guaranteed no-op on + // disk. The threat model is the NEVER_VALID_FLAGS one (gsd-tools.cjs): a + // caller supplies a token it believes is inert and a destructive operation + // proceeds unchecked — here the caller-side belief was that the `query` + // meta-prefix implies a read, and `query milestone.complete ` archived + // the milestone with no confirmation. The prefix is an intentional + // invocation-compatibility mechanism, not a permission boundary, so the + // gate lives on the destructive command itself and covers every invocation + // path. --dry-run needs no confirmation (it mutates nothing and is the + // recommended first step); --force does NOT imply it (see + // MilestoneCompleteOptions.confirm). + if (!options.dryRun && !options.confirm) { + error( + `milestone complete is irreversible: it archives ROADMAP.md and REQUIREMENTS.md, MOVES every phase ` + + `directory for ${version} into .planning/milestones/, and rewrites STATE.md. ` + + `Nothing has been changed. Re-run with --confirm to proceed, or --dry-run to preview exactly what would move.`, + ); + } + const roadmapPath = planningPaths(cwd).roadmap; const reqPath = planningPaths(cwd).requirements; const statePath = planningPaths(cwd).state; diff --git a/tests/milestone-archive.test.cjs b/tests/milestone-archive.test.cjs index 51bce9a5a..976e82d58 100644 --- a/tests/milestone-archive.test.cjs +++ b/tests/milestone-archive.test.cjs @@ -49,7 +49,7 @@ describe('bug #2684: milestone.complete forwards version to phases.archive', () ); fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); - const result = runSdkQuery(['milestone.complete', 'v1.0'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete should succeed, got error: ${result.error}`); assert.ok( !result.error || !result.error.includes('version required'), @@ -67,7 +67,7 @@ describe('bug #2684: milestone.complete forwards version to phases.archive', () // the guard, so give Phase 1 a real directory so the scan is satisfied. fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); - const result = runSdkQuery(['milestone.complete', 'v2.5'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v2.5', '--confirm'], tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); assert.strictEqual(result.data.version, 'v2.5'); }); @@ -82,7 +82,7 @@ describe('bug #2684: milestone.complete forwards version to phases.archive', () fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan'); fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-phases'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-phases', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete --archive-phases failed: ${result.error}`); assert.strictEqual(result.data.version, 'v1.0'); assert.ok(result.data.archived.phases === true, 'phases should be archived'); @@ -580,7 +580,7 @@ describe('#2142: quick task archival at milestone close-out', () => { }); fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), quickTasksStateWithRows(1)); - const result = runSdkQuery(['milestone.complete', 'v1.0'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, false, 'archived.quick must be false when --archive-quick is absent'); assert.ok(fs.existsSync(quickDir), 'quick task directory must remain in place'); @@ -601,7 +601,7 @@ describe('#2142: quick task archival at milestone close-out', () => { for (const name of names) writeQuickTaskDir(tmpDir, name); fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), quickTasksStateWithRows(3)); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete --archive-quick failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, true); @@ -631,7 +631,7 @@ describe('#2142: quick task archival at milestone close-out', () => { test('noOpsWhenQuickDirectoryAbsent', () => { setupQuickArchiveRoadmap(tmpDir); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, false); assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'))); @@ -641,7 +641,7 @@ describe('#2142: quick task archival at milestone close-out', () => { setupQuickArchiveRoadmap(tmpDir); fs.mkdirSync(path.join(tmpDir, '.planning', 'quick'), { recursive: true }); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, false, 'boundary 0: an empty quick dir must not count as archived'); assert.ok( @@ -654,7 +654,7 @@ describe('#2142: quick task archival at milestone close-out', () => { setupQuickArchiveRoadmap(tmpDir); writeQuickTaskDir(tmpDir, '2026-02-01-only-one'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, true, 'boundary 1: a single quick task dir must archive'); assert.ok(fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick', '2026-02-01-only-one'))); @@ -665,7 +665,7 @@ describe('#2142: quick task archival at milestone close-out', () => { writeQuickTaskDir(tmpDir, '2026-02-01-first'); writeQuickTaskDir(tmpDir, '2026-02-02-second'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, true, 'boundary 2: multiple quick task dirs must archive'); const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'); @@ -678,7 +678,7 @@ describe('#2142: quick task archival at milestone close-out', () => { writeQuickTaskDir(tmpDir, '2026-03-01-no-section'); fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# STATE\n\n### Blockers/Concerns\nNone\n'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete must succeed even without a Quick Tasks Completed section: ${result.error}`); assert.strictEqual(result.data.archived.quick, true); assert.ok(fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick', '2026-03-01-no-section'))); @@ -709,7 +709,7 @@ describe('#2142: quick task archival at milestone close-out', () => { ].join('\n'); fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), nonCanonicalState); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete must succeed even when the reset is refused: ${result.error}`); // The quick directories still move — only the STATE.md table reset is refused. assert.strictEqual(result.data.archived.quick, true); @@ -741,7 +741,7 @@ describe('#2142: quick task archival at milestone close-out', () => { fs.mkdirSync(path.join(archiveDir, name), { recursive: true }); fs.writeFileSync(path.join(archiveDir, name, 'existing-marker.txt'), 'prior run\n'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.ok(fs.existsSync(path.join(archiveDir, name, 'existing-marker.txt')), 'prior archive entry must survive'); assert.strictEqual( @@ -761,7 +761,7 @@ describe('#2142: quick task archival at milestone close-out', () => { const name = '2026-05-01-per-task-summary'; writeQuickTaskDir(tmpDir, name, { [`${name}-SUMMARY.md`]: '# Summary\nDid the thing.\n' }); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'); assert.ok(fs.statSync(path.join(archiveDir, 'README.md')).isFile(), 'README.md index must be generated'); @@ -788,7 +788,7 @@ describe('#2142: quick task archival at milestone close-out', () => { const name = '2026-05-02-bare-summary'; writeQuickTaskDir(tmpDir, name, { 'SUMMARY.md': '# Summary\nDid the other thing.\n' }); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'); assert.ok(fs.statSync(path.join(archiveDir, 'README.md')).isFile(), 'README.md index must be generated'); @@ -807,7 +807,7 @@ describe('#2142: quick task archival at milestone close-out', () => { const name = '2026-05-03-no-summary'; writeQuickTaskDir(tmpDir, name); // no files at all - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'); assert.ok(fs.statSync(path.join(archiveDir, 'README.md')).isFile(), 'README.md index must be generated'); @@ -823,7 +823,7 @@ describe('#2142: quick task archival at milestone close-out', () => { fs.writeFileSync(path.join(tmpDir, '.planning', 'quick', 'stray-notes.txt'), 'not a task dir\n'); writeQuickTaskDir(tmpDir, '2026-06-01-real-task'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.ok( fs.existsSync(path.join(tmpDir, '.planning', 'quick', 'stray-notes.txt')), @@ -864,7 +864,7 @@ describe('#2142: quick task archival at milestone close-out', () => { setupQuickArchiveRoadmap(tmpDir); writeQuickTaskDir(tmpDir, '2026-08-01-evil-version'); - const result = runSdkQuery(['milestone.complete', '../evil', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', '../evil', '--archive-quick', '--confirm'], tmpDir); assert.strictEqual(result.success, false, 'a version containing a path separator must be rejected'); assert.ok(!fs.existsSync(path.join(tmpDir, '..', 'evil')), 'nothing must be created outside the temp fixture root'); const milestonesDir = path.join(tmpDir, '.planning', 'milestones'); @@ -884,7 +884,7 @@ describe('#2142: quick task archival at milestone close-out', () => { const name = '2026-01-01-café report'; writeQuickTaskDir(tmpDir, name); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.ok( fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick', name)), @@ -924,7 +924,7 @@ describe('#2142 escalation: milestone.archive-quick — narrow archival entry po fs.writeFileSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-ROADMAP.md'), archivedRoadmap); // Confirm the premise this command exists to fix. - const milestoneCompleteResult = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const milestoneCompleteResult = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.strictEqual( milestoneCompleteResult.success, false, @@ -1078,7 +1078,7 @@ describe('#2142 review: README injection, symlink escape, dry-run/real-run parit throw err; } - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'); @@ -1121,7 +1121,7 @@ describe('#2142 review: README injection, symlink escape, dry-run/real-run parit try { writeQuickTaskDir(tmpDir, '2026-10-02-real-task'); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete failed: ${result.error}`); assert.strictEqual(result.data.archived.quick, true, 'the real task dir must still archive'); @@ -1182,7 +1182,7 @@ describe('#2142 review: README injection, symlink escape, dry-run/real-run parit 'the skipped symlink entry must not appear in the dry-run preview', ); - const real = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick'], tmpDir); + const real = runSdkQuery(['milestone.complete', 'v1.0', '--archive-quick', '--confirm'], tmpDir); assert.ok(real.success, `real run failed: ${real.error}`); const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-quick'); const archivedNames = fs @@ -1252,7 +1252,7 @@ describe('#3597: milestone complete refuses to archive on a non-COMPLETE window test('archives NOTHING and leaves every phase dir on disk when the workstream has no ROADMAP.md', () => { const alphaPhases = seedUnreadableWorkstream(tmpDir); - const result = runSdkQuery(['milestone.complete', 'v1.0'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete should still succeed (UNREADABLE is not a whole-command refusal): ${result.error}`); assert.strictEqual(result.data.archived.phases, false, 'phases must NOT be reported as archived'); @@ -1311,7 +1311,7 @@ describe('#3597: milestone complete refuses to archive on a non-COMPLETE window fs.mkdirSync(path.join(betaPhases, '01-foo'), { recursive: true }); fs.mkdirSync(path.join(betaPhases, '02-out-of-window'), { recursive: true }); - const result = runSdkQuery(['milestone.complete', 'v1.0'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete should succeed: ${result.error}`); assert.strictEqual(result.data.archived.phases, true, 'phases must be reported as archived'); @@ -1346,7 +1346,7 @@ describe('#3597: milestone complete refuses to archive on a non-COMPLETE window fs.mkdirSync(path.join(phasesDir, '01-parser'), { recursive: true }); fs.mkdirSync(path.join(phasesDir, '02-printable-output'), { recursive: true }); - const result = runSdkQuery(['milestone.complete', 'v1.0', '--force'], tmpDir); + const result = runSdkQuery(['milestone.complete', 'v1.0', '--force', '--confirm'], tmpDir); assert.ok(result.success, `milestone.complete should succeed: ${result.error}`); assert.strictEqual(result.data.archived.phases, true, 'phases must still be archived for an UNSCOPED (not UNREADABLE) window'); diff --git a/tests/milestone-window-single-owner.test.cjs b/tests/milestone-window-single-owner.test.cjs index 0e7ebef39..21638cf3f 100644 --- a/tests/milestone-window-single-owner.test.cjs +++ b/tests/milestone-window-single-owner.test.cjs @@ -1046,7 +1046,7 @@ test('milestone complete refuses to archive a truncated window', (t) => { t.after(() => cleanup(cwd)); buildTruncatedFixture(cwd); - const result = runGsdTools(['milestone', 'complete', 'v3.0', '--cwd', cwd, '--raw'], cwd); + const result = runGsdTools(['milestone', 'complete', 'v3.0', '--cwd', cwd, '--raw', '--confirm'], cwd); assert.strictEqual(result.success, false); assert.notStrictEqual(result.exitCode, 0); }); @@ -1057,7 +1057,7 @@ test('refusal leaves the phases directory untouched', (t) => { buildTruncatedFixture(cwd); const before = fs.readdirSync(path.join(cwd, '.planning', 'phases')).sort(); - const result = runGsdTools(['milestone', 'complete', 'v3.0', '--cwd', cwd, '--raw'], cwd); + const result = runGsdTools(['milestone', 'complete', 'v3.0', '--cwd', cwd, '--raw', '--confirm'], cwd); assert.strictEqual(result.success, false); const after = fs.readdirSync(path.join(cwd, '.planning', 'phases')).sort(); assert.deepStrictEqual(after, before); @@ -1068,7 +1068,7 @@ test('--force overrides the truncation refusal', (t) => { t.after(() => cleanup(cwd)); buildTruncatedFixture(cwd); - const result = runGsdTools(['milestone', 'complete', 'v3.0', '--force', '--cwd', cwd, '--raw'], cwd); + const result = runGsdTools(['milestone', 'complete', 'v3.0', '--force', '--cwd', cwd, '--raw', '--confirm'], cwd); assert.strictEqual(result.success, true, result.error); const parsed = JSON.parse(result.output); assert.strictEqual(parsed.archived.phases, true); @@ -1083,7 +1083,7 @@ test('genuinely empty milestone still completes', (t) => { const filter = getMilestonePhaseFilter(cwd, 'v1.0'); assert.strictEqual(filter.scope, SCOPE.COMPLETE); - const result = runGsdTools(['milestone', 'complete', 'v1.0', '--cwd', cwd, '--raw'], cwd); + const result = runGsdTools(['milestone', 'complete', 'v1.0', '--cwd', cwd, '--raw', '--confirm'], cwd); assert.strictEqual(result.success, true, result.error); }); diff --git a/tests/milestone.test.cjs b/tests/milestone.test.cjs index 13ed2fce1..23966b5fb 100644 --- a/tests/milestone.test.cjs +++ b/tests/milestone.test.cjs @@ -68,7 +68,7 @@ describe('milestone complete command', () => { // No ROADMAP.md — mirrors 'handles missing ROADMAP.md gracefully' so the // milestone-phase-filter guard never fires. - const result = runGsdTools('milestone complete v0.5 --name Test', tmpDir); + const result = runGsdTools('milestone complete v0.5 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); @@ -91,7 +91,7 @@ describe('milestone complete command', () => { writeState(tmpDir); mkPhaseDir(tmpDir, '01-foundation', { oneLiner: 'Set up project infrastructure' }); - const result = runGsdTools('milestone complete v1.0 --name MVP Foundation', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP Foundation --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -249,7 +249,7 @@ describe('milestone complete command', () => { writeRoadmap(tmpDir, `# Roadmap v1.0\n`); writeState(tmpDir); - const result = runGsdTools('milestone complete v1.0 --name Beta', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Beta --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const milestones = fs.readFileSync(path.join(tmpDir, '.planning', 'MILESTONES.md'), 'utf-8'); @@ -266,9 +266,9 @@ describe('milestone complete command', () => { writeRoadmap(tmpDir, `# Roadmap v1.1\n`); writeState(tmpDir); - assert.ok(runGsdTools('milestone complete v1.1 --name Second', tmpDir).success); + assert.ok(runGsdTools('milestone complete v1.1 --name Second --confirm', tmpDir).success); writeRoadmap(tmpDir, `# Roadmap v1.2\n`); - assert.ok(runGsdTools('milestone complete v1.2 --name Third', tmpDir).success); + assert.ok(runGsdTools('milestone complete v1.2 --name Third --confirm', tmpDir).success); const m = fs.readFileSync(path.join(tmpDir, '.planning', 'MILESTONES.md'), 'utf-8'); const [i10, i11, i12] = ['v1.0 First', 'v1.1 Second', 'v1.2 Third'].map(s => m.indexOf(s)); @@ -282,7 +282,7 @@ describe('milestone complete command', () => { writeState(tmpDir); mkPhaseDir(tmpDir, '01-foundation', { oneLiner: 'Set up project infrastructure' }); - const result = runGsdTools('milestone complete v1.0 --name MVP --archive-phases', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP --archive-phases --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -299,7 +299,7 @@ describe('milestone complete command', () => { writeRoadmap(tmpDir, `# Roadmap v1.0\n`); writeState(tmpDir); - assert.ok(runGsdTools('milestone complete v1.0 --name MVP', tmpDir).success); + assert.ok(runGsdTools('milestone complete v1.0 --name MVP --confirm', tmpDir).success); const archivedReq = fs.readFileSync( path.join(tmpDir, '.planning', 'milestones', 'v1.0-REQUIREMENTS.md'), 'utf-8', @@ -315,7 +315,7 @@ describe('milestone complete command', () => { writeRoadmap(tmpDir, `# Roadmap v1.0\n`); writeState(tmpDir); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -333,7 +333,7 @@ describe('milestone complete command', () => { `# State\n\n**Status:** In progress\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n\n## Current Position\n\nPhase: 03 — EXECUTING\nPlan: 03-02\nStatus: Executing\nLast activity: 2025-01-01 — Running phase\n\n## Operator Next Steps\n\n- Re-run /gsd:complete-milestone v1.0\n`, ); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); @@ -347,7 +347,7 @@ describe('milestone complete command', () => { writeRoadmap(tmpDir, `# Roadmap v1.0\n`); writeState(tmpDir); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); @@ -360,7 +360,7 @@ describe('milestone complete command', () => { test('handles missing ROADMAP.md gracefully', () => { writeState(tmpDir); - const result = runGsdTools('milestone complete v1.0 --name NoRoadmap', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name NoRoadmap --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -387,7 +387,7 @@ describe('milestone complete command', () => { fs.writeFileSync(path.join(p4, '04-02-PLAN.md'), '# Plan 2\n'); fs.writeFileSync(path.join(p4, '04-01-SUMMARY.md'), '---\none-liner: Polished UI\n---\n# Summary\n'); - const result = runGsdTools('milestone complete v1.1 --name "Second Release"', tmpDir); + const result = runGsdTools('milestone complete v1.1 --name "Second Release" --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -407,7 +407,7 @@ describe('milestone complete command', () => { mkPhaseDir(tmpDir, '01-old', { plan: true }); mkPhaseDir(tmpDir, '02-current', { plan: true }); - assert.ok(runGsdTools('milestone complete v1.1 --name Test --archive-phases', tmpDir).success); + assert.ok(runGsdTools('milestone complete v1.1 --name Test --archive-phases --confirm', tmpDir).success); assert.ok(fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.1-phases', '02-current'))); assert.ok(fs.existsSync(path.join(tmpDir, '.planning', 'phases', '01-old'))); @@ -421,7 +421,7 @@ describe('milestone complete command', () => { mkPhaseDir(tmpDir, '01-foundation', { plan: true, oneLiner: 'Foundation work' }); mkPhaseDir(tmpDir, '10-scaling', { plan: true, oneLiner: 'Scaling work' }); - const result = runGsdTools('milestone complete v1.0 --name MVP', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -439,7 +439,7 @@ describe('milestone complete command', () => { fs.mkdirSync(misc, { recursive: true }); fs.writeFileSync(path.join(misc, 'PLAN.md'), '# Not a phase\n'); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success); const output = JSON.parse(result.output); @@ -456,7 +456,7 @@ describe('milestone complete command', () => { mkPhaseDir(tmpDir, '457-integration', { plan: true }); mkPhaseDir(tmpDir, '45-old', { plan: true }); - const result = runGsdTools('milestone complete v1.49 --name DACP', tmpDir); + const result = runGsdTools('milestone complete v1.49 --name DACP --confirm', tmpDir); assert.ok(result.success); assert.strictEqual(JSON.parse(result.output).phases, 2); }); @@ -471,7 +471,7 @@ describe('milestone complete command', () => { `---\none-liner: Built the foundation\n---\n\n# Phase 1: Foundation Summary\n\n**Built the foundation**\n\n## Performance\n\n- **Duration:** 28 min\n- **Tasks:** 7\n- **Files modified:** 12\n`, ); - const result = runGsdTools('milestone complete v1.0 --name MVP', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP --confirm', tmpDir); assert.ok(result.success); assert.strictEqual(JSON.parse(result.output).tasks, 7); }); @@ -486,7 +486,7 @@ describe('milestone complete command', () => { `---\nphase: "01"\n---\n\n# Phase 1: Foundation Summary\n\n**JWT auth with refresh rotation using jose library**\n\n## Performance\n`, ); - const result = runGsdTools('milestone complete v1.0 --name MVP', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP --confirm', tmpDir); assert.ok(result.success); assert.ok(JSON.parse(result.output).accomplishments.includes('JWT auth with refresh rotation using jose library')); }); @@ -498,7 +498,7 @@ describe('milestone complete command', () => { `# State\n\nStatus: In progress\nLast Activity: 2025-01-01\nLast Activity Description: Working\n`, ); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success); assert.ok(fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8').includes('v1.0 milestone complete')); }); @@ -507,7 +507,7 @@ describe('milestone complete command', () => { writeRoadmap(tmpDir, `# Roadmap v1.0\n`); writeState(tmpDir); - const result = runGsdTools('milestone complete v1.0 --name EmptyPhases', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name EmptyPhases --confirm', tmpDir); assert.ok(result.success); const output = JSON.parse(result.output); @@ -534,7 +534,7 @@ describe('milestone complete command', () => { const statePath = path.join(tmpDir, '.planning', 'STATE.md'); const stateBefore = fs.readFileSync(statePath, 'utf-8'); - const result = runGsdTools(['milestone', 'complete', 'v1.0', '--name', 'Test'], tmpDir, PINNED_CLOCK_ENV); + const result = runGsdTools(['milestone', 'complete', 'v1.0', '--name', 'Test', '--confirm'], tmpDir, PINNED_CLOCK_ENV); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -549,7 +549,7 @@ describe('milestone complete command', () => { writeState(tmpDir); const statePath = path.join(tmpDir, '.planning', 'STATE.md'); - const run1 = runGsdTools(['milestone', 'complete', 'v1.0', '--name', 'Test', '--force'], tmpDir, PINNED_CLOCK_ENV); + const run1 = runGsdTools(['milestone', 'complete', 'v1.0', '--name', 'Test', '--force', '--confirm'], tmpDir, PINNED_CLOCK_ENV); assert.ok(run1.success, `first milestone complete failed: ${run1.error}`); const stateAfter1 = fs.readFileSync(statePath, 'utf-8'); @@ -558,7 +558,7 @@ describe('milestone complete command', () => { // genuine no-op for STATE.md content, even though MILESTONES.md still // gains a new (duplicate-looking) entry each call — the two flags are // independent and must not be conflated. - const run2 = runGsdTools(['milestone', 'complete', 'v1.0', '--name', 'Test', '--force'], tmpDir, PINNED_CLOCK_ENV); + const run2 = runGsdTools(['milestone', 'complete', 'v1.0', '--name', 'Test', '--force', '--confirm'], tmpDir, PINNED_CLOCK_ENV); assert.ok(run2.success, `second milestone complete failed: ${run2.error}`); const stateAfter2 = fs.readFileSync(statePath, 'utf-8'); @@ -622,7 +622,7 @@ describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469 test('B1: stale body Stopped at does not clobber a fresher curated frontmatter stopped_at', () => { writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' }); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); @@ -640,7 +640,7 @@ describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469 test('B2: the preserved frontmatter value is observable via a separate `state get` call', () => { writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' }); - const complete = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const complete = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(complete.success, `Command failed: ${complete.error}`); const got = runGsdTools('state get stopped_at', tmpDir); @@ -659,7 +659,7 @@ describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469 test('B3: a preserved divergence emits preservation_warnings[0].field === "stopped_at"', () => { writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' }); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -682,7 +682,7 @@ describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469 // derived result. writeStateWithSession(tmpDir, { fmStatus: 'executing', fmStoppedAt: undefined, sessionStoppedAt: undefined }); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -705,7 +705,7 @@ describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469 mkPhaseDir(tmpDir, '01-foundation', { oneLiner: 'Setup' }); writeState(tmpDir); // no frontmatter at all — nothing curated to diverge from - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -721,7 +721,7 @@ describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469 test('B3 (#3471 regression pin): preservation_warnings shape and content unchanged by Phase 4', () => { writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' }); - const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name Test --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); @@ -1739,7 +1739,7 @@ describe('milestone complete explicit version scope (#3043)', () => { fs.writeFileSync(path.join(p, 'SUMMARY.md'), `one-liner: ${liner}\n\n## Summary\n${liner.split(' ')[0]}\n`); } - const result = runGsdTools(['milestone', 'complete', 'v3.6', '--raw'], tmpDir); + const result = runGsdTools(['milestone', 'complete', 'v3.6', '--raw', '--confirm'], tmpDir); assert.equal(result.success, true, result.error || result.output); const payload = JSON.parse(result.output); assert.equal(payload.version, 'v3.6'); @@ -1757,7 +1757,7 @@ describe('milestone complete explicit version scope (#3043)', () => { fs.writeFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), '# Requirements\n'); fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); - const result = runGsdTools(['milestone', 'complete', 'v9.9', '--raw'], tmpDir); + const result = runGsdTools(['milestone', 'complete', 'v9.9', '--raw', '--confirm'], tmpDir); assert.equal(result.success, false, 'expected command to fail when no phases match explicit version'); assert.match(result.error || '', /no phases|phase/i); } finally { @@ -1789,7 +1789,7 @@ describe('#1911 — milestone complete --ws archives to the workstream', () => { // Root milestones dir pre-exists; it must NOT receive the workstream archive. fs.mkdirSync(path.join(tmpDir, '.planning', 'milestones'), { recursive: true }); - const result = runGsdTools('milestone complete v2.0 --ws ws1 --force', tmpDir); + const result = runGsdTools('milestone complete v2.0 --ws ws1 --force --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); // Archive lands inside the workstream. @@ -1820,7 +1820,7 @@ describe('#1911 — milestone complete --ws archives to the workstream', () => { fs.writeFileSync(path.join(wsBase, 'phases', '01-foo', '01-SUMMARY.md'), '---\none-liner: foo done\n---\n# Summary\n'); fs.mkdirSync(path.join(tmpDir, '.planning', 'milestones'), { recursive: true }); - const result = runGsdTools('milestone complete v2.0 --ws ws1 --force', tmpDir); + const result = runGsdTools('milestone complete v2.0 --ws ws1 --force --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const archivedReq = fs.readFileSync( @@ -1858,7 +1858,7 @@ describe('#1871 — milestone complete archives phase dirs by default', () => { test('archives phase dirs by default (no --archive-phases flag needed)', () => { seedCompletableMilestone(); - const result = runGsdTools('milestone complete v1.0 --name MVP', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const out = JSON.parse(result.output); assert.ok(out.archived.phases, 'phases should be archived by default'); @@ -1871,7 +1871,7 @@ describe('#1871 — milestone complete archives phase dirs by default', () => { test('--no-archive-phases opts out of default archiving', () => { seedCompletableMilestone(); const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); - const result = runGsdTools('milestone complete v1.0 --name MVP --no-archive-phases', tmpDir); + const result = runGsdTools('milestone complete v1.0 --name MVP --no-archive-phases --confirm', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); const out = JSON.parse(result.output); assert.ok(!out.archived.phases, 'phases should NOT be archived with --no-archive-phases'); @@ -1945,7 +1945,7 @@ describe('bug-978: milestone complete --force overrides unstarted-phase guard', makeGuardFixture(tmpDir, 'v1.0'); const result = runGsdTools( - ['milestone', 'complete', 'v1.0', '--name', 'Regression Test'], + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--confirm'], tmpDir, ); @@ -1960,7 +1960,7 @@ describe('bug-978: milestone complete --force overrides unstarted-phase guard', makeGuardFixture(tmpDir, 'v1.0'); const result = runGsdTools( - ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--force'], + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--force', '--confirm'], tmpDir, ); @@ -2482,3 +2482,250 @@ describe('enhancement #72 — Business Context template section', () => { }); }); } + +// ───────────────────────────────────────────────────────────────────────────── +// #3726: milestone complete requires --confirm before mutating +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3726: milestone complete refuses to mutate without --confirm', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + function seedMutableMilestone() { + writeRoadmap(tmpDir, `# Roadmap v1.0 MVP\n\n### Phase 1: Foundation\n**Goal:** Setup\n`); + writeState(tmpDir); + mkPhaseDir(tmpDir, '01-foundation', { plan: true, oneLiner: 'Set up foundation' }); + } + + // Full recursive content snapshot of .planning/ — "mutates nothing" is + // asserted as byte-identity of the whole tree, not spot checks. + function snapshotPlanning() { + const root = path.join(tmpDir, '.planning'); + const out = {}; + (function walk(dir) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, entry.name); + if (entry.isDirectory()) { out[path.relative(root, p) + '/'] = ''; walk(p); } + else out[path.relative(root, p)] = fs.readFileSync(p, 'utf-8'); + } + })(root); + return out; + } + + // #3726 AC 1 + AC 5: the canonical form and the `query` meta-prefix form + // resolve to the same implementation, so one test exercises the gate + // through both. The query form is the one the original incident used — + // `query milestone.complete ` read as a query and archived the + // milestone. + test('bare invocation refuses, names --confirm, and mutates nothing (canonical + query forms)', () => { + seedMutableMilestone(); + const before = snapshotPlanning(); + for (const argv of [ + ['milestone', 'complete', 'v1.0', '--name', 'MVP'], + ['query', 'milestone.complete', 'v1.0', '--name', 'MVP'], + ]) { + const result = runGsdTools(argv, tmpDir); + assert.strictEqual(result.success, false, `${argv.join(' ')} must refuse without --confirm`); + assert.match(result.error || '', /--confirm/, 'refusal must name the flag that proceeds'); + assert.match(result.error || '', /irreversible/i, 'refusal must say why it refused'); + assert.deepStrictEqual(snapshotPlanning(), before, `${argv.join(' ')} must leave .planning/ untouched`); + } + }); + + // #3726 AC 4 boundary: --force keeps its narrow meaning (bypass the + // TRUNCATED-scope / unstarted-phase guards) and does NOT double as the + // mutation opt-in. + test('--force alone does not satisfy the confirmation gate', () => { + seedMutableMilestone(); + const before = snapshotPlanning(); + const result = runGsdTools(['milestone', 'complete', 'v1.0', '--force'], tmpDir); + assert.strictEqual(result.success, false, '--force without --confirm must still refuse'); + assert.match(result.error || '', /--confirm/); + assert.deepStrictEqual(snapshotPlanning(), before, '--force refusal must leave .planning/ untouched'); + }); + + // #3726 boundary triple, third arm (review Minor 1): present-but-falsy. + // The gate is an exact-token match (`args.includes('--confirm')`), so + // `--confirm=false` / `--confirm=0` are not the token and refuse, + // fail-closed. Pinned so a future `=`-aware or prefix-matching arg parser + // cannot silently turn `--confirm=false` into a confirmed run of an + // irreversible command with no test going red. + test('--confirm=false / --confirm=0 do not satisfy the confirmation gate (canonical + query forms)', () => { + seedMutableMilestone(); + const before = snapshotPlanning(); + for (const argv of [ + ['milestone', 'complete', 'v1.0', '--confirm=false'], + ['milestone', 'complete', 'v1.0', '--confirm=0'], + ['query', 'milestone.complete', 'v1.0', '--confirm=false'], + ['query', 'milestone.complete', 'v1.0', '--confirm=0'], + ]) { + const result = runGsdTools(argv, tmpDir); + assert.strictEqual(result.success, false, `${argv.join(' ')} must refuse — the exact token is absent`); + assert.match(result.error || '', /--confirm/, 'refusal must name the flag that proceeds'); + assert.deepStrictEqual(snapshotPlanning(), before, `${argv.join(' ')} must leave .planning/ untouched`); + } + }); + + // #3726 AC 3: the preview needs no confirmation and still mutates nothing. + test('--dry-run previews without --confirm and mutates nothing', () => { + seedMutableMilestone(); + const before = snapshotPlanning(); + const result = runGsdTools(['milestone', 'complete', 'v1.0', '--dry-run', '--raw'], tmpDir); + assert.ok(result.success, `dry-run failed: ${result.error}`); + const preview = JSON.parse(result.output); + assert.strictEqual(preview.dry_run, true); + assert.deepStrictEqual(snapshotPlanning(), before, 'dry-run must leave .planning/ untouched'); + }); + + // #3726 AC 2: --confirm is the explicit opt-in and the archive then runs + // exactly as before the gate existed. + test('--confirm proceeds through the archive (query form)', () => { + seedMutableMilestone(); + const result = runGsdTools(['query', 'milestone.complete', 'v1.0', '--name', 'MVP', '--confirm'], tmpDir); + assert.ok(result.success, `--confirm run failed: ${result.error}`); + assert.ok(fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-ROADMAP.md')), 'ROADMAP archived'); + assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', 'phases', '01-foundation')), 'phase dir moved'); + }); + + // #3726 (PR #3774 review, Nit 1): the documented arg-discovery path is + // "invoke the command without args and the error lists what is required" + // (gsd-tools.cjs top-level usage). The version-required refusal is that + // error for `milestone complete`, so it must name --confirm too — otherwise + // discovering the flag takes a second round trip through the gate. + test('the version-required refusal names --confirm (arg-discovery path)', () => { + seedMutableMilestone(); + const before = snapshotPlanning(); + const result = runGsdTools(['milestone', 'complete'], tmpDir); + assert.strictEqual(result.success, false, 'bare `milestone complete` must refuse'); + assert.match(result.error || '', /version required/, 'refusal must still name the missing version'); + assert.match(result.error || '', /--confirm/, 'refusal must name --confirm so one invocation lists everything required'); + assert.deepStrictEqual(snapshotPlanning(), before, 'a version-less invocation must leave .planning/ untouched'); + }); +}); + +// ──────────────────────────────────────────────────────────────────────── +// #3726 docs pin (PR #3774 review, Minor 1). The changeset is `type: Fixed`, +// which the docs-required lint exempts, so nothing in CI would notice a later +// edit that reinstated the bare-`--force` override prose or dropped +// `--confirm` from the synopsis — doc completeness for this command would +// otherwise rest on review attention alone. This block is that gate. It is +// registered in scripts/docs-guard-registry.cjs so it runs on the PR that +// changes these docs, not only after merge. +// ──────────────────────────────────────────────────────────────────────── +describe('#3726 milestone complete docs pin', () => { + const REPO_ROOT = path.join(__dirname, '..'); + const readDoc = (...segs) => fs.readFileSync(path.join(REPO_ROOT, 'docs', ...segs), 'utf-8'); + const SYNOPSIS = 'node gsd-tools.cjs milestone complete (--confirm | --dry-run)'; + const MIRRORS = [['CLI-TOOLS.md'], ['ja-JP', 'CLI-TOOLS.md'], ['ko-KR', 'CLI-TOOLS.md'], ['pt-BR', 'CLI-TOOLS.md'], ['zh-CN', 'CLI-TOOLS.md']]; + + test('the synopsis renders --confirm and --dry-run as alternatives in CLI-TOOLS.md and every localized mirror', () => { + for (const rel of MIRRORS) { + const synopsis = readDoc(...rel).split('\n').filter((l) => l.includes('milestone complete ')); + assert.strictEqual(synopsis.length, 1, `docs/${rel.join('/')}: expected exactly one synopsis line`); + assert.ok(synopsis[0].startsWith(SYNOPSIS), `docs/${rel.join('/')}: synopsis must begin "${SYNOPSIS}", got: ${synopsis[0]}`); + } + }); + + test('the flag table documents --confirm as required to mutate and not implied by --force', () => { + const row = readDoc('CLI-TOOLS.md').split('\n').find((l) => l.startsWith('| `--confirm` |')); + assert.ok(row, 'docs/CLI-TOOLS.md: `--confirm` flag row missing'); + assert.ok(row.includes('Required to mutate'), '`--confirm` row must say it is required to mutate'); + assert.ok(row.includes('Not implied by `--force`'), '`--confirm` row must say --force does not imply it'); + }); + + // The milestone-complete sections of each doc, bounded by heading — a + // renamed heading fails loudly here instead of silently emptying the sweep. + function section(text, doc, startRe, endRe) { + const lines = text.split('\n'); + const from = lines.findIndex((l) => startRe.test(l)); + assert.ok(from >= 0, `docs/${doc}: section heading ${startRe} not found`); + let to = lines.findIndex((l, i) => i > from && endRe.test(l)); + if (to < 0) to = lines.length; + return lines.slice(from, to).map((line, i) => ({ line, n: from + i + 1 })); + } + const SECTIONS = () => { + const cli = readDoc('CLI-TOOLS.md'); + return [ + { doc: 'CLI-TOOLS.md', rows: section(cli, 'CLI-TOOLS.md', /^### `milestone complete` refuses an untrustworthy window/, /^## /) }, + { doc: 'CLI-TOOLS.md', rows: section(cli, 'CLI-TOOLS.md', /^## Milestone Commands/, /^## /) }, + { doc: 'COMMANDS.md', rows: section(readDoc('COMMANDS.md'), 'COMMANDS.md', /^### `\/gsd-complete-milestone`/, /^### /) }, + ]; + }; + + // Prose is soft-wrapped, so a line is the wrong unit: `Pass \`--force --confirm\` + // to override … (\`--force\` alone does not imply it)` spans two lines in + // docs/CLI-TOOLS.md, and the second reads bare on its own. A table row is a + // paragraph on its own; every paragraph is joined and split at sentence + // boundaries. The unit reported is the paragraph's first line. + function units(rows) { + const out = []; + let para = []; + const flush = () => { + if (para.length === 0) return; + const n = para[0].n; + const text = para.map(({ line }) => line.trim()).join(' '); + // Clause-level: a semicolon-spliced instruction ("…to override; or pass + // `--force` alone…") must not coalesce with the compliant clause before it. + for (const sentence of text.split(/(?<=[.!?;])\s+/)) out.push({ n, unit: sentence }); + para = []; + }; + for (const row of rows) { + const line = row.line.trim(); + if (line === '') { flush(); continue; } + // A table row is its own paragraph, but it is still sentence-split: a + // bare instruction appended inside the `--confirm` cell must not hide + // behind that cell's own `--confirm` token (reviewer-found escape). + if (line.startsWith('|')) { flush(); para.push({ line, n: row.n }); flush(); continue; } + para.push(row); + } + flush(); + return out; + } + + // Every --force unit that legitimately carries no --confirm, pinned EXACTLY — + // the identity-ratchet shape scripts/lib/allowlist-ratchet.cjs uses for the + // repo's lints. Classifying prose intent by regex is a snapshot of the shapes + // seen so far (the first version of this pin was one, and a reviewer refuted + // it with "re-run with `--force`"); pinning the benign set instead means a + // new --force sentence or clause without --confirm fails, whatever its + // wording, and an edit to a benign one fails loudly until the list is + // updated by hand. Named residual: a bare instruction spliced into the SAME + // clause as a compliant one ("…to override, or just `--force` if you like") + // coalesces with it and passes here; the test below pins the four known + // instructions by guard name (a substring match on each instruction's own + // `--force --confirm` text), so those cannot lose the pairing unnoticed. + const BENIGN_BARE_FORCE_UNITS = [ + '| `--force` | Override the unstarted-phase guard (see below).', + 'Not implied by `--force`, which only overrides the guards below.', + '`--force` alone does not imply it).', + "The guard runs whenever `--force` is absent, independent of `STATE.md`'s `milestone:` field — if that field is present but does not match ``, a WARNING naming both values is emitted to stderr and the scan still runs (#2946).", + ]; + + test('inside the milestone complete sections, every --force unit without --confirm is a pinned benign one', () => { + const bare = []; + for (const { doc, rows } of SECTIONS()) { + for (const { n, unit } of units(rows)) { + if (unit.includes('--force') && !unit.includes('--confirm')) bare.push({ doc, n, unit }); + } + } + const unexpected = bare.filter(({ unit }) => !BENIGN_BARE_FORCE_UNITS.includes(unit)); + assert.deepStrictEqual(unexpected.map(({ doc, n, unit }) => `docs/${doc}:${n} ${unit.slice(0, 100)}`), [], + 'a --force sentence without --confirm that is not in BENIGN_BARE_FORCE_UNITS — --force alone refuses since #3726; an override instruction must say --force --confirm, and a genuinely benign new sentence is added to the pinned list by hand'); + const missing = BENIGN_BARE_FORCE_UNITS.filter((u) => !bare.some(({ unit }) => unit === u)); + assert.deepStrictEqual(missing, [], 'a pinned benign unit no longer appears verbatim — update BENIGN_BARE_FORCE_UNITS to the edited text (or drop it) so the pin stays exact'); + }); + + test('both guard-override instructions in each doc read --force --confirm', () => { + // Pinned by guard, not by count: a lost instruction cannot be masked by an + // unrelated new match. CLI-TOOLS.md carries one guard per section; the + // COMMANDS.md section carries both as blockquotes. + const has = (rows, re) => rows.some(({ line }) => re.test(line)); + const [truncated, unstarted, commands] = SECTIONS(); + assert.ok(has(truncated.rows, /Pass `--force --confirm` to override/), 'docs/CLI-TOOLS.md truncated-window guard: override instruction missing or not --force --confirm'); + assert.ok(has(unstarted.rows, /Pass `--force --confirm` to override/), 'docs/CLI-TOOLS.md unstarted-phase guard: override instruction missing or not --force --confirm'); + assert.ok(has(commands.rows, /\*\*Truncated-window guard\.\*\*.*milestone complete --force --confirm/), 'docs/COMMANDS.md truncated-window guard: override instruction missing or not --force --confirm'); + assert.ok(has(commands.rows, /\*\*Unstarted-phase guard\.\*\*.*milestone complete --force --confirm/), 'docs/COMMANDS.md unstarted-phase guard: override instruction missing or not --force --confirm'); + }); +}); diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index c6ee6ab06..612fe94e2 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -469,7 +469,7 @@ describe('milestone complete: version path-traversal guard (#2288 security)', () // `version` is interpolated into `${version}-ROADMAP.md`, `${version}-phases`, // etc. as a MOVED/written path component — a traversal value must be rejected // before any filesystem mutation. - const result = runGsdTools('milestone complete ../../../gsd-ms-escape', tmpDir); + const result = runGsdTools('milestone complete ../../../gsd-ms-escape --confirm', tmpDir); assert.ok(!result.success, 'milestone complete must FAIL on a path-traversal version'); // No artifact created outside the project via traversal. @@ -491,7 +491,7 @@ describe('milestone complete: version path-traversal guard (#2288 security)', () fs.mkdirSync(phase1, { recursive: true }); fs.writeFileSync(path.join(phase1, '01-01-PLAN.md'), '# Plan'); - const result = runGsdTools('milestone complete v1\\\\..\\\\evil', tmpDir); + const result = runGsdTools('milestone complete v1\\\\..\\\\evil --confirm', tmpDir); assert.ok(!result.success, 'milestone complete must FAIL on a backslash-separator version'); assert.ok(fs.existsSync(phase1), 'phase dir must remain in place'); }); diff --git a/tests/phase-resolution-parity.test.cjs b/tests/phase-resolution-parity.test.cjs index 02d343289..d173b15ae 100644 --- a/tests/phase-resolution-parity.test.cjs +++ b/tests/phase-resolution-parity.test.cjs @@ -480,7 +480,7 @@ describe('#2528 consumer parity — the eight sites migrated to matchPhaseDirs', // 8. milestone complete — mutating, own project. The guard blocks // completion while any roadmap phase has no directory. - const completion = runGsdTools('milestone complete v1.0', project(dirs, query)); + const completion = runGsdTools('milestone complete v1.0 --confirm', project(dirs, query)); assert.strictEqual( completion.success, resolves, diff --git a/tests/qa/scenarios/milestone-rollover.json b/tests/qa/scenarios/milestone-rollover.json index 7b9f2e561..a9d5b6fdf 100644 --- a/tests/qa/scenarios/milestone-rollover.json +++ b/tests/qa/scenarios/milestone-rollover.json @@ -41,7 +41,7 @@ { "at": "ship:post", "run": [ - ["milestone", "complete", "1.0", "--force"] + ["milestone", "complete", "1.0", "--force", "--confirm"] ] }, { diff --git a/tests/state-contract.test.cjs b/tests/state-contract.test.cjs index 022e6a3cc..2bc575406 100644 --- a/tests/state-contract.test.cjs +++ b/tests/state-contract.test.cjs @@ -232,7 +232,10 @@ const BOUNDARY_COMMANDS = [ }, { label: 'milestone complete', - argv: ['milestone', 'complete', 'v1.1', '--force'], + // --confirm is the mutation opt-in (#3726) — without it the command + // refuses before touching anything, and this fixture must genuinely + // succeed. --force alone does not imply it. + argv: ['milestone', 'complete', 'v1.1', '--force', '--confirm'], setup: (tmpDir) => { // --force bypasses both the TRUNCATED-scope guard and the // unstarted-phase (no_directory) guard, so this fixture only needs a diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 98c844bc6..967819b1f 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -15682,7 +15682,7 @@ describe('fix #1580 — milestone complete ignores the 999 backlog sentinel', () test('completes WITHOUT --force despite a Phase 999 backlog heading', () => { const result = runGsdTools( - ['milestone', 'complete', 'v1.0', '--name', 'Regression'], + ['milestone', 'complete', 'v1.0', '--name', 'Regression', '--confirm'], tmpDir, ); assert.ok(