fix(#3726)!: require --confirm before milestone complete mutates (#3774)

* fix(#3726): require --confirm before milestone complete mutates

`milestone complete <version>` 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 <version>`,
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 <version> --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 <version>`, 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 <ci@gsd-redux>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
0xdhx
2026-08-29 16:00:45 -05:00
committed by GitHub
parent 370cfc6680
commit 472f585f7c
19 changed files with 400 additions and 87 deletions

View File

@@ -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 <version> [--name <name>] [--no-archive-phases] [--force] [--dry-run] [--archive-quick]
node gsd-tools.cjs milestone complete <version> (--confirm | --dry-run) [--name <name>] [--no-archive-phases] [--force] [--archive-quick]
# Archive .planning/quick/* into milestones/<version>-quick/ WITHOUT the milestone complete close-out (#2142)
node gsd-tools.cjs milestone archive-quick <version> [--dry-run]
@@ -896,13 +897,14 @@ node gsd-tools.cjs requirements mark-complete <ids>
| Flag | Description |
|------|-------------|
| `<version>` | 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 <name>` | Display name for the MILESTONES.md entry. Defaults to `<version>`. |
| `--no-archive-phases` | Leave phase directories in place instead of moving them into `.planning/milestones/<version>-phases/`. |
| `--archive-quick` | Opt-in (default OFF, #2142): also move every directory under `.planning/quick/` into `.planning/milestones/<version>-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 `<version>` 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 `<version>`, 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 `<version>` 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 `<version>`, 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/<version>-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.

View File

@@ -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 "<version>" 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 <version> --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 "<version>" 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 <version> --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 <version> --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 `<version>` 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 <version> --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 `<version>` 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.

View File

@@ -323,7 +323,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi
```bash
# マイルストーンをアーカイブ
node gsd-tools.cjs milestone complete <version> [--name <name>] [--no-archive-phases]
node gsd-tools.cjs milestone complete <version> (--confirm | --dry-run) [--name <name>] [--no-archive-phases] [--force] [--archive-quick]
# 要件を完了としてマーク
node gsd-tools.cjs requirements mark-complete <ids>

View File

@@ -323,7 +323,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi
```bash
# 마일스톤 아카이브
node gsd-tools.cjs milestone complete <version> [--name <name>] [--no-archive-phases]
node gsd-tools.cjs milestone complete <version> (--confirm | --dry-run) [--name <name>] [--no-archive-phases] [--force] [--archive-quick]
# 요구사항을 완료로 표시
node gsd-tools.cjs requirements mark-complete <ids>

View File

@@ -325,7 +325,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi
```bash
# Arquiva milestone
node gsd-tools.cjs milestone complete <version> [--name <name>] [--no-archive-phases]
node gsd-tools.cjs milestone complete <version> (--confirm | --dry-run) [--name <name>] [--no-archive-phases] [--force] [--archive-quick]
# Marca requisitos como concluídos
node gsd-tools.cjs requirements mark-complete <ids>

View File

@@ -323,7 +323,7 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi
```bash
# 归档里程碑
node gsd-tools.cjs milestone complete <version> [--name <name>] [--no-archive-phases]
node gsd-tools.cjs milestone complete <version> (--confirm | --dry-run) [--name <name>] [--no-archive-phases] [--force] [--archive-quick]
# 将需求标记为完成
node gsd-tools.cjs requirements mark-complete <ids>