diff --git a/.claude/hooks/metta-guard-bash.mjs b/.claude/hooks/metta-guard-bash.mjs index 70a203ad..f7276016 100755 --- a/.claude/hooks/metta-guard-bash.mjs +++ b/.claude/hooks/metta-guard-bash.mjs @@ -76,9 +76,10 @@ const BLOCKED_SUBCOMMANDS = new Set([ const BLOCKED_TWO_WORD = new Map([ ['backlog', new Set(['add', 'done', 'promote', 'migrate'])], ['changes', new Set(['abandon'])], - // `milestone create` mutates state (writes spec/milestones/.md) — Tier-2 scope - // key 'milestone:create', minted only by the metta-backlog skill. - ['milestone', new Set(['create'])], + // `milestone create`/`close`/`update` mutate state (write spec/milestones/.md) — + // Tier-2 scope keys 'milestone:create', 'milestone:close', 'milestone:update', minted + // only by the metta-backlog skill. + ['milestone', new Set(['create', 'close', 'update'])], ['roadmap', new Set(['add', 'reorder', 'next', 'remove'])], // `release cut` mutates state (version bump, tag, release commit) — Tier-2 scope // key 'release:cut', minted only by the metta-release skill. diff --git a/.claude/hooks/metta-session-mint.mjs b/.claude/hooks/metta-session-mint.mjs index a0ac90dd..21633b3c 100755 --- a/.claude/hooks/metta-session-mint.mjs +++ b/.claude/hooks/metta-session-mint.mjs @@ -32,7 +32,9 @@ const SKILL_SCOPES = { 'metta-refresh': ['refresh'], 'metta-import': ['import'], 'metta-init': ['init', 'refresh'], - 'metta-backlog': ['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create'], + // Milestone mutation scopes mint only via metta-backlog; future ship/finalize-driven + // closers need their own scope extension here rather than reusing this one. + 'metta-backlog': ['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create', 'milestone:close', 'milestone:update'], 'metta-fix-gap': ['fix-gap', 'complete', 'finalize'], 'metta-roadmap': ['roadmap:add', 'roadmap:reorder', 'roadmap:next', 'roadmap:remove'], 'metta-release': ['release:cut'], diff --git a/.claude/skills/metta-backlog/SKILL.md b/.claude/skills/metta-backlog/SKILL.md index c6c45d97..c94c0e4a 100644 --- a/.claude/skills/metta-backlog/SKILL.md +++ b/.claude/skills/metta-backlog/SKILL.md @@ -25,10 +25,12 @@ Drive the `metta backlog` CLI. The backlog is a view over `spec/issues/` frontma - **promote** → run `metta backlog list --json`, parse `.backlog[].slug` from the output, present the slugs via `AskUserQuestion`, then run `metta backlog promote `. The CLI prints the `/metta-fix-issues ` handoff command; echo that back to the user. - **done** → run `metta backlog list --json`, parse `.backlog[].slug` from the output to build the list of available slugs. Present the slugs via `AskUserQuestion`. Then ask, via `AskUserQuestion`, for an optional change name to record as `--change ` (free-form; if the user skips or leaves blank, omit the flag). Run `metta backlog done ` or `metta backlog done --change ` as appropriate. Echo the archived path (`spec/issues/resolved/.md`) printed by the CLI back to the user. - **migrate** → run `metta backlog migrate --json`. Report the converted counts (`converted.active`, `converted.done`), any `collisions` (each with `slug`, `legacy_path`, `existing_path` — collisions are reported, never overwritten), and the `archived_to` location. If `nothing_to_do` is true, tell the user there were no legacy `spec/backlog/` files to migrate. - - **milestone** → ask via `AskUserQuestion` which milestone action to take: `create | list | show`. + - **milestone** → ask via `AskUserQuestion` which milestone action to take: `create | list | show | close | update`. - **create** → ask for `slug`, `name`, and optional `target` (date) and `description`. First run `metta milestone list --json` (allow-listed; lets the session-credential mint hook complete a prior Bash cycle — output can be ignored), then run `metta milestone create --name ""` plus `--target ` / `--description ` when supplied. - **list** → run `metta milestone list` and report the rollups (open/resolved counts and percent per milestone) plus any warnings. - **show** → run `metta milestone list --json`, present the milestone slugs via `AskUserQuestion`, then run `metta milestone show ` and report the per-issue breakdown. + - **close** → run `metta milestone list --json` (allow-listed; lets the session-credential mint hook complete a prior Bash cycle), present the open-milestone slugs via `AskUserQuestion`, then ask via `AskUserQuestion` whether the milestone was achieved (`closed`) or dropped (`abandoned`). Run `metta milestone close `, appending `--abandoned` when the user chose dropped. Echo the resulting status and the commit line printed by the CLI. + - **update** → run `metta milestone list --json`, present the milestone slugs via `AskUserQuestion`, then collect which fields to change via `AskUserQuestion`: `name` / `target` / clear target / `description` / `status`. Run `metta milestone update ` with only the flags matching the chosen fields — `--name ""`, `--target `, `--clear-target`, `--description ""`, `--status `. `--status open` is the reopen path for a closed or abandoned milestone. Echo the resulting fields and the commit line printed by the CLI. 3. Echo the slug / path / next command printed by the CLI. diff --git a/docs/api.md b/docs/api.md index 3413350e..7fdb9c58 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1088,6 +1088,8 @@ Scenarios: - Milestone created with defaults - Creating a duplicate milestone is refused - Invalid milestone status is rejected +- Abandoned status validates through the schema +- Pre-existing open and closed files are unaffected ### Milestone and priority assignment via issue frontmatter @@ -1186,6 +1188,47 @@ Scenarios: - Tasks --json rendering routes through the shared helper - Helper is idempotent and precise at range boundaries +### Milestone store update applies validated patches + +Scenarios: +- Status patch preserves untouched fields +- Target is cleared from frontmatter +- Invalid patch is rejected and the file is untouched +- Updating a missing milestone fails without side effects + +### Milestone close CLI verb transitions to a terminal state + +Scenarios: +- Open milestone is closed and auto-committed +- Abandoned flag writes the abandoned state +- Closing an already-terminal milestone is a conflict +- Closing a missing milestone reports not found +- Main-branch guard applies to close + +### Milestone update CLI verb edits mutable fields + +Scenarios: +- Description is replaced without touching other fields +- Clear-target removes the field +- A mistakenly closed milestone is reopened +- Invalid field value fails validation and leaves the file untouched +- Updating a missing milestone reports not found +- No field options is an error + +### Renderers and rollups handle the abandoned state + +Scenarios: +- List sorts terminal states after open with distinct markers +- Show reports the abandoned state accurately +- Status and progress render abandoned without crashing +- Open and closed output stays byte-compatible + +### Guard authorization for milestone close and update + +Scenarios: +- Authorized skill context may invoke the new verbs +- Unauthorized context is blocked identically + ## orchestration-guard ### Inline Command-Text Tokens Never Authorize a Blocked Subcommand diff --git a/docs/architecture.md b/docs/architecture.md index 69016a19..03ec3235 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -47,7 +47,7 @@ 13 requirements ### issue-logging -29 requirements +34 requirements ### orchestration-guard 36 requirements diff --git a/docs/changelog.md b/docs/changelog.md index 9e8f8921..977b1d66 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,10 +1,50 @@ - + # Changelog ## Unreleased +### 2026-08-26 — fix-milestone-status-write-once-dead-field-no-close + +# Summary: fix-milestone-status-write-once-dead-field-no-close + +## What changed + +Milestones gained a validated write-back lifecycle. Previously `metta milestone create` wrote `status: open` permanently — no CLI path could close a milestone, edit its body, or change its target (issue `milestone-status-is-a-write-once-dead-field-with-no-close-or`). + +## Implementation (7 tasks, 4 batches) + +- **Schema** (`src/schemas/milestone-frontmatter.ts`): status enum extended to `open | closed | abandoned`; default and `.strict()` unchanged. Commit `3864badc5`. +- **Store** (`src/milestones/milestones-store.ts`): exported `MilestonePatch` and `update(slug, patch)` — read → patch → full-frontmatter Zod re-validation **before any I/O** → write; failing patches provably leave the file byte-identical; `clearTarget` removes the key entirely. Commit `3864badc5`. +- **Rollup** (`src/milestones/milestone-rollup.ts`): two-state sort replaced with a rank comparator (open first, terminal group slug-ascending — behavior-identical for open/closed-only inputs); exported shared `MILESTONE_MARKERS` (`▸`/`✓`/`✗`). Commit `959e2805d`. +- **CLI** (`src/cli/commands/milestone.ts`): new `milestone close [--abandoned]` (conflict pre-check, `chore: close milestone ` auto-commit) and `milestone update ` (`--name/--target/--clear-target/--description/--status`, Commander `conflicts`/`choices`, `chore: update milestone `); shared `commitMilestones` helper extracted from `create`; all failures exit 4 with typed JSON envelopes (`branch_guard`/`not_found`/`milestone_conflict`/`milestone_error`). Commit `5a4e3406d`. +- **Renderers** (`src/cli/commands/status.ts`, `progress.ts`): abandoned milestones render red `✗` via `MILESTONE_MARKERS`; open/closed output byte-identical to pre-change. Commits `e0063d78b`, `6429c7b88`. +- **Guard/mint hooks** (deployed + `src/templates/hooks/` mirrors, byte-identical): `milestone close`/`update` join the Tier-2 blocked set; `SKILL_SCOPES['metta-backlog']` gains `milestone:close`/`milestone:update`; 7 new guard test cases close the previously-empty milestone coverage gap. Commit `8541e2f2b`. +- **Skill** (`.claude/skills/metta-backlog/SKILL.md` + template mirror): milestone actions now `create | list | show | close | update` with dispatch branches for both new verbs. Commit `df57b4692`. + +## Verification during implementation + +Every task ran `npx tsc --noEmit` (clean) and its focused vitest suites (all green), including 309 guard-hook tests, byte-identity pins for hook mirrors, byte-compat ordering pins for the rollup sort, and byte-identical-file assertions for all failure paths. + +## Notable deviations + +- Task 1.1 pre-widened `MilestoneRollup.status` (one line) because the enum extension broke compilation — work the design assigned to the rollup component anyway. +- Task 3.3's test landed in `tests/cli-status.test.ts` (the file actually covering progress milestone rendering) rather than the plan's speculative `progress-secondary-line.test.ts`; a brief mid-batch file overlap between tasks 3.2/3.3 was reconciled with no lost work. + +## Risks + +- `status: abandoned` files fail validation under older metta builds (accepted one-way door, documented in intent). +- `update` re-serializes frontmatter via YAML.stringify — hand-edited key order/comments are normalized (accepted; hand-editing is the workflow this change eliminates). + +## Verification (3 parallel verifiers, 2 iterations) + +- **Tests**: iteration 1 found 3 failures — pre-existing scope-pin suites (`tests/metta-session-mint.test.ts`, `tests/cli-metta-guard-bash-integration.test.ts`) still expected the old `metta-backlog` scope list without `milestone:close`/`milestone:update`. Expectations updated (commit `4a9b24382`); iteration 2: **135 files, 2802 passed, 2 skipped, 0 failed**. +- **Typecheck/lint/build**: `npx tsc --noEmit` clean; `npm run lint` clean; `npm run build` (tsc + copy-templates + emit-build-stamp) succeeded. +- **Spec coverage**: all 22 scenarios across the 6 spec.md requirements have cited passing tests (per-scenario evidence table produced by the verifier). One noted caveat: R5's byte-compatibility scenario is verified via unit-level pins (legacy-comparator reproduction, marker map, envelope shapes) rather than a literal pre/post output byte-diff — the guarantee is derived, since pre-change binaries are not available in-tree. +- Review follow-up: the quality reviewer's single warning (no direct CLI test for `show` on an abandoned milestone) was closed by commit `26e0703f0`. + + ### 2026-08-23 — enforce-agent-executed-uat-run-results-attached-pr-before # Summary: enforce-agent-executed-uat-run-results-attached-pr-before diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/.metta.yaml b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/.metta.yaml new file mode 100644 index 00000000..065cfa1a --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/.metta.yaml @@ -0,0 +1,351 @@ +workflow: standard +created: 2026-08-25T23:54:01.085Z +status: active +current_artifact: verification +base_versions: {} +artifacts: + intent: complete + stories: complete + spec: complete + research: complete + design: complete + tasks: complete + implementation: complete + verification: complete +complexity_score: + score: 2 + signals: + file_count: 4 + recommended_workflow: standard +artifact_timings: + intent: + started: 2026-08-25T23:54:05.981Z + completed: 2026-08-25T23:55:12.170Z + stories: + started: 2026-08-25T23:55:16.196Z + completed: 2026-08-25T23:56:11.733Z + spec: + started: 2026-08-25T23:56:15.643Z + completed: 2026-08-25T23:58:14.236Z + research: + started: 2026-08-25T23:58:18.542Z + completed: 2026-08-26T00:03:33.558Z + design: + started: 2026-08-26T00:03:37.179Z + completed: 2026-08-26T00:06:54.524Z + tasks: + started: 2026-08-26T00:06:58.100Z + completed: 2026-08-26T00:08:43.608Z + implementation: + started: 2026-08-26T00:08:48.737Z + completed: 2026-08-26T00:23:49.771Z + verification: + completed: 2026-08-26T00:56:39.556Z +artifact_tokens: + intent: + context: 763 + budget: 20000 + stories: + context: 2412 + budget: 20000 + spec: + context: 4159 + budget: 40000 + research: + context: 4598 + budget: 60000 + design: + context: 5742 + budget: 80000 + tasks: + context: 10344 + budget: 40000 + implementation: + context: 3277 + budget: 10000 +review_iterations: 1 +verify_iterations: 2 +worktree: /home/utx0/Code/metta/.metta/worktrees/fix-milestone-status-write-once-dead-field-no-close +token_usage: + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 3223 + timestamp: 2026-08-25T23:54:27.070Z + source: hook + - task: intent + agent: metta-proposer + model: fable + tokens: 3274 + timestamp: 2026-08-25T23:55:06.852Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 4566 + timestamp: 2026-08-25T23:55:28.443Z + source: hook + - task: stories + agent: metta-product + model: fable + tokens: 3134 + timestamp: 2026-08-25T23:56:05.092Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 6406 + timestamp: 2026-08-25T23:56:33.931Z + source: hook + - task: spec + agent: metta-specifier + model: fable + tokens: 8758 + timestamp: 2026-08-25T23:58:09.085Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 9268 + timestamp: 2026-08-25T23:58:47.129Z + source: hook + - task: research + agent: metta-researcher + model: fable + tokens: 7280 + timestamp: 2026-08-26T00:00:16.910Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 9360 + timestamp: 2026-08-26T00:00:20.692Z + source: hook + - task: research + agent: metta-researcher + model: fable + tokens: 9694 + timestamp: 2026-08-26T00:00:47.241Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 9384 + timestamp: 2026-08-26T00:00:50.337Z + source: hook + - task: research + agent: metta-researcher + model: fable + tokens: 20634 + timestamp: 2026-08-26T00:02:58.753Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 13533 + timestamp: 2026-08-26T00:03:51.697Z + source: hook + - task: design + agent: metta-architect + model: fable + tokens: 16064 + timestamp: 2026-08-26T00:06:49.344Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 14942 + timestamp: 2026-08-26T00:07:12.897Z + source: hook + - task: tasks + agent: metta-planner + model: fable + tokens: 7499 + timestamp: 2026-08-26T00:08:38.301Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 17037 + timestamp: 2026-08-26T00:09:09.457Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 7030 + timestamp: 2026-08-26T00:10:52.304Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 18264 + timestamp: 2026-08-26T00:11:06.963Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 6092 + timestamp: 2026-08-26T00:12:22.284Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 22609 + timestamp: 2026-08-26T00:13:02.830Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 12653 + timestamp: 2026-08-26T00:18:38.850Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 22638 + timestamp: 2026-08-26T00:18:43.224Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 9135 + timestamp: 2026-08-26T00:19:55.239Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 22794 + timestamp: 2026-08-26T00:20:00.082Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 8852 + timestamp: 2026-08-26T00:20:08.212Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 25469 + timestamp: 2026-08-26T00:20:39.410Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 3889 + timestamp: 2026-08-26T00:21:28.113Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 25501 + timestamp: 2026-08-26T00:21:32.276Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 6306 + timestamp: 2026-08-26T00:22:58.690Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 31795 + timestamp: 2026-08-26T00:24:17.143Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 6474 + timestamp: 2026-08-26T00:25:49.898Z + source: hook + - task: implementation + agent: metta-reviewer + model: fable + tokens: 7844 + timestamp: 2026-08-26T00:25:53.427Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 31822 + timestamp: 2026-08-26T00:25:57.502Z + source: hook + - task: implementation + agent: metta-reviewer + model: fable + tokens: 8857 + timestamp: 2026-08-26T00:30:05.374Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 31868 + timestamp: 2026-08-26T00:30:09.455Z + source: hook + - task: implementation + agent: metta-reviewer + model: fable + tokens: 11586 + timestamp: 2026-08-26T00:31:30.162Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 34839 + timestamp: 2026-08-26T00:32:11.558Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 2208 + timestamp: 2026-08-26T00:36:15.855Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 36263 + timestamp: 2026-08-26T00:36:38.298Z + source: hook + - task: verification + agent: metta-verifier + model: fable + tokens: 660 + timestamp: 2026-08-26T00:37:06.534Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 36295 + timestamp: 2026-08-26T00:37:11.334Z + source: hook + - task: verification + agent: metta-verifier + model: fable + tokens: 8137 + timestamp: 2026-08-26T00:45:14.044Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 36335 + timestamp: 2026-08-26T00:45:19.152Z + source: hook + - task: verification + agent: metta-verifier + model: fable + tokens: 1907 + timestamp: 2026-08-26T00:45:55.392Z + source: hook + - task: metta-skill-host + agent: metta-skill-host + model: fable + tokens: 37959 + timestamp: 2026-08-26T00:46:20.560Z + source: hook + - task: implementation + agent: metta-executor + model: fable + tokens: 2715 + timestamp: 2026-08-26T00:56:17.088Z + source: hook diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/TOKENS.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/TOKENS.md new file mode 100644 index 00000000..a5d1b318 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/TOKENS.md @@ -0,0 +1,96 @@ +# Token usage: fix-milestone-status-write-once-dead-field-no-close + +- **Change**: fix-milestone-status-write-once-dead-field-no-close +- **Generated**: 2026-08-26 + +> Provenance per row: `hook (exact)` rows are harness-measured token counts +> recorded automatically by the token-recording hook; `prose (estimate)` rows +> are orchestrator-estimated figures and may under- or over-count actual +> provider usage. When both exist for the same task and agent, the exact hook +> figure is used. + +## Total + +**~682,852 tokens** across 47 record(s). + +## Per artifact + +| Artifact/task | Agent | Model | Tokens | Provenance | +|---|---|---|---|---| +| metta-skill-host | metta-skill-host | fable | 3,223 | hook (exact) | +| intent | metta-proposer | fable | 3,274 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 4,566 | hook (exact) | +| stories | metta-product | fable | 3,134 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 6,406 | hook (exact) | +| spec | metta-specifier | fable | 8,758 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 9,268 | hook (exact) | +| research | metta-researcher | fable | 7,280 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 9,360 | hook (exact) | +| research | metta-researcher | fable | 9,694 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 9,384 | hook (exact) | +| research | metta-researcher | fable | 20,634 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 13,533 | hook (exact) | +| design | metta-architect | fable | 16,064 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 14,942 | hook (exact) | +| tasks | metta-planner | fable | 7,499 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 17,037 | hook (exact) | +| implementation | metta-executor | fable | 7,030 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 18,264 | hook (exact) | +| implementation | metta-executor | fable | 6,092 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 22,609 | hook (exact) | +| implementation | metta-executor | fable | 12,653 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 22,638 | hook (exact) | +| implementation | metta-executor | fable | 9,135 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 22,794 | hook (exact) | +| implementation | metta-executor | fable | 8,852 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 25,469 | hook (exact) | +| implementation | metta-executor | fable | 3,889 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 25,501 | hook (exact) | +| implementation | metta-executor | fable | 6,306 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 31,795 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 6,474 | hook (exact) | +| implementation | metta-reviewer | fable | 7,844 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 31,822 | hook (exact) | +| implementation | metta-reviewer | fable | 8,857 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 31,868 | hook (exact) | +| implementation | metta-reviewer | fable | 11,586 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 34,839 | hook (exact) | +| implementation | metta-executor | fable | 2,208 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 36,263 | hook (exact) | +| verification | metta-verifier | fable | 660 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 36,295 | hook (exact) | +| verification | metta-verifier | fable | 8,137 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 36,335 | hook (exact) | +| verification | metta-verifier | fable | 1,907 | hook (exact) | +| metta-skill-host | metta-skill-host | fable | 37,959 | hook (exact) | +| implementation | metta-executor | fable | 2,715 | hook (exact) | + +## Per role + +| Agent | Tokens | +|---|---| +| metta-architect | 16,064 | +| metta-executor | 58,880 | +| metta-planner | 7,499 | +| metta-product | 3,134 | +| metta-proposer | 3,274 | +| metta-researcher | 37,608 | +| metta-reviewer | 28,287 | +| metta-skill-host | 508,644 | +| metta-specifier | 8,758 | +| metta-verifier | 10,704 | + +## Per model + +| Model | Tokens | +|---|---| +| fable | 682,852 | + +## Cheap/pinned (non-inherit) vs inherit + +- **Cheap/pinned (non-inherit)**: ~682,852 tokens +- **Inherit**: ~0 tokens + +## Gaps + +No gaps found. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/UAT.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/UAT.md new file mode 100644 index 00000000..f8fa745d --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/UAT.md @@ -0,0 +1,350 @@ +# UAT: fix-milestone-status-write-once-dead-field-no-close + +- **Change**: fix-milestone-status-write-once-dead-field-no-close +- **Generated**: 2026-08-26 +- **Source**: user stories (stories.md) + +## Reporting failures + +If any step below fails or behaves unexpectedly, log a metta issue +(`/metta-issue `) referencing this file and the step number. +The sanctioned UAT runner (`/metta-uat`) may flip a step's Pass checkbox +to reflect a genuinely observed outcome and may append dated `## UAT run` +records below the steps. Never fabricate a pass: do not alter step content, +and never check a box for behavior that was not actually observed. + +## Acceptance steps + +### US-1: Close a completed milestone from the CLI + +*Independent test:* Running `metta milestone close ` against an open milestone transitions its frontmatter to `status: closed` on disk via a validated write, and the transition is auto-committed with a conventional `chore:` message. + +#### Step 1.1 +- **Setup**: a milestone file with `status: open` +- **Do**: the user runs `metta milestone close ` +- **Observe**: the file's frontmatter is rewritten to `status: closed`, the result passes `MilestoneFrontmatterSchema` validation before write, and the change is auto-committed with a conventional `chore:` message +- [ ] Pass + +#### Step 1.2 +- **Setup**: a milestone that is already `closed` +- **Do**: the user runs `metta milestone close ` +- **Observe**: the command fails with a clear conflict error using the standard JSON error envelope and the file is not modified +- [ ] Pass + +#### Step 1.3 +- **Setup**: a slug with no matching milestone file +- **Do**: the user runs `metta milestone close ` +- **Observe**: the command fails with a clear not-found error and no file is created or modified +- [ ] Pass + +#### Step 1.4 +- **Setup**: the repository is on the main branch +- **Do**: the user runs `metta milestone close ` without `--on-branch` acknowledgment (Run: `metta milestone create`) +- **Observe**: the command respects the same main-branch guard behavior as `metta milestone create` +- [ ] Pass + +### US-2: Update a milestone's mutable fields without hand-editing YAML + +*Independent test:* Running `metta milestone update ` with field options patches exactly the specified fields, re-validates the full frontmatter before write, auto-commits, and leaves unspecified fields untouched. + +#### Step 2.1 +- **Setup**: an existing milestone +- **Do**: the user runs `metta milestone update --description ""` +- **Observe**: the description body is replaced, all other fields are unchanged, and the write is validated and auto-committed +- [ ] Pass + +#### Step 2.2 +- **Setup**: an existing milestone with a `target` set +- **Do**: the user runs `metta milestone update --clear-target` +- **Observe**: the target is removed and the resulting frontmatter still passes schema validation +- [ ] Pass + +#### Step 2.3 +- **Setup**: a closed milestone that was closed by mistake +- **Do**: the user explicitly requests a status change via `metta milestone update ` +- **Observe**: the milestone is reopened through the same validated update path +- [ ] Pass + +#### Step 2.4 +- **Setup**: an update whose resulting frontmatter would fail `MilestoneFrontmatterSchema` +- **Do**: the command runs +- **Observe**: the write is rejected with a clear validation error and no unvalidated state reaches disk +- [ ] Pass + +#### Step 2.5 +- **Setup**: a slug with no matching milestone file +- **Do**: the user runs `metta milestone update ` +- **Observe**: the command fails with a clear not-found error +- [ ] Pass + +### US-3: Distinguish abandoned milestones from completed ones + +*Independent test:* Closing a milestone with the abandoned flag writes `status: abandoned`, which the schema accepts, and existing files with `open`/`closed` continue to validate unchanged. + +#### Step 3.1 +- **Setup**: an open milestone +- **Do**: the user runs `metta milestone close --abandoned` +- **Observe**: the frontmatter is written as `status: abandoned` and passes validation +- [ ] Pass + +#### Step 3.2 +- **Setup**: existing milestone files carrying `status: open` or `status: closed` +- **Do**: any milestone command reads them +- **Observe**: they validate and behave exactly as before this change +- [ ] Pass + +#### Step 3.3 +- **Setup**: an `abandoned` milestone +- **Do**: the user views it via `milestone show` (Run: `milestone show`) +- **Observe**: the status line clearly reports the abandoned state rather than mislabeling it as open or closed +- [ ] Pass + +### US-4: Accurate milestone status in dashboards and lists + +*Independent test:* `milestone list`, `milestone show`, and the `status`/`progress` milestone sections render `closed` and `abandoned` states with distinct, sensible output, with terminal-state milestones sorted after open ones, and byte-identical output for `open`/`closed` milestones compared to the prior release. + +#### Step 4.1 +- **Setup**: milestones in `open`, `closed`, and `abandoned` states +- **Do**: the user runs `metta milestone list` (Run: `metta milestone list`) +- **Observe**: each state renders a sensible marker and terminal-state milestones (closed and abandoned) sort after open ones +- [ ] Pass + +#### Step 4.2 +- **Setup**: an `abandoned` milestone +- **Do**: `metta status` or `metta progress` renders the milestone section (Run: `metta status`, `metta progress`) +- **Observe**: the rollup renders the state without crashing or mislabeling it +- [ ] Pass + +#### Step 4.3 +- **Setup**: only `open` and `closed` milestones +- **Do**: any milestone rendering runs +- **Observe**: human and `--json` output is unchanged from pre-change behavior +- [ ] Pass + +### US-5: New milestone verbs usable from authorized AI sessions + +*Independent test:* The `metta-guard-bash` allow-list authorizes `milestone close` and `milestone update` under the same trust rules as existing milestone verbs, with no change to the trust-tier model. + +#### Step 5.1 +- **Setup**: an authorized skill context that can already invoke existing milestone verbs +- **Do**: it invokes `metta milestone close ` or `metta milestone update ` (Run: `milestone close`, `milestone update`) +- **Observe**: the guard hook permits the command +- [ ] Pass + +#### Step 5.2 +- **Setup**: an unauthorized context +- **Do**: it attempts the new verbs +- **Observe**: the guard blocks them exactly as it blocks existing milestone verbs +- [ ] Pass + +## Additional scenarios + +#### Step 6.1: Milestone created with defaults +- **Setup**: no milestone `v0-6` exists +- **Do**: the user runs `metta milestone create v0-6 --name "v0.6" --target 2026-09-30 --description "Backlog/milestone unification release"` (Run: `metta milestone create v0-6 --name "v0.6" --target 2026-09-30 --description "Backlog/milestone unification release"`, `metta milestone list`) +- **Observe**: `spec/milestones/v0-6.md` is written with frontmatter `name: v0.6`, `target: 2026-09-30`, `status: open`, the description as body, and `metta milestone list` includes `v0-6` +- [ ] Pass + +#### Step 6.2: Creating a duplicate milestone is refused +- **Setup**: `spec/milestones/v0-6.md` already exists +- **Do**: the user runs `metta milestone create v0-6 --name "v0.6 again"` (Run: `metta milestone create v0-6 --name "v0.6 again"`) +- **Observe**: the command exits non-zero with an error stating the milestone already exists, and the existing file is unmodified +- [ ] Pass + +#### Step 6.3: Invalid milestone status is rejected +- **Setup**: a milestone file whose frontmatter contains `status: shipped` +- **Do**: the milestone store reads the file +- **Observe**: validation fails with an error naming the `status` field and the allowed values `open`, `closed`, `abandoned` +- [x] Pass + +#### Step 6.4: Abandoned status validates through the schema +- **Setup**: a milestone file whose frontmatter contains `status: abandoned` +- **Do**: the milestone store reads the file +- **Observe**: validation succeeds and the parsed milestone reports status `abandoned` +- [x] Pass + +#### Step 6.5: Pre-existing open and closed files are unaffected +- **Setup**: milestone files on disk carrying `status: open` and `status: closed` written before the enum extension +- **Do**: any milestone command reads them +- **Observe**: both files validate without error and produce the same parsed status values as before the change +- [x] Pass + +#### Step 6.6: Status patch preserves untouched fields +- **Setup**: `spec/milestones/m1.md` with frontmatter `name: M1`, `target: 2026-09-30`, `status: open` and a non-empty description body +- **Do**: `update('m1', { status: 'closed' })` is called +- **Observe**: the rewritten file carries `status: closed`, retains `name: M1`, `target: 2026-09-30`, and the identical description body, and the written frontmatter passed `MilestoneFrontmatterSchema` validation before the write +- [x] Pass + +#### Step 6.7: Target is cleared from frontmatter +- **Setup**: a milestone whose frontmatter includes a `target` field +- **Do**: `update` is called with a patch that clears the target +- **Observe**: the rewritten frontmatter contains no `target` key (not `target: null`) and still passes schema validation +- [x] Pass + +#### Step 6.8: Invalid patch is rejected and the file is untouched +- **Setup**: an existing milestone file and a byte snapshot of its content +- **Do**: `update` is called with a patch producing invalid frontmatter (e.g. `target: '2026-02-30'` or an empty `name`) +- **Observe**: the call throws a validation error identifying the offending field, and the file on disk is byte-identical to the snapshot +- [x] Pass + +#### Step 6.9: Updating a missing milestone fails without side effects +- **Setup**: no file exists at `spec/milestones/ghost.md` +- **Do**: `update('ghost', { status: 'closed' })` is called +- **Observe**: the call throws an error stating milestone `ghost` was not found, and no file is created under `spec/milestones/` +- [x] Pass + +#### Step 6.10: Open milestone is closed and auto-committed +- **Setup**: `spec/milestones/m1.md` with `status: open` in a git repository on the main branch +- **Do**: the user runs `metta milestone close m1` (Run: `metta milestone close m1`) +- **Observe**: the file's frontmatter reads `status: closed`, a commit exists with message `chore: close milestone m1`, and the command exits 0 reporting the closure +- [ ] Pass + +#### Step 6.11: Abandoned flag writes the abandoned state +- **Setup**: `spec/milestones/m6.md` with `status: open` +- **Do**: the user runs `metta milestone close m6 --abandoned` (Run: `metta milestone close m6 --abandoned`) +- **Observe**: the file's frontmatter reads `status: abandoned`, the result passed schema validation before write, and the transition is auto-committed with `chore: close milestone m6` +- [ ] Pass + +#### Step 6.12: Closing an already-terminal milestone is a conflict +- **Setup**: `spec/milestones/m1.md` with `status: closed` and a byte snapshot of the file +- **Do**: the user runs `metta milestone close m1 --json` (Run: `metta milestone close m1 --json`) +- **Observe**: the command exits 4, stdout carries a JSON error envelope with a conflict-typed error naming the milestone's current status, and the file is byte-identical to the snapshot +- [ ] Pass + +#### Step 6.13: Closing a missing milestone reports not found +- **Setup**: no file exists at `spec/milestones/ghost.md` +- **Do**: the user runs `metta milestone close ghost --json` (Run: `metta milestone close ghost --json`) +- **Observe**: the command exits 4 with a JSON error envelope of type `not_found`, and no file is created or modified +- [ ] Pass + +#### Step 6.14: Main-branch guard applies to close +- **Setup**: the repository checkout is on a branch other than the configured main branch +- **Do**: the user runs `metta milestone close m1` without `--on-branch` (Run: `metta milestone close m1`, `metta milestone create`) +- **Observe**: the command refuses with the same branch-guard error behavior as `metta milestone create`, and the milestone file is unmodified +- [ ] Pass + +#### Step 6.15: Description is replaced without touching other fields +- **Setup**: `spec/milestones/m1.md` with `status: open`, a `target`, and a stale body reading "In flight as PR #24" +- **Do**: the user runs `metta milestone update m1 --description "Shipped in v0.5.0"` (Run: `metta milestone update m1 --description "Shipped in v0.5.0"`) +- **Observe**: the body reads "Shipped in v0.5.0", the frontmatter `name`, `target`, and `status` values are unchanged, and the change is auto-committed with `chore: update milestone m1` +- [ ] Pass + +#### Step 6.16: Clear-target removes the field +- **Setup**: a milestone with `target: 2026-09-30` +- **Do**: the user runs `metta milestone update --clear-target` +- **Observe**: the rewritten frontmatter contains no `target` key and passes schema validation +- [ ] Pass + +#### Step 6.17: A mistakenly closed milestone is reopened +- **Setup**: a milestone with `status: closed` +- **Do**: the user runs `metta milestone update --status open` +- **Observe**: the frontmatter reads `status: open`, the write passed validation, and the change is auto-committed +- [ ] Pass + +#### Step 6.18: Invalid field value fails validation and leaves the file untouched +- **Setup**: an existing milestone and a byte snapshot of its file +- **Do**: the user runs `metta milestone update --target 2026-02-30 --json` +- **Observe**: the command exits 4, stdout carries a JSON error envelope with a validation message naming `target`, and the file is byte-identical to the snapshot +- [ ] Pass + +#### Step 6.19: Updating a missing milestone reports not found +- **Setup**: no file exists at `spec/milestones/ghost.md` +- **Do**: the user runs `metta milestone update ghost --name "Ghost"` (Run: `metta milestone update ghost --name "Ghost"`) +- **Observe**: the command exits 4 with a not-found error, and no file is created +- [ ] Pass + +#### Step 6.20: No field options is an error +- **Setup**: an existing milestone +- **Do**: the user runs `metta milestone update ` with no field options +- **Observe**: the command exits non-zero with an error stating at least one field option is required, and the file is unmodified +- [ ] Pass + +#### Step 6.21: List sorts terminal states after open with distinct markers +- **Setup**: milestones in `open`, `closed`, and `abandoned` states +- **Do**: the user runs `metta milestone list` (Run: `metta milestone list`) +- **Observe**: open milestones appear before both terminal milestones, each of the three states renders its own marker, and the `abandoned` row's marker differs from `▸` and `✓` +- [ ] Pass + +#### Step 6.22: Show reports the abandoned state accurately +- **Setup**: an `abandoned` milestone +- **Do**: the user runs `metta milestone show ` (human and `--json`) +- **Observe**: the human output contains `Status: abandoned` and the JSON output carries `"status": "abandoned"` +- [ ] Pass + +#### Step 6.23: Status and progress render abandoned without crashing +- **Setup**: at least one `abandoned` milestone exists alongside open milestones +- **Do**: `metta status` or `metta progress` renders its milestone section (Run: `metta status`, `metta progress`) +- **Observe**: the command exits 0 and the abandoned milestone appears in the rollup sorted after open milestones, labeled with its abandoned state +- [ ] Pass + +#### Step 6.24: Open and closed output stays byte-compatible +- **Setup**: a project containing only `open` and `closed` milestones +- **Do**: `metta milestone list`, `metta milestone show`, `metta status`, and `metta progress` run in both human and `--json` modes (Run: `metta milestone list`, `metta milestone show`) +- **Observe**: the output of each is byte-identical to the output produced before this change +- [ ] Pass + +#### Step 6.25: Authorized skill context may invoke the new verbs +- **Setup**: a skill context holding a valid session credential that authorizes `metta milestone create` +- **Do**: it invokes `metta milestone close ` or `metta milestone update ` (Run: `metta milestone create`) +- **Observe**: the guard hook permits both commands +- [ ] Pass + +#### Step 6.26: Unauthorized context is blocked identically +- **Setup**: a context without a valid session credential +- **Do**: it attempts `metta milestone close ` or `metta milestone update ` (Run: `metta milestone create`) +- **Observe**: the guard blocks the commands with the same denial behavior it applies to `metta milestone create` +- [ ] Pass + +## UAT run — 2026-08-26 + +- **Runner**: metta-uat-runner agent via /metta-uat, model: claude-fable-5 (self-reported) +- **Completed**: 2026-08-26T01:11:08.507Z +- **Result**: 7 pass / 0 fail / 36 skip (of 43 steps) + +| Step | Outcome | Note | +|------|---------|------| +| 1.1 | skip | requires forbidden `metta milestone close` CLI invocation (runner limited to `metta status --json`) | +| 1.2 | skip | requires forbidden `metta milestone close` CLI invocation | +| 1.3 | skip | requires forbidden `metta milestone close` CLI invocation | +| 1.4 | skip | requires forbidden `metta milestone close`/`create` CLI invocations | +| 2.1 | skip | requires forbidden `metta milestone update` CLI invocation | +| 2.2 | skip | requires forbidden `metta milestone update` CLI invocation (store-level equivalent verified in 6.7) | +| 2.3 | skip | requires forbidden `metta milestone update` CLI invocation | +| 2.4 | skip | requires forbidden `metta milestone update` CLI invocation (store-level equivalent verified in 6.8) | +| 2.5 | skip | requires forbidden `metta milestone update` CLI invocation (store-level equivalent verified in 6.9) | +| 3.1 | skip | requires forbidden `metta milestone close --abandoned` CLI invocation | +| 3.2 | skip | "behave exactly as before" requires forbidden milestone CLI runs; store-level validation of open/closed files confirmed in 6.5 | +| 3.3 | skip | requires forbidden `metta milestone show` CLI invocation | +| 4.1 | skip | requires forbidden `metta milestone list` CLI invocation | +| 4.2 | skip | requires `metta progress`/human-mode `metta status` (forbidden); project also has no milestone files for setup | +| 4.3 | skip | requires forbidden milestone CLI invocations plus pre-change output comparison | +| 5.1 | skip | requires invoking guarded `metta milestone` verbs from an authorized skill context — forbidden to this runner | +| 5.2 | skip | requires invoking guarded `metta milestone` verbs from an unauthorized context — forbidden to this runner | +| 6.1 | skip | requires forbidden `metta milestone create` CLI invocation | +| 6.2 | skip | requires forbidden `metta milestone create` CLI invocation | +| 6.3 | pass | store read threw: `status: Invalid enum value. Expected 'open' \| 'closed' \| 'abandoned', received 'shipped'` | +| 6.4 | pass | validation succeeded; parsed status `abandoned` | +| 6.5 | pass | open/closed fixtures validated; parsed statuses `open`/`closed` | +| 6.6 | pass | file rewritten with `status: closed`; name, target, body byte-identical; validation precedes write in `MilestonesStore.update` | +| 6.7 | pass | rewritten frontmatter has no `target` key; re-read passes schema validation | +| 6.8 | pass | throws validation error naming `target`; file byte-identical to snapshot | +| 6.9 | pass | throws `Milestone 'ghost' not found`; no file created | +| 6.10 | skip | requires forbidden `metta milestone close` CLI invocation and git commit inspection | +| 6.11 | skip | requires forbidden `metta milestone close --abandoned` CLI invocation | +| 6.12 | skip | requires forbidden `metta milestone close --json` CLI invocation | +| 6.13 | skip | requires forbidden `metta milestone close --json` CLI invocation | +| 6.14 | skip | requires forbidden `metta milestone close`/`create` CLI invocations | +| 6.15 | skip | requires forbidden `metta milestone update` CLI invocation | +| 6.16 | skip | requires forbidden `metta milestone update --clear-target` CLI invocation (store-level equivalent verified in 6.7) | +| 6.17 | skip | requires forbidden `metta milestone update --status` CLI invocation | +| 6.18 | skip | requires forbidden `metta milestone update --json` CLI invocation (store-level equivalent verified in 6.8) | +| 6.19 | skip | requires forbidden `metta milestone update` CLI invocation (store-level equivalent verified in 6.9) | +| 6.20 | skip | requires forbidden `metta milestone update` CLI invocation | +| 6.21 | skip | requires forbidden `metta milestone list` CLI invocation | +| 6.22 | skip | requires forbidden `metta milestone show` CLI invocation | +| 6.23 | skip | requires `metta progress`/human-mode `metta status` (forbidden); project has no abandoned milestone for setup | +| 6.24 | skip | requires forbidden milestone CLI invocations plus pre-change byte comparison | +| 6.25 | skip | requires invoking guarded `metta milestone` verbs with a session credential — forbidden to this runner | +| 6.26 | skip | requires invoking guarded `metta milestone` verbs without a credential — forbidden to this runner | + +- **Note**: Edit tool refused by guard; document rewritten via heredoc fallback diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/design.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/design.md new file mode 100644 index 00000000..89a3529a --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/design.md @@ -0,0 +1,272 @@ +# Design: fix-milestone-status-write-once-dead-field-no-close + +## Approach + +Implement the selected research approach (`research.md`, detailed in `research-store-update.md`): give milestones a validated write-back path by mirroring the proven sibling `IssuesStore.updateFrontmatter` pattern (`src/issues/issues-store.ts:223-234`) — read → patch → full-frontmatter Zod re-validation → write, with git commits staying at the CLI edge ("functional core, imperative shell"). Composition throughout: the new CLI verbs compose the existing `MilestonesStore`, `assertOnMainBranch`, and a small extracted commit helper; no inheritance, no new abstractions beyond a 3-entry marker map and that helper (which removes duplication). + +Five concrete moves, in dependency order (this is also the recommended implementation order): + +1. **Schema** — extend the status enum with `abandoned` (spec: "MODIFIED: Milestone store with Zod-validated frontmatter and CLI"). +2. **Store** — add `MilestonesStore.update(slug, patch)`; validation precedes all I/O, so byte-identical-on-failure is structural, not defensive (spec: "Milestone store update applies validated patches"; constitution: "No unvalidated state writes"). +3. **Rollup + renderers** — rank-comparator sort (open=0, terminal=1; provably behavior-identical for open/closed-only inputs) and a shared exported `MILESTONE_MARKERS` map replacing three duplicated ternaries (spec: "Renderers and rollups handle the abandoned state", including the byte-compat scenario). +4. **CLI** — `milestone close [--abandoned]` and `milestone update` cloned from the `create` action shape: same branch guard, same JSON error envelope, same swallow-on-failure auto-commit via a shared `commitMilestones` helper (spec: close/update CLI requirements). +5. **Hooks + skill** — `close`/`update` join `BLOCKED_TWO_WORD` in the guard (scope keys auto-derive), `milestone:close`/`milestone:update` join `SKILL_SCOPES['metta-backlog']` in the mint hook, each edit mirrored byte-identically into `src/templates/hooks/`; the `metta-backlog` skill (both copies) gains `close`/`update` branches (spec: "Guard authorization for milestone close and update"). + +Rejected alternatives (derived status; close-only + advisory) are documented with rationale in `research.md` — both fail most spec requirements and the second doubles lifecycle cost. No vendor lock-in anywhere in this change: all dependencies are already-installed npm packages and in-repo modules. + +## Components + +### 1. `src/schemas/milestone-frontmatter.ts` (edit, 1 line + type ripple) + +- Line 26: `status: z.enum(['open', 'closed']).default('open')` → `status: z.enum(['open', 'closed', 'abandoned']).default('open')`. +- Zod's enum error already names the received value and the allowed values; `formatZodError` renders it — satisfies the "`status: shipped` rejected naming allowed values" scenario with no extra code. +- `.strict()` and the `target` regex + real-calendar-date refinement are untouched. + +### 2. `src/milestones/milestones-store.ts` (edit) + +- `Milestone.status` (line 14): replace the duplicated literal union with the schema-derived type — `status: MilestoneFrontmatter['status']` — killing the duplication permanently (single source of truth in the schema). +- New exported interface `MilestonePatch` and new method `update` (signatures in API Design). Flow: + 1. `assertSafeSlug(slug)` (same guard as every existing method). + 2. Programmer-error check: `patch.target !== undefined && patch.clearTarget` → `throw new Error('clearTarget and target are mutually exclusive')`. + 3. Exists check on `join('milestones', `${slug}.md`)`; throw `` `Milestone '${slug}' not found` `` — exact text of `show` (line 133), so the CLI's existing `message.includes('not found')` → `not_found` mapping works unchanged. Never creates a file. + 4. `this.state.readRaw(relPath)` + `parseMilestone(content, slug, relPath)` — a corrupt current file fails here, before any write. + 5. Build next frontmatter: `name: patch.name ?? current.name`; `target` key present iff `patch.clearTarget` is false and (`patch.target ?? current.target`) is defined; `status: patch.status ?? current.status`. + 6. `validateFrontmatter(next, relPath)` — full resulting frontmatter through `MilestoneFrontmatterSchema` **before any I/O**. A failing patch throws here; the file is byte-identical by construction. + 7. `this.state.writeRaw(relPath, formatMilestone(validated, patch.description ?? current.description))`. `formatMilestone`'s `YAML.stringify` (yaml default `keepUndefined: false`) omits an absent `target` key entirely — never `target: null` — satisfying the clear-target scenario. + 8. Return the updated `Milestone` (re-built from validated frontmatter + body) so the CLI emits JSON without a second read. +- `create`/`list`/`show`/`exists` are untouched. +- An empty patch (`update(slug, {})`) is a validated no-op rewrite at store level; the CLI enforces "at least one field option" so the store stays simple (per research §2). + +### 3. `src/milestones/milestone-rollup.ts` (edit) + +- `MilestoneRollup.status` (line 7): `'open' | 'closed'` → `Milestone['status']`. +- Sort (lines 77-80): replace the two-state comparator with a rank comparator (exact form in API Design). For open/closed-only inputs this is behaviorally identical (open→0, closed→1), preserving the spec's byte-compat requirement; `abandoned` joins the terminal group, slug-ascending within it. Update the function's doc comment ("sorted open-first, then terminal, then slug ascending"). +- New export: `MILESTONE_MARKERS` map (`▸` open, `✓` closed, `✗` abandoned) — the single source for all three render sites so glyphs cannot drift. `✗` (U+2717) matches the width class of `✓` (U+2713); no `padEnd` misalignment. +- Barrel: `src/index.ts` already does `export * from './milestones/milestone-rollup.js'` and `.../milestones-store.js` — `MilestonePatch` and `MILESTONE_MARKERS` flow through with no barrel edit. + +### 4. `src/cli/commands/milestone.ts` (edit — the largest single file change) + +- **`commitMilestones` helper** (module-private async function): extract `create`'s commit block (lines 77-87) verbatim — `git add spec/milestones` → `git commit -m ` → `git rev-parse HEAD`, whole block in a swallowing try/catch returning `{ committed: false }` when git is unavailable or there is nothing to commit. `create` refactors onto it (behavior-preserving; commit message string `chore: create milestone ${slug}` passed in by the caller — no behavior change). +- **`close` subcommand**: registered after `create`; option/flow details in API Design. Conflict check happens in the CLI (`current.status !== 'open'`) before any store call, so an already-terminal milestone's file is provably untouched. +- **`update` subcommand**: option-driven patch; builds `MilestonePatch` from provided options only; "no field options" pre-check; `--target`/`--clear-target` mutual exclusion via Commander's `.conflicts()` (available — repo pins `commander: ^13.1.0` in `package.json:37`; `.conflicts()` shipped in Commander 9). Import ripple: `import { Command, Option } from 'commander'` (`Option` needed for `.choices()` and `.conflicts()`). +- **`list` renderer** (line 121): ternary → `MILESTONE_MARKERS[r.status]` (uncolored). +- `show` needs no change: it prints `Status: ${item.status}` and passes `rollup.status` through to JSON — `abandoned` flows through as data once the type widens. + +### 5. `src/cli/commands/status.ts` (edit, 1 line) + +- `printMilestoneSection` (line 33): ternary → colored map lookup: `✓` stays green (32), `▸` stays cyan (36), `✗` renders red (31). Implemented as a small local lookup over `MILESTONE_MARKERS` glyphs with a color-code map `{ open: 36, closed: 32, abandoned: 31 }` (kept local per site since only the two colored sites need colors). + +### 6. `src/cli/commands/progress.ts` (edit, 1 line) + +- Line 217: identical treatment to status.ts. The adjacent grey (90) target text motivates red over grey for `✗` (visually distinct); glyph — not just color — differs, satisfying the distinguishability requirement in no-color terminals. + +### 7. Guard hook pair (2 files, byte-identical edit) + +- `.claude/hooks/metta-guard-bash.mjs` line 81: `['milestone', new Set(['create'])]` → `['milestone', new Set(['create', 'close', 'update'])]`; extend the adjacent comment (lines 79-80) to name all three Tier-2 scope keys, minted only by the metta-backlog skill. +- Scope keys auto-derive at lines 902-905 (`` `${sub}:${third}` `` for two-word blocked forms) — **no other guard logic changes**; the new verbs ride the existing Tier-2 machinery exactly as `milestone create` does. `milestone list`/`show` stay in `ALLOWED_TWO_WORD` (line 60), credential-free. +- Mirror the edit byte-identically into `src/templates/hooks/metta-guard-bash.mjs` (`tests/hooks-byte-identity.test.ts` pins the pair; a forgotten mirror fails CI — the desired tripwire). + +### 8. Mint hook pair (2 files, byte-identical edit) + +- `.claude/hooks/metta-session-mint.mjs` line 35: append `'milestone:close', 'milestone:update'` to `SKILL_SCOPES['metta-backlog']`. +- Mirror byte-identically into `src/templates/hooks/metta-session-mint.mjs`. + +### 9. Skill pair (2 files, same edit) + +- `.claude/skills/metta-backlog/SKILL.md` line 28: milestone action choices `create | list | show` → `create | list | show | close | update`, plus two dispatch branches: + - **close** → run `metta milestone list --json` (allow-listed; completes a prior mint cycle), present open-milestone slugs via `AskUserQuestion`, ask whether the milestone was achieved (`closed`) or dropped (`abandoned`), then run `metta milestone close ` (append `--abandoned` for dropped). Echo the resulting status and commit line. + - **update** → present milestone slugs the same way, collect which fields to change (`name` / `target` / clear target / `description` / `status`) via `AskUserQuestion`, then run `metta milestone update ` with only the matching flags. Note in the branch that `--status open` is the reopen path. +- Same edit in `src/templates/skills/metta-backlog/SKILL.md` (templates are copied to `dist/` at build time per project conventions; keep both copies in sync — no byte-identity test pins skills, so this is a review checklist item, and `tests/skill-template-anchoring.test.ts` lints both trees). + +## Data Model + +### Milestone file (on disk, `spec/milestones/.md`) — schema change only + +```yaml +--- +name: # required +target: # optional; key absent when unset/cleared (never null) +status: open | closed | abandoned # default open ← enum gains 'abandoned' +--- + +``` + +- **Backward compatible reads:** every existing file carries `open` or `closed`; both stay valid; `.default('open')` unchanged. Spec scenario "Pre-existing open and closed files are unaffected" holds structurally. +- **Forward-compat caveat (accepted in intent.md §Impact):** a file written with `status: abandoned` fails validation under older metta builds — a one-way door once any milestone is abandoned. +- **Normalization (accepted, research §7):** `update` re-serializes the full frontmatter via `YAML.stringify` (unlike the issue store's minimal-diff Document API). Files written by `milestone create` round-trip stably; hand-edited files (reordered keys, comments) are normalized on first update. Acceptable — milestone frontmatter is three metta-owned keys and hand-editing is the workflow this change eliminates. `formatMilestone`/`parseMilestone` both trim the body, so body normalization is limited to leading/trailing whitespace. + +### In-memory types + +```ts +// milestones-store.ts +interface Milestone { + slug: string + name: string + target?: string + status: MilestoneFrontmatter['status'] // was 'open' | 'closed' literal union + description: string +} + +export interface MilestonePatch { + name?: string + target?: string // set or change + clearTarget?: boolean // remove the key entirely; mutually exclusive with target + status?: Milestone['status'] + description?: string // full body replacement +} + +// milestone-rollup.ts +interface MilestoneRollup { status: Milestone['status'] /* … unchanged fields … */ } +export const MILESTONE_MARKERS = { open: '▸', closed: '✓', abandoned: '✗' } as const +``` + +Design decision (ADR-style, from research §2): **`clearTarget: boolean` rather than `target: string | null`.** Rationale: `null` would leak a YAML-serialization concern into the type and fight `YAML.stringify`'s `keepUndefined: false` behavior; a boolean maps 1:1 to the `--clear-target` CLI flag and keeps the patch type free of null unions. The sibling issue patch type never needed field removal, so there is no precedent conflict. + +### Session-tier scope keys (guard/mint hooks) + +New Tier-2 scope keys `milestone:close` and `milestone:update`, auto-derived by the guard from `BLOCKED_TWO_WORD`, minted only into `metta-backlog` tokens. No change to token file format, TTL/GRACE policy, or the two-tier trust model. + +## API Design + +### Store + +```ts +// src/milestones/milestones-store.ts +async update(slug: string, patch: MilestonePatch): Promise +``` + +Errors (all plain `Error`, matching the store's existing error style — no new error classes; the CLI maps messages, exactly as it does for `show`): + +| Condition | Message | CLI mapping | +|---|---|---| +| unsafe slug | existing `assertSafeSlug` text | `milestone_error`, exit 4 | +| `target` + `clearTarget` both set | `clearTarget and target are mutually exclusive` | unreachable from CLI (`.conflicts()`); programmer error | +| no file | `Milestone '' not found` | `not_found`, exit 4 | +| patched frontmatter invalid | `Invalid milestone frontmatter in :\n - : …` (via `formatZodError`) | `milestone_error`, exit 4; names the offending field | + +### CLI: `metta milestone close ` + +```ts +milestone + .command('close') + .argument('', 'Milestone slug') + .option('--abandoned', 'Mark abandoned instead of closed') + .option('--on-branch ', 'Acknowledge non-main branch and proceed') + .description('Close (or abandon) an open milestone') +``` + +Action flow (clones `create`'s structure): +1. `json = program.opts().json`; `createCliContext()`; load config; `assertOnMainBranch(ctx.projectRoot, config.git?.pr_base ?? 'main', options.onBranch)`. +2. `current = await ctx.milestonesStore.show(slug)` — not-found propagates to the catch. +3. Conflict: `current.status !== 'open'` → exit 4, `type: 'milestone_conflict'`, message `` `Milestone '${slug}' is already ${current.status}` `` (names the current status per spec). No store call — file untouched. +4. `await ctx.milestonesStore.update(slug, { status: options.abandoned ? 'abandoned' : 'closed' })`. +5. `commitMilestones(ctx.projectRoot, `chore: close milestone ${slug}`)` — one message for both closed and abandoned (spec pins this for the `--abandoned` scenario too). +6. Output — JSON: `{ slug, status, committed, commit_sha }`; text: `Closed milestone: ` or `Abandoned milestone: ` plus ` Committed: ` when committed (matches `create` conventions). +7. Catch: `type = message.startsWith('Refusing to write') ? 'branch_guard' : message.includes('not found') ? 'not_found' : 'milestone_error'`; envelope `{ error: { code: 4, type, message } }` under `--json`, plain stderr otherwise; `process.exit(4)`. + +### CLI: `metta milestone update ` + +```ts +milestone + .command('update') + .argument('', 'Milestone slug') + .option('--name ', 'Rename display name') + .addOption(new Option('--target ', 'Set or change target date (YYYY-MM-DD)').conflicts('clearTarget')) + .option('--clear-target', 'Remove the target date') + .option('--description ', 'Replace the description body') + .addOption(new Option('--status ', 'Set status explicitly (reopen with --status open)').choices(['open', 'closed', 'abandoned'])) + .option('--on-branch ', 'Acknowledge non-main branch and proceed') + .description('Edit milestone fields (name, target, description, status)') +``` + +**Resolved:** Commander `.conflicts()` and `Option.choices()` are used — `package.json:37` pins `commander: ^13.1.0`; both APIs exist since Commander 9. No manual mutual-exclusion check needed; Commander exits non-zero with its standard conflict message. `.choices()` gives friendly rejection of bad `--status` values; Zod remains the authoritative write gate (defense in depth). + +Action flow: +1. Same context / config / branch-guard preamble as `close`. +2. "No field options" pre-check: none of `name`, `target`, `clearTarget`, `description`, `status` present → exit 4, `type: 'milestone_error'`, message `At least one field option is required (--name, --target, --clear-target, --description, --status)`. No store call. +3. Build `MilestonePatch` from provided options only (spread conditionally — absent options never appear in the patch, so untouched fields are preserved by the store). +4. `await ctx.milestonesStore.update(slug, patch)` — validation failures and not-found propagate to the same catch mapping as `close`. +5. `commitMilestones(ctx.projectRoot, `chore: update milestone ${slug}`)`. +6. Output — JSON: `{ slug, changed, committed, commit_sha }` where `changed` is the ordered list of patched field names (e.g. `['target', 'status']`; `clearTarget` reports as `'target'` — the field that changed); text: `Updated milestone: ()` plus committed line. + +### Shared helper + +```ts +// src/cli/commands/milestone.ts (module-private) +async function commitMilestones( + projectRoot: string, + message: string, +): Promise<{ committed: boolean; commitSha?: string }> +``` + +Same `git add spec/milestones` → `commit -m ` → `rev-parse HEAD` sequence and swallow-on-failure semantics as `create` today; `create` refactors onto it. + +### Rollup comparator (exact replacement, `milestone-rollup.ts:77-80`) + +```ts +const rank = (s: Milestone['status']): number => (s === 'open' ? 0 : 1) +rollups.sort((a, b) => + rank(a.status) - rank(b.status) || + (a.slug < b.slug ? -1 : a.slug > b.slug ? 1 : 0)) +``` + +### Exit codes / envelope summary + +All failure paths on the new verbs use exit code 4 with the standard envelope `{ error: { code: 4, type, message } }` under `--json` (types: `branch_guard`, `not_found`, `milestone_conflict`, `milestone_error`), plain stderr message otherwise — byte-consistent with `create`/`show`. Successful commands exit 0. Commander-level option errors (`.conflicts()`, `.choices()`) exit with Commander's own non-zero code, matching every other subcommand's behavior for bad flags. + +## Dependencies + +**External (all already installed — no additions, no version bumps, no lock-in):** +- `commander ^13.1.0` — `.command`/`.argument`/`.option`/`.addOption`, `Option.choices()`, `Option.conflicts()`. +- `zod` — `MilestoneFrontmatterSchema` enum extension; `safeParse` via existing `validateFrontmatter`. +- `yaml` — `YAML.stringify` with default `keepUndefined: false` (the clear-target mechanism) via existing `formatMilestone`. +- `vitest` + existing test helpers (`tests/helpers/cli.js`: `runCli`, `installFixture`, `execAsync`). + +**Internal:** +- `StateStore` (`readRaw`/`writeRaw`/`exists`) — unchanged, consumed by `update`. +- `src/cli/helpers.ts` — `assertOnMainBranch`, `createCliContext`, `outputJson`, `getErrorMessage` — unchanged, consumed by both new verbs. +- `formatZodError`, `assertSafeSlug` — unchanged. +- `loadMilestoneRollups` / `toMilestoneCountsRow` — unchanged; `status`/`progress` inherit `abandoned` through them. +- Guard/mint hooks and their `src/templates/hooks/` mirrors; `metta-backlog` skill and its `src/templates/skills/` mirror. + +**Complete file list (12 source + 5 test files to extend):** + +| # | File | Edit | +|---|---|---| +| 1 | `src/schemas/milestone-frontmatter.ts` | enum + `abandoned` | +| 2 | `src/milestones/milestones-store.ts` | `MilestonePatch`, `update()`, derived status type | +| 3 | `src/milestones/milestone-rollup.ts` | status type, rank comparator, `MILESTONE_MARKERS` | +| 4 | `src/cli/commands/milestone.ts` | `commitMilestones`, `close`, `update`, marker lookup, `Option` import | +| 5 | `src/cli/commands/status.ts` | marker lookup (line 33) | +| 6 | `src/cli/commands/progress.ts` | marker lookup (line 217) | +| 7 | `.claude/hooks/metta-guard-bash.mjs` | `BLOCKED_TWO_WORD` milestone set + comment | +| 8 | `src/templates/hooks/metta-guard-bash.mjs` | byte-identical mirror of 7 | +| 9 | `.claude/hooks/metta-session-mint.mjs` | `SKILL_SCOPES['metta-backlog']` + 2 keys | +| 10 | `src/templates/hooks/metta-session-mint.mjs` | byte-identical mirror of 9 | +| 11 | `.claude/skills/metta-backlog/SKILL.md` | `close`/`update` branches | +| 12 | `src/templates/skills/metta-backlog/SKILL.md` | mirror of 11 | + +**Test plan (extends existing files — near-1:1 ratio holds; no new test files required):** + +| Test file | New cases | +|---|---| +| `tests/milestones-store.test.ts` | `update` status patch preserves name/target/body; `clearTarget` removes the key (assert raw file content has no `target:` line); invalid patch (`target: '2026-02-30'`, empty `--name` equivalent) throws naming the field AND file byte-identical to a pre-call `readFile` snapshot (`seedMilestoneFile` helper); `target`+`clearTarget` throws; not-found throws and `milestones/` gains no file; `abandoned` round-trips through `show`; seeded `status: abandoned` file validates; seeded `status: shipped` rejects naming allowed values; seeded pre-change `open`/`closed` files parse identically (back-compat pin) | +| `tests/cli-milestone.test.ts` | `close` happy path (frontmatter `status: closed`, `git log` contains `chore: close milestone `, JSON `{ slug, status, committed, commit_sha }`); `close --abandoned` writes `abandoned` with same commit message; close on already-closed → exit 4 + `milestone_conflict` envelope naming current status + byte-identical file; close on missing slug → exit 4 `not_found`, no file created; branch-guard refusal without `--on-branch`; `update --description` replaces body only; `--clear-target` removes key; `--status open` reopens a closed milestone; `--target 2026-02-30 --json` → exit 4, envelope names `target`, byte-identical file; update missing slug → `not_found`; zero field options → exit 4, file untouched; `--target` + `--clear-target` together rejected by Commander | +| `tests/milestone-rollup.test.ts` | mixed open/closed/abandoned sorts open-first then terminal slug-ascending; open/closed-only ordering unchanged (byte-compat pin); `abandoned` passes through the rollup row; `MILESTONE_MARKERS` covers all three statuses | +| `tests/cli-status.test.ts` + a progress test file (`tests/progress-secondary-line.test.ts` or sibling) | one case each: abandoned milestone renders exit 0 with `✗`, sorted after open; existing open/closed assertions double as byte-compat pins | +| `tests/metta-guard-bash.test.ts` | `metta milestone close x` / `metta milestone update x` blocked without credential (exit 2, mirroring the `backlog add` case); allowed with a minted metta-backlog token covering `milestone:close`/`milestone:update` (token-fixture helpers already present); `milestone list`/`show` still allowed credential-free. Note: file currently has **zero** milestone-specific cases (research §1.6) — this closes that gap | + +`tests/hooks-byte-identity.test.ts` and `tests/skill-template-anchoring.test.ts` need no edits — they automatically pin/lint the mirrored files. `tests/metta-guard-mint-seam.test.ts` does not enumerate `metta-backlog` scopes (verified by grep) — no edit needed. + +## Risks & Mitigations + +1. **Forgotten hook template mirror** (files 8/10) — Mitigation: `tests/hooks-byte-identity.test.ts` fails CI on any divergence; the tripwire is pre-existing and requires no new work. +2. **Rollup comparator regression** — the only behavioral rewrite of existing logic. Mitigation: rank comparator is provably identical to the old comparator for two-state inputs (open→0, closed→1 reproduces `a.status === 'open' ? -1 : 1`); byte-compat pin tests in `milestone-rollup`, `cli-milestone`, status, and progress suites. +3. **Skill template drift** (files 11/12) — no byte-identity test pins `.claude/skills/` against `src/templates/skills/`. Mitigation: both files are in the explicit file list above as a paired task; the anchoring lint covers both trees for path hygiene. (Flag for a possible follow-up: a skills byte-identity test analogous to the hooks one.) +4. **Marker glyph rendering** — `✗` (U+2717) is the same width class as `✓` (U+2713); Mitigation: text-mode assertion in the CLI list test verifies column alignment; glyph (not just color) differs, so no-color terminals still distinguish states. +5. **`update --status closed` bypasses `close`'s conflict check** — intentional and symmetric with the reopen scenario (spec US-2): `close` is the guarded transition, `update` is the explicit override verb. Mitigation: documented in both command descriptions. +6. **Forward-compat one-way door** — `status: abandoned` fails validation under older metta builds. Accepted in `intent.md` §Impact; no mitigation beyond documentation. +7. **Frontmatter normalization on first update of hand-edited files** — accepted (research §7): three metta-owned keys, and hand-editing is the workflow being eliminated. Not worth importing the issue store's Document-API minimal-diff machinery. +8. **TOCTOU between the CLI's `show` and the store's `update` re-read in `close`** — single-user, single-process, local files; the store re-validates on its own read. Consistent with every existing metta command. Accepted. +9. **Future programmatic closers** — only `metta-backlog` mints `milestone:close`/`milestone:update`; a future ship/finalize flow wanting to close milestones needs its own scope extension. Mitigation: comment at the `SKILL_SCOPES` line noting the mint boundary; out of scope now. +10. **Vendor lock-in** — none: no new dependencies, no external services; Commander `.conflicts()`/`.choices()` usage is already the repo-wide pattern for option validation. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/gates.yaml b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/gates.yaml new file mode 100644 index 00000000..b117116a --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/gates.yaml @@ -0,0 +1,18 @@ +finalized_at: 2026-08-26T01:06:03.092Z +all_passed: true +results: + - gate: stories-valid + status: pass + duration_ms: 554 + - gate: tests + status: pass + duration_ms: 532858 + - gate: lint + status: pass + duration_ms: 5311 + - gate: typecheck + status: pass + duration_ms: 5658 + - gate: build + status: pass + duration_ms: 6899 diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/intent.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/intent.md new file mode 100644 index 00000000..044e4d86 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/intent.md @@ -0,0 +1,49 @@ +# fix-milestone-status-write-once-dead-field-no-close + +## Problem + +Milestone status is a write-once dead field. `metta milestone create` writes `status: open` (plus `name`, optional `target`, and a free-form description body) to `spec/milestones/.md`, and no CLI path can ever change any of it afterward: + +- `MilestonesStore` (`src/milestones/milestones-store.ts`) exposes only `create`, `list`, `show`, and `exists` — there is no update/write-back method, and `create` explicitly throws if the file already exists. +- `registerMilestoneCommand` (`src/cli/commands/milestone.ts`) wires only `create`, `list`, and `show` subcommands — no user-facing verb can transition status, edit the body, or change/clear the target. +- Meanwhile the read path fully supports a lifecycle the write path never implements: `MilestoneFrontmatterSchema` models `status: z.enum(['open', 'closed'])`, `milestone list` renders a `✓` marker for closed milestones, and the rollup sort orders closed milestones last. The `closed` state is modeled, rendered, and unreachable. + +Who is affected: anyone using milestones as a system of record — reported concretely by the zeus session (2026-08-26), where milestone `m1-real-trade-exit-correctness` has all attached issues resolved yet permanently reports `status: open` with a body still reading "In flight as PR #24", and `m6`'s body still lists a prerequisite that is now satisfied. The only workaround is hand-editing YAML frontmatter, which bypasses Zod validation and auto-commit entirely — violating the project's "no unvalidated state writes" constraint. Downstream, `metta status` and `metta progress` surface these rollups, so the dashboards lie about shipped work. + +## Proposal + +Give milestones a real lifecycle by adding a validated update path — candidate solution 1 from the issue, matching the sibling `IssuesStore` resolve pattern: + +1. **Store: `MilestonesStore.update(slug, patch)`** — reads the existing milestone file, applies a patch (status transition, name change, description replacement, target set/change/clear), re-validates the resulting frontmatter through `MilestoneFrontmatterSchema` before write, and writes via `StateStore` like `create` does. Fails with a clear error when the milestone does not exist. No unvalidated state ever reaches disk. + +2. **Schema: extend the status enum with `abandoned`** — `z.enum(['open', 'closed', 'abandoned'])` in `src/schemas/milestone-frontmatter.ts`, so milestones that are dropped (rather than achieved) are representable and distinguishable from completed ones. The store's `Milestone` interface status union is updated to match. + +3. **CLI: `metta milestone close `** — transitions `open → closed` (or `abandoned` via a flag such as `--abandoned`). Idempotency/conflict behavior (already-closed) reports a clear error with the standard JSON error envelope, consistent with existing milestone subcommands. Auto-commits with a conventional `chore:` message following the `create` pattern, respecting the same main-branch guard (`--on-branch` acknowledgment). + +4. **CLI: `metta milestone update `** — edits mutable fields via options (e.g. `--name`, `--target`, `--clear-target`, `--description`), including reopening or otherwise setting status where explicitly requested. Same validation, branch-guard, auto-commit, and `--json` output conventions as the other milestone subcommands. + +5. **Renderers: handle `abandoned`** — `milestone list` marker and rollup ordering (`computeMilestoneRollups` sort), `milestone show` status line, and the `status`/`progress` milestone sections render the new state sensibly instead of crashing or mislabeling. + +6. **Guard allow-list** — the `metta-guard-bash` hook's command allow-list is extended so the new `milestone close` / `milestone update` verbs are invocable from authorized skill contexts, consistent with how existing milestone verbs are authorized. + +7. **Tests** — store, schema, and CLI test coverage for the new paths, maintaining the near 1:1 test-to-source ratio (update semantics, validation rejection, not-found errors, status transitions including `abandoned`, target clearing, renderer output for the new state). + +## Impact + +- `src/milestones/milestones-store.ts` — new `update` method; `Milestone.status` type widens to include `'abandoned'`. Existing `create`/`list`/`show`/`exists` behavior is unchanged. +- `src/schemas/milestone-frontmatter.ts` — status enum gains `abandoned`. Backward compatible for reads: all existing files carry `open` or `closed`, both still valid. Forward compatibility caveat: files written with `status: abandoned` will fail validation under older builds of metta. +- `src/cli/commands/milestone.ts` — two new subcommands; existing subcommand output (human and `--json`) is unchanged for the `open`/`closed` states. `list`/`show`/rollup rendering gains handling for `abandoned`. +- `src/milestones/milestone-rollup.ts` — sort/marker logic accounts for `abandoned` (grouped with terminal states, after open milestones). +- `metta status` / `metta progress` — milestone sections inherit the new state through the shared rollup path; existing output for `open`/`closed` milestones stays byte-compatible. +- Guard hook allow-list — gains the two new milestone verbs; no change to the trust-tier model. +- Existing milestone files on disk — untouched by this change itself; users can now close stale milestones (e.g. `m1-real-trade-exit-correctness`) through a validated, auto-committed path instead of hand-editing YAML. + +## Out of Scope + +- **Automatic status derivation from issue rollups** (candidate 2) — closing remains an explicit human/orchestrator decision; a milestone with all issues resolved is not auto-closed, because issue counts do not necessarily represent the milestone's full scope. +- **"Eligible for close" advisory warnings** in `list`/`show`/`status`/`progress` (part of candidate 3) — a possible follow-up, not part of this fix. +- **Milestone deletion or renaming (slug changes)** — `update` patches fields within an existing file; it does not move or remove files. +- **Reassigning issues between milestones** — issue-side `milestone:` frontmatter editing stays with the issue tooling. +- **Closed/abandoned-milestone archival** — terminal milestones remain in `spec/milestones/`; no archive move is introduced. +- **Interactive editor flows** (e.g. `$EDITOR`-based body editing) — updates are option-driven only. +- **Changes to milestone attachment semantics or rollup counting rules** — `computeMilestoneRollups` counting logic is untouched beyond ordering/marker support for the new state. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-close-only.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-close-only.md new file mode 100644 index 00000000..6895d6a0 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-close-only.md @@ -0,0 +1,87 @@ +# Research: Minimal close-only approach + +Approach under evaluation: implement only `metta milestone close ` (validated status flip + auto-commit), plus an "eligible for close" advisory in `list`/`show`/rollups. No body/target/name editing, no `abandoned` state, no generic `update` verb. + +## 1. Current state (evidence) + +- `MilestonesStore` exposes only `create` / `list` / `show` / `exists`; `create` throws if the file exists (`src/milestones/milestones-store.ts:87-142`). There is no write-back path of any kind. +- The read path already round-trips the full file: `parseMilestone` (`milestones-store.ts:36-64`) validates frontmatter via `MilestoneFrontmatterSchema` and extracts the body; `formatMilestone` (`milestones-store.ts:66-72`) re-serializes frontmatter with `YAML.stringify` and omits an absent `target`. A close operation can be composed entirely from these two existing helpers. +- Schema: `status: z.enum(['open', 'closed']).default('open')` (`src/schemas/milestone-frontmatter.ts:26`). Close-only needs **zero schema changes** — `closed` is already modeled. +- CLI: `registerMilestoneCommand` wires `create`/`list`/`show` (`src/cli/commands/milestone.ts:44-188`). The `create` action (`milestone.ts:57-101`) is the exact template for `close`: branch guard via `assertOnMainBranch` (`milestone.ts:63`), exit-4 JSON error envelope (`milestone.ts:67, 98`), swallowed-git auto-commit block (`milestone.ts:77-87`), `--json` success object (`milestone.ts:89-90`). +- Rollups: `computeMilestoneRollups` (`src/milestones/milestone-rollup.ts:25-83`) is pure; sort already orders `closed` last (`milestone-rollup.ts:77-80`); `list` already renders `✓` for closed (`milestone.ts:121`). The `MilestoneRollup.status` union `'open' | 'closed'` (`milestone-rollup.ts:7`) needs no change. +- Sibling precedent for a narrow mutation: `IssuesStore.updateFrontmatter(slug, patch)` — exists-check, read, patch, idempotence short-circuit, validated write (`src/issues/issues-store.ts:223-234`). A `MilestonesStore.close()` following this shape is fully idiomatic. +- Guard hook: read-only verbs allow-listed at `.claude/hooks/metta-guard-bash.mjs:60` (`['milestone', new Set(['list', 'show'])]`); Tier-2 mutating scope at `metta-guard-bash.mjs:81` (`['milestone', new Set(['create'])]`). Close-only means adding `'close'` to that one set. + +## 2. Minimal store change + +Two viable shapes: + +**Option A — dedicated `close(slug)`** (~20-25 LOC): +reads via `state.readRaw`, `parseMilestone` (validates on read), throws not-found if missing, throws conflict if `status !== 'open'`, writes `formatMilestone({ ...frontmatter, status: 'closed' }, description)` after a `MilestoneFrontmatterSchema` re-validate. Pros: intent-revealing, conflict rule lives in the store, nothing speculative. Cons: a follow-up `update` change later duplicates the read-modify-validate-write skeleton. + +**Option B — narrow `update(slug, { status })`** restricted to status: same LOC, generic name. Pros: the eventual full `update(slug, patch)` (spec.md's ADDED store requirement) grows out of it without a rename. Cons: a method named `update` that only accepts `status` is a misleading API surface until the follow-up lands. + +For a genuinely close-only scope, **Option A** is the cleaner store change; it mirrors how `IssuesStore` separates `archive`/`remove` verbs from `updateFrontmatter`. + +Caveat worth recording: `parseMilestone` trims the body (`milestones-store.ts:55`) and `formatMilestone` re-serializes frontmatter through `YAML.stringify`, so a close of a *hand-edited* file may not be byte-preserving outside the status field (key order/quoting normalization). Files created by `metta milestone create` round-trip cleanly since they use the same formatter. Acceptable, but the spec's "retains the identical description body" scenario should be tested against store-created files. + +## 3. CLI wiring cost + +One new subcommand, ~50-60 LOC, near-verbatim clone of the `create` action (`milestone.ts:57-101`): +- `assertOnMainBranch` + `--on-branch` acknowledgment (same as `milestone.ts:63`). +- Not-found and already-closed → exit 4 with `{ error: { code: 4, type, message } }` under `--json` (`type: 'not_found'` matching `show`'s mapping at `milestone.ts:183`, plus a conflict type, e.g. `milestone_conflict`). +- Auto-commit `chore: close milestone ` with swallowed git failure, reported via `committed`/`commit_sha` (pattern at `milestone.ts:77-94`). + +Plus one line in the guard hook (`metta-guard-bash.mjs:81`) and its integration test (`tests/cli-metta-guard-bash-integration.test.ts` exists). Tests extend `tests/cli-milestone.test.ts` and a milestones-store test — the 1:1 test ratio is maintained with no new test files. + +Total estimated delta: ~80-100 src LOC, ~150-200 test LOC. Lowest-cost option of the candidates by a wide margin. + +## 4. The "eligible for close" advisory + +Mechanically cheap: `computeMilestoneRollups` already has `status`, `open`, `resolved`, `total` per rollup (`milestone-rollup.ts:71-75`). Eligibility is `status === 'open' && total > 0 && open === 0`. Two placement options: + +- **Pure field**: add `eligible: boolean` to `MilestoneRollup` and let renderers consume it. Cleanest (functional core), but it flows into `toMilestoneCountsRow` (`milestone.ts:39-42`) and therefore into `milestone list --json` **and** the `milestones` JSON keys of `status`/`progress` (`src/cli/commands/status.ts:16-23`, `src/cli/commands/progress.ts:143-146`) — a JSON shape change on three surfaces. +- **Render-side computation**: compute the flag inline in `list`/`show` text rendering only, e.g. `list` appends `— all issues resolved; run: metta milestone close ` and `show` adds an advisory line after `Progress:` (`milestone.ts:168`). Smaller blast radius; JSON untouched. + +**Conflict, either way:** the advisory is explicitly *out of scope* in this change's own intent (`intent.md:44` — "'Eligible for close' advisory warnings … a possible follow-up, not part of this fix"), and spec.md's renderer requirement demands **byte-identical** human and `--json` output for `milestone list`/`show`/`status`/`progress` when only `open`/`closed` milestones exist (spec.md, "Renderers and rollups handle the abandoned state", incl. the "Open and closed output stays byte-compatible" scenario). Any advisory shown for an open-with-all-resolved milestone breaks that scenario — exactly the zeus `m1` case that motivates the change. Adopting this approach therefore requires rewriting both intent.md (pull the advisory in, drop `update`/`abandoned`) and spec.md (drop/relax the byte-compat scenario for eligible milestones). It cannot be bolted onto the current artifacts. + +## 5. What stays broken + +- **Stale bodies remain stale.** `m1-real-trade-exit-correctness` can be closed, but its body still reads "In flight as PR #24" (intent.md:11); `m6`'s satisfied-prerequisite text stays wrong. The only body-edit path remains hand-editing YAML — the exact "unvalidated state write" violation this change was filed to eliminate. Close-only fixes the status half of the reported problem and leaves the body half untouched. +- **No `abandoned` state.** Dropped milestones must either sit `open` forever (dashboard lies persist) or be mislabeled `closed` (semantically wrong: "achieved" vs "dropped"). Schema stays `z.enum(['open','closed'])`. +- **No target/name edits.** A slipped target date is uncorrectable except by hand-edit. +- **Close is irreversible via CLI.** With no `update --status open`, a mistaken close can only be undone by hand-editing frontmatter or `git revert` of the auto-commit. `create` can't help — it refuses existing files (`milestones-store.ts:91-93`). This is a real operational sharp edge for a one-way verb. + +## 6. Follow-up work implied + +A near-certain second change carrying: `MilestonesStore.update(slug, patch)`, `milestone update` CLI verb (name/target/clear-target/description/status incl. reopen), the `abandoned` enum value + renderer/rollup handling, and the `milestone update` guard entry. That follow-up re-touches every file this change touches (store, CLI, rollup, guard, all four test files) — i.e. the split roughly doubles review/ship overhead versus doing it once, with the interim window leaving hand-editing as the only body/target path. + +## 7. Does it satisfy this change's spec.md? + +No — it misses most of it. spec.md defines 1 MODIFIED + 5 ADDED requirements: + +| Requirement (spec.md) | Close-only coverage | +|---|---| +| MODIFIED: store + CLI surface (`abandoned` enum, `close` **and** `update` verbs, guard tiers for both) | **Partial** — `close` verb and its guard entry only; no `abandoned`, no `update`. 3 of 5 scenarios unmet or inapplicable (abandoned-validates, invalid-status naming three allowed values, update registration). | +| ADDED: `update(slug, patch)` store method | **Missed entirely** — all 4 scenarios (field-preserving patch, clear-target, invalid-patch byte-identity, missing-slug). A `close()` covers only the status→closed slice of scenario 1. | +| ADDED: `milestone close` CLI verb | **Mostly satisfied** — 4 of 5 scenarios pass; the "Abandoned flag writes the abandoned state" scenario fails (no `--abandoned`). | +| ADDED: `milestone update` CLI verb | **Missed entirely** — all 6 scenarios, including the reopen scenario and the stale-body ("In flight as PR #24" → "Shipped in v0.5.0") scenario that traces directly to the reported problem. | +| ADDED: renderers/rollups handle `abandoned` | **Missed** — 3 of 4 scenarios inapplicable (no abandoned state exists); the 4th (byte-compat) is actively **violated** if the advisory ships. | +| ADDED: guard authorization for close **and** update | **Half** — `milestone close` only. | + +Score: 0 of 5 ADDED requirements fully satisfied; 1 nearly satisfied (close verb, minus `--abandoned`); 3 wholly or almost wholly missed; and the advisory feature contradicts both intent.md's Out of Scope list (`intent.md:44`) and spec.md's byte-compat scenario. + +## 8. Assessment + +**Pros** +- Smallest possible delta (~100 src LOC); one store method, one subcommand, one guard-set entry; zero schema change; zero rollup-type change. +- Fixes the single most-reported symptom (permanently-open completed milestones) through a fully validated, auto-committed path. +- Every piece follows an existing in-repo pattern (`create` action clone, `IssuesStore.updateFrontmatter` store shape). + +**Cons** +- Fails this change's spec.md as written — the change artifacts (intent, spec, stories) would need substantial rewriting to match the narrower scope, and the advisory piece directly contradicts the current intent's out-of-scope list. +- Leaves the body/target half of the reported defect (stale "In flight as PR #24") unfixed, with hand-editing (unvalidated writes) still the only recourse. +- Introduces an irreversible CLI operation (close with no reopen). +- The implied follow-up change re-touches the identical file set, roughly doubling total lifecycle cost. + +**Recommendation:** do not adopt close-only as scoped here. If minimalism is the goal, the defensible minimal cut is `close` **plus** `--abandoned` and the enum extension (cheap: one enum literal, one flag, marker/sort tweaks) while deferring only `milestone update` — that keeps 3 of 5 ADDED requirements and avoids the mislabel-dropped-milestones trap. If close-only is nonetheless chosen, drop the advisory (it conflicts with intent.md's own out-of-scope list and the byte-compat scenario) and re-scope spec.md before execution, accepting the follow-up change for `update`/body editing as committed debt. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-derived-status.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-derived-status.md new file mode 100644 index 00000000..8142d619 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-derived-status.md @@ -0,0 +1,67 @@ +# Research: Derived milestone status from issue rollups + +**Approach under evaluation:** compute milestone open/closed automatically from issue rollups — a milestone whose attached issues are all resolved renders as closed. No explicit close verb, no store write path. + +**Verdict up front:** this approach is cheap and self-maintaining for the narrow "dashboard lies" symptom, but it cannot satisfy the change's spec.md. It fails or leaves unmet all six requirement blocks (quantified in §6), cannot represent `abandoned`, does nothing for stale body text, and turns the persisted `status` field into a second, conflicting source of truth. It is also explicitly listed as out of scope in the change's own intent (`intent.md:43`). + +--- + +## 1. Where derivation would live + +The natural home is the existing pure rollup function — the single choke point every renderer already flows through: + +- `src/milestones/milestone-rollup.ts:25-83` — `computeMilestoneRollups(milestones, openIssues, resolvedIssues)` buckets issues per milestone in one pass and already computes `open`, `resolved`, `total`, `percent` per milestone. Derivation is one extra line per rollup, e.g. `derivedStatus = total > 0 && open === 0 ? 'closed' : milestone.status`. +- All four rendering surfaces consume it through one wiring helper, so a change here propagates everywhere automatically: + - `src/cli/commands/milestone.ts:22-32` — `loadMilestoneRollups`, shared by all surfaces. + - `src/cli/commands/milestone.ts:121` — `list` marker: `r.status === 'closed' ? '✓' : '▸'`. + - `src/cli/commands/milestone.ts:154,166` — `show` JSON `status` field and human `Status:` line (note: the human line reads `item.status` from the store directly, not the rollup, so `show` would need a second touch point). + - `src/cli/commands/status.ts:5,62` and `src/cli/commands/progress.ts:9,110` — status/progress milestone sections reuse the same helper. +- Sort order already keys off status (`milestone-rollup.ts:77-80`); a derived value would slot in without structural change. + +Implementation cost is genuinely small: ~10 lines in the rollup, one field rename (`status` → `derivedStatus` or a precedence rule), no store changes, no CLI verbs, no guard allow-list changes (`.claude/hooks/metta-guard-bash.mjs:60` stays as-is). + +**Pros:** zero manual toil; dashboards can never drift from issue reality; no new mutating surface, so no new guard tier work; no state writes at all, trivially satisfying "no unvalidated state writes." + +## 2. Zero-issue milestones and scope beyond issues (premature auto-close) + +This is the approach's central semantic flaw, in two forms: + +- **Zero attached issues.** `computeMilestoneRollups` documents and implements `Milestones with zero issues roll up 0/0/0 at 0%` (`milestone-rollup.ts:23`, `:74`). Under "all attached issues resolved ⇒ closed," the vacuous-truth reading auto-closes every freshly created milestone before any work is attached — a newly created milestone (`MilestonesStore.create` writes `status: open` by default, `milestones-store.ts:95-106`, schema default at `src/schemas/milestone-frontmatter.ts:26`) would render `✓` immediately. The only fix is a special case ("zero issues ⇒ open"), which then makes milestones whose scope is *not* issue-tracked permanently unclosable — the exact write-once dead-field bug this change exists to fix, reproduced under a different rule. +- **Scope beyond issues.** Issue attachment is a one-way optional pointer: `spec/issues/*.md` frontmatter carries `milestone: ` (`src/schemas/issue-frontmatter.ts:12`, surfaced via `issues-store.ts:32,101` and settable on create/update at `issues-store.ts:153,189-208`). Nothing forces a milestone's full scope to be represented as issues — the milestone body is free-form prose that can name PRs, prerequisites, or non-issue work (the zeus report's `m6` lists a prose prerequisite). Auto-closing when the *issue subset* completes closes milestones whose real scope is unfinished. The intent already adjudicated this: "a milestone with all issues resolved is not auto-closed, because issue counts do not necessarily represent the milestone's full scope" (`intent.md:43`). +- **Non-monotonicity.** Derived status is not a state transition, it is a live function of the issue set: attaching a new open issue to a derived-closed milestone silently reopens it; resolving it re-closes it. There is no closure *event*, so nothing to auto-commit — the project treats git as the transaction log, and this approach records no transaction for the most meaningful lifecycle moment. + +## 3. Inability to represent `abandoned` + +Derivation has no input from which to infer abandonment. A milestone dropped with unresolved issues is indistinguishable from an active one — issue counts say "open" forever. The spec makes `abandoned` a hard requirement: the schema MUST model `open | closed | abandoned` (spec.md:5), with dedicated validation scenarios (spec.md:22-30), a `--abandoned` close flag (spec.md:60, 67-70), and renderer handling with a distinct third marker (spec.md:123-140). None of these are reachable without an explicit write path. A hybrid ("derive closed, persist abandoned") reintroduces the write path this approach exists to avoid — at which point you have built the explicit-verb approach anyway, plus derivation complexity on top. + +## 4. Stale body text remains uneditable + +The zeus report's second half — `m1`'s body still reading "In flight as PR #24", `m6` listing a satisfied prerequisite — is untouched by derivation. `MilestonesStore` still exposes only `create`/`list`/`show`/`exists` (`milestones-store.ts:80-143`); the only body-edit path remains hand-editing YAML frontmatter files, bypassing Zod validation and auto-commit, in direct violation of the "no unvalidated state writes" constraint the intent cites (`intent.md:11`). spec.md dedicates a full requirement to option-driven field editing including `--description`, `--name`, `--target`, `--clear-target`, and status reopening (spec.md:88-120); derivation addresses none of it. + +## 5. The persisted `status` field becomes vestigial and conflicting + +- Every milestone file on disk carries `status:` in validated frontmatter (`milestone-frontmatter.ts:26`, parsed at `milestones-store.ts:61`). If renderers switch to derived status, the persisted field is dead weight that *still validates and still disagrees* with what every command displays — `spec/milestones/m1.md` says `open` while `milestone list` shows `✓`. The system of record now lies in the opposite direction. +- **Precedence ambiguity is unavoidable.** Someone hand-edits `status: closed` (the current workaround) on a milestone with open issues: does persisted or derived win? Any answer creates a class of files whose displayed state cannot be explained by reading the file. +- **Removing the field is worse.** `MilestoneFrontmatterSchema` is `.strict()` (`milestone-frontmatter.ts:20-27`), so dropping `status` from the schema makes every existing file carrying `status: open`/`status: closed` fail validation on read — a breaking migration requiring a rewrite pass over `spec/milestones/`, contradicting the intent's "existing milestone files on disk — untouched by this change" stance (`intent.md:39`) and spec.md's back-compat scenario (spec.md:27-30). +- **Byte-compatibility breaks either way.** spec.md:125 requires that projects containing only `open`/`closed` milestones produce output "byte-identical to pre-change behavior" (scenario at spec.md:142-145). Flipping `m1` from `▸` to `✓` via derivation changes `list`, `show`, `status`, and `progress` output for existing data — the derived approach violates this requirement by design, since changing existing output *is* its mechanism. + +## 6. Requirements coverage — quantified + +spec.md contains six requirement blocks. Derived status satisfies **zero of six**: + +| # | Requirement (spec.md line) | Derived-status outcome | +|---|---|---| +| 1 | Schema enum `open\|closed\|abandoned` + `close`/`update` CLI verbs (spec.md:3-5) | **Fails** — no `abandoned`, no verbs | +| 2 | `MilestonesStore.update(slug, patch)` validated write path (spec.md:33-35) | **Fails** — no store write path at all | +| 3 | `metta milestone close ` with `--abandoned`, auto-commit, branch guard (spec.md:58-60) | **Fails** — verb explicitly not built | +| 4 | `metta milestone update ` field editing (spec.md:88-90) | **Fails** — no editing surface | +| 5 | Renderers handle `abandoned`; open/closed output byte-identical (spec.md:123-125) | **Fails** — `abandoned` unrepresentable; byte-compat scenario (spec.md:142-145) actively violated | +| 6 | Guard allow-list for `milestone close`/`milestone update` (spec.md:148-150) | **Vacuous/fails** — verbs it must authorize don't exist | + +It also contradicts the change's own scope decision (`intent.md:41-43`, Out of Scope item 1). + +## 7. Recommendation + +**Do not adopt derived status for this change.** It cannot satisfy any of the six spec.md requirements, cannot represent `abandoned`, leaves the body-editing half of the reported problem unsolved, breaks the byte-compatibility requirement, and converts the persisted `status` field from "dead" to "actively contradicted." The explicit lifecycle approach (store `update` + `close`/`update` verbs) that intent.md commits to is the correct fit. + +**Salvageable follow-up:** the derivation *signal* — `total > 0 && open === 0 && status === 'open'` — is cheap to compute inside `computeMilestoneRollups` and would make a good advisory ("eligible for close") in `list`/`show`/`status`/`progress`. That is exactly the deferred candidate-3 warning noted in `intent.md:44`, and it pairs well with the explicit close verb rather than replacing it. Not part of this change. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-store-update.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-store-update.md new file mode 100644 index 00000000..e52d9469 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research-store-update.md @@ -0,0 +1,239 @@ +# Research: Full store-level update (`MilestonesStore.update` + `milestone close` / `milestone update` CLI verbs + `abandoned` enum) + +Change: `fix-milestone-status-write-once-dead-field-no-close` +Approach under evaluation: **Option 1 — full store-level update mirroring the sibling `IssuesStore` write-back pattern.** + +All paths below are relative to the change root +`/home/utx0/Code/metta/.metta/worktrees/fix-milestone-status-write-once-dead-field-no-close/` unless absolute. + +--- + +## 1. Existing patterns scanned + +### 1.1 `MilestonesStore` today (`src/milestones/milestones-store.ts`) + +- Read-only after create: `create` (:87-107), `list` (:109-127), `show` (:129-137), `exists` (:139-142). No write-back path. +- `create` is the validate-before-write template this approach extends: builds a plain frontmatter object, runs it through `validateFrontmatter` (:26-34, wrapping `MilestoneFrontmatterSchema.safeParse` with `formatZodError`), then serializes via `formatMilestone` (:66-72) and writes with `StateStore.writeRaw` (:106). +- `formatMilestone` (:66-72) uses `YAML.stringify` with the yaml default `keepUndefined: false` — an absent `target` is *omitted* from the block, never serialized as `target: null`. This is exactly the mechanism `--clear-target` needs: delete the key from the patched object and the serializer drops it. +- `parseMilestone` (:36-64) already round-trips file → validated frontmatter + body — `update` can reuse it verbatim for the read half. +- Not-found error text precedent: `show` throws `` `Milestone '${slug}' not found` `` (:132-134); the CLI maps `message.includes('not found')` → JSON error type `not_found` (`src/cli/commands/milestone.ts:183`). + +### 1.2 The sibling `IssuesStore` write-back (`src/issues/issues-store.ts`) + +The pattern this approach mirrors: + +- `updateFrontmatter(slug, patch)` (:223-234): `assertSafeSlug` → `exists` check throwing `` `Issue '${slug}' not found` `` (:225-227) → `readRaw` → pure patch function (`applyFrontmatterPatch`) → byte-compare short-circuit (`if (patched === content) return { changed: false }`, :231) → `writeRaw`. Read → patch → validate → write, with the file untouched on any failure because validation happens inside the pure patch step *before* the write. +- `archive(slug, changeName?)` (:281-294) repeats the same exists-guard + readRaw + writeRaw shape. +- Auto-commit is **not** in the store — `IssuesStore` (and `MilestonesStore.create`) leave git to the CLI edge. `milestone create`'s commit block lives at `src/cli/commands/milestone.ts:77-87`: `git add spec/milestones` → `git commit -m 'chore: create milestone '` → `rev-parse HEAD`, with the whole block in a swallowing try/catch (`committed: false` reported when git is unavailable). This matches "functional core, imperative shell" and must be copied (or extracted) for the two new verbs. +- One deliberate divergence: issues patch frontmatter through the `yaml` Document API for minimal diffs (`src/issues/issue-frontmatter.ts` header comment, :6-24) because issue bodies/frontmatter may be hand-authored. Milestone frontmatter is a 3-key block always written by `formatMilestone`, so full re-serialization is fine (see §7 back-compat caveat). + +### 1.3 Schema (`src/schemas/milestone-frontmatter.ts`) + +- `status: z.enum(['open', 'closed']).default('open')` at :26 — the one-line enum extension point. `.strict()` object (:27) means unknown keys already fail loudly. +- `target` carries the regex + real-calendar-date refinement (:22-25); `update` re-validating the *full* patched frontmatter gets this for free (the spec's `target: '2026-02-30'` rejection scenario). + +### 1.4 Renderers / rollup + +- `src/milestones/milestone-rollup.ts:7` — `MilestoneRollup.status: 'open' | 'closed'` (duplicated literal union; should become `Milestone['status']`). +- Sort at :77-80: `if (a.status !== b.status) return a.status === 'open' ? -1 : 1` — correct for two states, **unstable for three** (`closed` vs `abandoned` comparison would order by whichever is `a`). Must become a rank comparator. +- Marker rendering is duplicated in three places, all `status === 'closed' ? '✓' : '▸'`: + - `src/cli/commands/milestone.ts:121` (`milestone list`, uncolored) + - `src/cli/commands/status.ts:33` (`printMilestoneSection`, `color('✓', 32)` / `color('▸', 36)`) + - `src/cli/commands/progress.ts:216` (same colored form) +- `milestone show` prints `Status: ${item.status}` verbatim (`milestone.ts:166`) and passes `rollup.status` through to JSON (`milestone.ts:154`) — both handle `abandoned` automatically once the type widens; no code change needed there. + +### 1.5 Guard hook + mint hook + +- `.claude/hooks/metta-guard-bash.mjs`: + - `ALLOWED_TWO_WORD` :60 — `['milestone', new Set(['list', 'show'])]` (read-only, no credential). + - `BLOCKED_TWO_WORD` :79-81 — `['milestone', new Set(['create'])]` with the comment "Tier-2 scope key 'milestone:create', minted only by the metta-backlog skill". + - Scope keys auto-derive at :902-905: a two-word blocked form produces `` `${sub}:${third}` `` — so adding `close`/`update` to the blocked set automatically yields scope keys `milestone:close` / `milestone:update`. No other guard logic changes. +- `.claude/hooks/metta-session-mint.mjs:35` — `SKILL_SCOPES['metta-backlog']` currently `['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create']`. +- **Byte-identity constraint:** `tests/hooks-byte-identity.test.ts` pins every `.claude/hooks/*.mjs` byte-identical to `src/templates/hooks/*.mjs`. Both copies of both hooks must be edited in lockstep or CI fails. +- The driving skill body also enumerates milestone actions: `.claude/skills/metta-backlog/SKILL.md:28` offers `create | list | show` — needs `close` and `update` branches so an authorized session can actually reach the new verbs. + +### 1.6 Test harnesses + +- `tests/milestones-store.test.ts` (136 lines): `mkdtemp(join(tmpdir(), 'metta-milestones-'))` per test, `seedMilestoneFile` helper writes raw file content directly (:20-23) — ideal for byte-snapshot assertions. +- `tests/cli-milestone.test.ts` (298 lines): `runCli` / `installFixture` / `execAsync` from `tests/helpers/cli.js`; `installFixture(tempDir)` provisions a git-initialized project so commit assertions work (`git log --format=%s` pattern at :63-65). +- `tests/milestone-rollup.test.ts` (148 lines): pure-function tests over `computeMilestoneRollups`. +- `tests/metta-guard-bash.test.ts` (1717 lines): `runHook` spawns the hook with a JSON event on stdin (:36-58); **zero** existing milestone-specific cases (verified by grep) — coverage today comes only from the generic two-word block/allow cases (`backlog add` block at :98, `backlog list` allow at :163). + +--- + +## 2. Proposed store API shape + +```ts +// src/milestones/milestones-store.ts +export interface MilestonePatch { + name?: string + target?: string // set or change + clearTarget?: boolean // remove the key entirely (mutually exclusive with target) + status?: Milestone['status'] + description?: string // full body replacement +} + +async update(slug: string, patch: MilestonePatch): Promise +``` + +Flow (read → patch → Zod re-validate → write; commit stays at the CLI edge): + +1. `assertSafeSlug(slug)` — same guard as every other method (:22-24). +2. Exists check on `join('milestones', `${slug}.md`)`; throw `` `Milestone '${slug}' not found` `` (mirrors `show` :132-134 and `IssuesStore.updateFrontmatter` :225-227). Never creates a file. +3. `readRaw` + `parseMilestone` — the current file is itself validated on read (a corrupt file fails here, before any write). +4. Build the next frontmatter object: + ```ts + const next: Record = { + name: patch.name ?? current.name, + ...(resolveTarget(patch, current)), // target key present iff a value survives + status: patch.status ?? current.status, + } + ``` + where `clearTarget: true` omits the key, `patch.target` replaces it, otherwise the current value (if any) carries through. `clearTarget` + `target` together is a programmer error — throw. +5. `validateFrontmatter(next, relPath)` — the **full** resulting frontmatter through `MilestoneFrontmatterSchema` before any I/O. A failing patch throws here; the file on disk is byte-identical by construction because nothing has been written. +6. `formatMilestone(validated, patch.description ?? current.description)` → `state.writeRaw`. `keepUndefined: false` guarantees a cleared target leaves no `target:` line (spec scenario "no `target` key (not `target: null`)"). +7. Return the updated `Milestone` (parsed shape) so the CLI can emit status/fields in JSON without a second read. + +Why a `clearTarget` boolean rather than `target: string | null`: the issue-side patch type (`IssueFrontmatterPatch = Partial>`, `src/schemas/issue-frontmatter.ts:17`) never needed field *removal*; introducing `null` into the milestone patch would leak a YAML-serialization concern into the type and fight `YAML.stringify`'s `keepUndefined` behavior. A boolean maps 1:1 to the CLI's `--clear-target` flag and keeps the patch type free of null unions. + +An empty patch (`update(slug, {})`) is a validated no-op rewrite at the store level; the *CLI* enforces "at least one field option required" (per spec) so the store stays simple and testable. + +## 3. CLI wiring (`src/cli/commands/milestone.ts`) + +### Shared commit helper + +`create`'s commit block (:77-87) would otherwise be copy-pasted twice more. Extract: + +```ts +async function commitMilestones(projectRoot: string, message: string): + Promise<{ committed: boolean; commitSha?: string }> +``` + +— same `git add spec/milestones` → `commit` → `rev-parse`, same swallow-on-failure semantics. `create` refactors onto it (behavior-preserving). + +### `metta milestone close ` + +```ts +milestone + .command('close') + .argument('', 'Milestone slug') + .option('--abandoned', 'Mark abandoned instead of closed') + .option('--on-branch ', 'Acknowledge non-main branch and proceed') + .description('Close (or abandon) an open milestone') +``` + +Action flow, mirroring `create`'s structure (:57-100): + +1. `createCliContext()`, load config, `assertOnMainBranch(ctx.projectRoot, mainBranch, options.onBranch)` — identical branch guard (:61-63). +2. `ctx.milestonesStore.show(slug)` — not-found surfaces here (exit 4, type `not_found` via the existing `:183` mapping). +3. Conflict check: if `current.status !== 'open'` → exit 4 with `{ error: { code: 4, type: 'milestone_conflict', message: 'Milestone '' is already ' } }` (message names the current status per spec). File untouched — no store call made. +4. `update(slug, { status: options.abandoned ? 'abandoned' : 'closed' })`. +5. `commitMilestones(ctx.projectRoot, 'chore: close milestone ')` — one message for both closed and abandoned (spec pins `chore: close milestone ` for the `--abandoned` scenario too). +6. JSON: `{ slug, status, committed, commit_sha }`; text: `Closed milestone: ` / `Abandoned milestone: ` + committed line, matching `create`'s output conventions (:89-94). + +### `metta milestone update ` + +```ts +milestone + .command('update') + .argument('', 'Milestone slug') + .option('--name ', 'Rename display name') + .option('--target ', 'Set or change target date (YYYY-MM-DD)') + .option('--clear-target', 'Remove the target date') + .option('--description ', 'Replace the description body') + .addOption(new Option('--status ', 'Set status explicitly').choices(['open', 'closed', 'abandoned'])) + .option('--on-branch ', 'Acknowledge non-main branch and proceed') +``` + +Notes: + +- `Option.choices()` gives Commander-level rejection of bad `--status` values with a helpful message; Zod remains the authoritative write gate (defense in depth, consistent with "validate all state writes"). +- `--target` vs `--clear-target` mutual exclusion via Commander's `.conflicts('clearTarget')` (Commander >= 9; the repo is on a modern Commander — verify version in `package.json` during planning) or a manual pre-check that exits 4. +- "No field options" check: if none of `name/target/clearTarget/description/status` present → exit 4 (`type: 'milestone_error'`, message "at least one field option is required"), no store call. +- Validation failures from the store (`Invalid milestone frontmatter … target …`) already carry field names via `formatZodError`; the catch block maps to exit 4 with the standard envelope — same shape as `create`'s catch (:95-100). +- Commit message: `chore: update milestone `. +- JSON: `{ slug, changed: ['target', 'status', …], committed, commit_sha }` (spec: "the fields changed"). + +`--json` parity comes free: both subcommands read `program.opts().json` exactly as `create`/`list`/`show` do (:58, :107, :134), and every error path uses `outputJson({ error: { code, type, message } })`. + +## 4. Schema + rollup + renderer changes + +1. `src/schemas/milestone-frontmatter.ts:26` → `status: z.enum(['open', 'closed', 'abandoned']).default('open')`. Zod's enum error already names the received value and allowed values (spec scenario "status: shipped rejected naming allowed values" — `formatZodError` renders it). +2. `src/milestones/milestones-store.ts:14` → `status: 'open' | 'closed' | 'abandoned'` (or derive: `status: MilestoneFrontmatter['status']` to kill the duplication permanently). +3. `src/milestones/milestone-rollup.ts`: + - `:7` → `status: Milestone['status']`. + - Sort `:77-80` → rank comparator: + ```ts + const rank = (s: Milestone['status']) => (s === 'open' ? 0 : 1) + rollups.sort((a, b) => rank(a.status) - rank(b.status) + || (a.slug < b.slug ? -1 : a.slug > b.slug ? 1 : 0)) + ``` + For inputs containing only `open`/`closed` this is *behaviorally identical* to the current comparator (open→0, closed→1), preserving the spec's byte-compat requirement; `abandoned` joins the terminal group, slug-ascending within it. + - Export a shared marker map so three render sites can't drift: + ```ts + export const MILESTONE_MARKERS = { open: '▸', closed: '✓', abandoned: '✗' } as const + ``` +4. Renderers (three sites): replace the ternary with the map lookup. + - `src/cli/commands/milestone.ts:121` → `MILESTONE_MARKERS[r.status]` (uncolored). + - `src/cli/commands/status.ts:33` and `src/cli/commands/progress.ts:216` → colored: `✓` stays green 32, `▸` stays cyan 36, `✗` red 31 (or grey 90 — subjective; recommend red 31 as visually distinct from grey target text at `progress.ts:217`). Glyph — not just color — differs, satisfying "visually distinguishable from both `▸` and `✓`" in no-color terminals. + - `milestone show` (`milestone.ts:154`, `:166`): no change — status passes through as data. + - `status`/`progress` JSON (`toMilestoneCountsRow`, `milestone.ts:39-42`): no change — status is already carried in the rollup row. + +## 5. Guard hook + mint hook + skill + +Four files, two logical edits, byte-identical pairs: + +| Edit | Deployed | Template | +|---|---|---| +| `BLOCKED_TWO_WORD`: `['milestone', new Set(['create', 'close', 'update'])]` (line 81) + comment | `.claude/hooks/metta-guard-bash.mjs` | `src/templates/hooks/metta-guard-bash.mjs` | +| `SKILL_SCOPES['metta-backlog']`: append `'milestone:close', 'milestone:update'` (line 35) | `.claude/hooks/metta-session-mint.mjs` | `src/templates/hooks/metta-session-mint.mjs` | + +No trust-model change: the guard's scope-key derivation (`metta-guard-bash.mjs:902-905`) and the two-band freshness logic are untouched — the new verbs ride the existing Tier-2 machinery exactly as `milestone create` does. `milestone list`/`show` stay in `ALLOWED_TWO_WORD` (:60), unchanged. + +Plus: extend `.claude/skills/metta-backlog/SKILL.md` (milestone branch at :28) with `close` and `update` actions so the minted credential is actually reachable through the sanctioned skill flow. (Skill templates: check whether `SKILL.md` has a `src/templates/` counterpart during planning — the hooks do; skills are copied to `dist/` at build time per project conventions.) + +## 6. Test plan + +Near-1:1 ratio holds — every touched source file has an existing test file to extend; no new test files strictly required (optionally one new `tests/milestone-frontmatter.test.ts` for direct schema cases). + +| Test file | New cases | +|---|---| +| `tests/milestones-store.test.ts` | `update` status patch preserves name/target/body; `clearTarget` removes the key (assert `readFile` content has no `target`); invalid patch (`target: '2026-02-30'`, empty name) throws naming the field AND file byte-identical to pre-call snapshot (use `seedMilestoneFile` + `readFile` before/after); not-found throws, `milestones/` dir gains no file; `abandoned` round-trips through `show`; reading a seeded `status: abandoned` file validates; seeded `status: shipped` file rejects naming allowed values; seeded pre-change `open`/`closed` files parse identically (back-compat pin). Temp-dir isolation: existing `mkdtemp`/`rm` beforeEach/afterEach (:11-18). | +| `tests/cli-milestone.test.ts` | `close` happy path: file frontmatter reads `status: closed`, `git log` contains `chore: close milestone ` (pattern at :63-65), JSON `{ slug, status, committed, commit_sha }`; `close --abandoned` writes `abandoned`; close on already-`closed` → exit 4, conflict envelope, byte-identical file; close on missing slug → exit 4 `not_found`, no file created; branch-guard refusal without `--on-branch` (create has the precedent test); `update --description` replaces body only; `--clear-target` removes key; `--status open` reopens a closed milestone; `--target 2026-02-30 --json` → exit 4, envelope names `target`, byte-identical file; missing milestone → `not_found`; zero field options → non-zero exit, file untouched. Harness: `installFixture(tempDir)` + `runCli`, per existing suite. | +| `tests/milestone-rollup.test.ts` | Mixed `open`/`closed`/`abandoned` input sorts open-first then terminal slug-ascending; open/closed-only ordering unchanged (byte-compat pin); `abandoned` status passes through the rollup row. | +| `tests/cli-status.test.ts` / progress tests | One case each: an `abandoned` milestone renders (exit 0, `✗` marker, sorted after open); existing open/closed output assertions double as byte-compat pins. | +| `tests/metta-guard-bash.test.ts` | `metta milestone close x` / `metta milestone update x` blocked without credential (exit 2, mirroring `backlog add` at :98); allowed with a minted metta-backlog-scoped token covering `milestone:close`/`milestone:update` (token-fixture helpers already in the file); `milestone list`/`show` still allowed credential-free. | +| `tests/hooks-byte-identity.test.ts` | No edits — automatically pins the deployed/template hook pairs; failing it is the desired tripwire if one copy is forgotten. | +| `tests/metta-guard-mint-seam.test.ts` | Check whether it pins `SKILL_SCOPES` contents (it inspects scope overlap around :491); extend the metta-backlog scope expectation if so. | + +## 7. Back-compat analysis + +- **Read path:** enum extension is strictly additive — every existing file carries `open` or `closed`, both still valid; `.default('open')` unchanged. The spec's "byte-identical output when only open/closed exist" holds: the rank comparator is behavior-identical for two states (§4.3), markers for open/closed are the same glyphs/colors, JSON shapes gain no new keys. +- **Forward-compat caveat (accepted in intent.md §Impact):** a file written with `status: abandoned` fails validation under older metta builds. One-way door once any milestone is abandoned. +- **Serialization normalization:** `update` re-serializes the whole frontmatter block via `YAML.stringify`, unlike the issue store's minimal-diff Document API. Files written by `metta milestone create` round-trip stably (same serializer); a *hand-edited* file (reordered keys, YAML comments, unusual quoting) is normalized on first update and comments are dropped. Acceptable: milestone frontmatter is three metta-owned keys, metta never writes comments, and hand-editing is precisely the workflow this change eliminates. Not worth importing the Document-API machinery. +- **Body preservation:** `formatMilestone` trims the body (:70) — a body with leading/trailing blank lines is normalized on any update. Same class of accepted normalization; `parseMilestone` already trims on read (:55), so the parsed value is unchanged. + +## 8. Risks + +1. **Forgotten hook template mirror** — `hooks-byte-identity.test.ts` catches it in CI; low residual risk. +2. **Rollup comparator regression** — the only behavioral rewrite of existing logic; mitigated by the byte-compat pin tests (§6) and the fact that the two-state case is provably identical. +3. **Marker glyph rendering** — `✗` (U+2717) matches the width/class of the existing `✓` (U+2713); no `padEnd` misalignment expected. Verify in the CLI test's text-mode assertion. +4. **Guard scope routing** — only `metta-backlog` mints `milestone:close`/`milestone:update`. If a future ship/finalize flow wants to close milestones programmatically, its skill scope will need extending; out of scope now, but worth a comment at the `SKILL_SCOPES` line. +5. **`update --status closed` bypasses close's conflict check** — intentional per spec (US-2 reopen scenario is symmetric); `update` is the explicit "I know what I'm doing" verb, `close` is the guarded transition. Document in the command descriptions. +6. **Commander `.conflicts()` availability** — depends on the installed Commander major; if absent, a two-line manual check is equivalent. Resolve during planning by reading `package.json` (deterministic, no user escalation needed). +7. **Race between show and update in the close flow** — the conflict check (CLI reads, then store re-reads) is TOCTOU-shaped, but the store re-validates on its own read and the CLI is single-user/single-process over local files; consistent with every existing metta command. Accepted. + +## 9. Assessment of this approach + +**Pros** +- Mirrors the proven sibling pattern (`IssuesStore.updateFrontmatter`, `issues-store.ts:223-234`) — reviewers already know the shape; validate-before-write and CLI-edge commits are established conventions. +- Byte-identical-on-failure is structural (validation precedes I/O), not defensive — satisfies "no unvalidated state writes" with zero extra machinery. +- All new surface rides existing rails: error envelope, branch guard, auto-commit, Tier-2 scope derivation, temp-dir test harnesses. No new abstractions except a 3-entry marker map and a small commit helper (which *removes* duplication). +- Directly closes the reported zeus-session pain (`m1`/`m6` stale status and bodies) with a validated, auditable path. + +**Cons / costs** +- Touches ~10 files (3 milestone modules, 2 renderer sites, 4 hook files, 1 skill file) plus 5-6 test files — the widest of the candidate options, though each edit is small. +- Full re-serialization normalizes hand-edited frontmatter (accepted, documented above). +- `abandoned` is a one-way forward-compat door for older builds (accepted in intent). + +**Recommendation:** proceed with this approach as specified. It is the smallest design that makes the modeled lifecycle actually reachable, and every sub-decision (patch shape, clear-target boolean, CLI-edge commits, rank comparator, marker map, hook scope keys) has a direct precedent in the codebase cited above. Suggested implementation order: schema enum → store `update` + tests → rollup/renderers + tests → CLI verbs + tests → hooks/skill + guard tests. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research.md new file mode 100644 index 00000000..ad14beba --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/research.md @@ -0,0 +1,31 @@ +# Research: fix-milestone-status-write-once-dead-field-no-close + +## Decision: Full store-level update (`MilestonesStore.update` + `milestone close` / `milestone update` CLI verbs + `abandoned` enum) + +### Approaches Considered + +1. **Full store-level update** (selected) — `MilestonesStore.update(slug, patch)` mirroring the proven sibling `IssuesStore.updateFrontmatter` pattern (`src/issues/issues-store.ts:223-234`), two new CLI verbs (`close`, `update`) cloned from the `create` action shape, `abandoned` added to the status enum, guard/mint hook scope extension. Satisfies all six spec.md requirement blocks; every sub-decision has a cited in-repo precedent. Details: `research-store-update.md`. +2. **Derived status from issue rollups** — compute closed-ness from `computeMilestoneRollups`. Rejected: satisfies **0 of 6** spec requirements, cannot represent `abandoned`, auto-closes zero-issue milestones vacuously (or reproduces the write-once bug via a special case), leaves stale bodies uneditable, converts the persisted `status` field into an actively contradicted source of truth, and violates the byte-compat scenario by design. Also explicitly out of scope per `intent.md`. Details: `research-derived-status.md`. +3. **Minimal close-only + advisory** — only `milestone close` plus an "eligible for close" warning. Rejected: fully satisfies **0 of 5** ADDED spec requirements (close verb is 4/5 minus `--abandoned`), leaves the stale-body half of the reported defect unfixed with hand-editing (unvalidated writes) as the only recourse, introduces an irreversible close with no reopen, and the advisory contradicts both `intent.md`'s out-of-scope list and spec.md's byte-compat scenario. The implied follow-up change re-touches the identical file set, roughly doubling lifecycle cost. Details: `research-close-only.md`. + +### Rationale + +The full-update approach is the smallest design that makes the already-modeled lifecycle reachable and closes **both** halves of the reported defect (permanently-open status and stale body text) through a validated, auto-committed path: + +- **Proven pattern**: read → patch → full-frontmatter Zod re-validate → write mirrors `IssuesStore.updateFrontmatter`; validation precedes I/O, so byte-identical-on-failure is structural, satisfying "no unvalidated state writes" with zero extra machinery. +- **Patch shape**: `MilestonePatch` with a `clearTarget: boolean` (not `target: null`) — `YAML.stringify`'s `keepUndefined: false` drops the key naturally; keeps the type free of null unions. +- **Commit at the CLI edge**: extract `create`'s swallow-on-failure commit block (`src/cli/commands/milestone.ts:77-87`) into a shared `commitMilestones` helper; messages `chore: close milestone ` / `chore: update milestone `. +- **Rollup sort** (`src/milestones/milestone-rollup.ts:77-80`) must become a rank comparator (open=0, terminal=1) — provably behavior-identical for open/closed-only inputs, preserving the spec's byte-compat requirement. +- **Markers**: export a shared `MILESTONE_MARKERS` map (`▸` open, `✓` closed, `✗` abandoned) to de-duplicate three render sites (`milestone.ts:121`, `status.ts:33`, `progress.ts:216`). +- **Guard**: add `close`/`update` to `BLOCKED_TWO_WORD` in `metta-guard-bash.mjs:81` (scope keys auto-derive) and extend `SKILL_SCOPES['metta-backlog']` in `metta-session-mint.mjs:35` — **each edit mirrored byte-identically into `src/templates/hooks/`** (pinned by `tests/hooks-byte-identity.test.ts`), plus `close`/`update` branches in `.claude/skills/metta-backlog/SKILL.md`. +- **Test gap found**: `tests/metta-guard-bash.test.ts` has zero milestone-specific cases — new verbs need block/allow coverage following the `backlog add` pattern. + +Accepted risks (documented in intent): `status: abandoned` is a one-way forward-compat door for older builds; full YAML re-serialization normalizes hand-edited frontmatter (acceptable — milestone frontmatter is three metta-owned keys and hand-editing is the workflow being eliminated). Commander `.conflicts()` availability for `--target`/`--clear-target` mutual exclusion is resolved during planning by reading `package.json`. + +Suggested implementation order: schema enum → store `update` + tests → rollup/renderers + tests → CLI verbs + tests → hooks/skill + guard tests. Footprint: ~10 source files plus 5-6 test files, all small edits. + +### Artifacts Produced + +- [Research: full store-level update](research-store-update.md) +- [Research: derived status from rollups](research-derived-status.md) +- [Research: minimal close-only](research-close-only.md) diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/review.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/review.md new file mode 100644 index 00000000..07875814 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/review.md @@ -0,0 +1,31 @@ +# Review: fix-milestone-status-write-once-dead-field-no-close + +Three parallel reviews (correctness, security, quality) over the full `main...HEAD` diff. **Overall: PASS_WITH_WARNINGS — no critical or blocking issues.** + +## Correctness — PASS + +Verified: validate-before-I/O ordering in `MilestonesStore.update` (byte-identical on failure is structural, pinned by byte-snapshot tests); clearTarget/target mutual exclusion enforced at both store and Commander levels; close conflict pre-check makes no store call and provably leaves the file untouched; empty-patch handling; `changed` field reporting per design; rank comparator provably identical to the legacy comparator for open/closed-only inputs (pinned in-test against a legacy reimplementation); reopen via `update --status open`; target calendar refinement (`2026-02-30` rejected naming the field); guard/mint/skill mirrors byte-identical with scope keys auto-derived. + +Suggestions (informational, no action): `changed` reports patched fields rather than value-diffed fields (matches design §API); branch guard precedes the no-field-options check (matches `create` precedence). + +## Security — PASS + +- Slug injection: `assertSafeSlug` on every new path-constructing entry point; `SLUG_RE` excludes traversal characters. +- Exec safety: `commitMilestones` uses `execFile` argv arrays (no shell); slug only ever a `-m` value; `git add` path is a fixed literal. +- Guard trust model: strictly additive — `close`/`update` join the fail-closed Tier-2 blocked set; scopes minted only for `metta-backlog`; `list`/`show` verified pure reads; deployed/template hook copies sha256-identical. +- YAML: default `YAML.parse` (no code-executing tags) + strict Zod; `--name` values are escaped by `YAML.stringify`; frontmatter fence regex anchored, body `---` never re-interpreted. +- No unvalidated write paths. + +Minor (pre-existing, inherited from `create`, not introduced here): `git commit -m` commits the whole index, so independently pre-staged files ride the auto-commit; close's show→update conflict check is TOCTOU-shaped (accepted in design for a single-user local CLI). + +## Quality — PASS_WITH_WARNINGS + +Verified: conventions (`.js` import extensions, naming, no inlined templates, functional core/imperative shell), 375 tests green across the five touched suites, mirror byte-identity independently confirmed, byte-compat pins at store/rollup/CLI levels, guard block/allow/scope-missing matrix with audit assertions. + +Warning (should fix): spec scenario "Show reports the abandoned state accurately" had no direct CLI test — no assertion that `milestone show` on an abandoned milestone prints `Status: abandoned` / `"status": "abandoned"`. **Resolution: addressed in a follow-up commit adding the missing test (see git log).** + +Suggestions (accepted as-is): color map duplicated across two render sites (design-documented decision); catch-block error mapping duplicated between close/update (repo per-action style); symmetric scope-missing guard test for `milestone:update`. + +## Loop outcome + +Iteration 1: PASS / PASS / PASS_WITH_WARNINGS — exit criteria met on first pass. The single test-coverage warning fixed post-review; no re-review required (additive test only, passthrough already verified correct by the correctness reviewer). diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/spec.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/spec.md new file mode 100644 index 00000000..3aeeeb53 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/spec.md @@ -0,0 +1,160 @@ +# issue-logging + +## MODIFIED: Requirement: Milestone store with Zod-validated frontmatter and CLI + +Milestones MUST be stored as one markdown file per milestone at `spec/milestones/.md`. Each file MUST carry YAML frontmatter validated by a Zod schema with fields: `name` (string, required), `target` (ISO 8601 date string `YYYY-MM-DD`, optional), and `status` (enum `open` | `closed` | `abandoned`, defaulting to `open`); the body below the frontmatter is the free-form description. The CLI MUST provide `metta milestone create --name [--target ] [--description ]`, `metta milestone list`, `metta milestone show `, `metta milestone close `, and `metta milestone update `. `create` MUST refuse to overwrite an existing milestone file. Invalid frontmatter values (e.g., a malformed `target` date or unknown `status`) MUST produce a clear validation error on read or write. Existing milestone files carrying `status: open` or `status: closed` MUST continue to validate and behave exactly as before the enum extension. The mutating subcommands `milestone create`, `milestone close`, and `milestone update` MUST be registered with the orchestration guard as Tier 2 (session-tier) scoped two-word forms, consistent with `backlog add/done/promote`; the read-only `milestone list` and `milestone show` subcommands MUST be permitted without a session credential, consistent with other read-only commands. (Traces: US-3, US-5; intent proposal §2, §6.) + +### Scenario: Milestone created with defaults +- GIVEN no milestone `v0-6` exists +- WHEN the user runs `metta milestone create v0-6 --name "v0.6" --target 2026-09-30 --description "Backlog/milestone unification release"` +- THEN `spec/milestones/v0-6.md` is written with frontmatter `name: v0.6`, `target: 2026-09-30`, `status: open`, the description as body, and `metta milestone list` includes `v0-6` + +### Scenario: Creating a duplicate milestone is refused +- GIVEN `spec/milestones/v0-6.md` already exists +- WHEN the user runs `metta milestone create v0-6 --name "v0.6 again"` +- THEN the command exits non-zero with an error stating the milestone already exists, and the existing file is unmodified + +### Scenario: Invalid milestone status is rejected +- GIVEN a milestone file whose frontmatter contains `status: shipped` +- WHEN the milestone store reads the file +- THEN validation fails with an error naming the `status` field and the allowed values `open`, `closed`, `abandoned` + +### Scenario: Abandoned status validates through the schema +- GIVEN a milestone file whose frontmatter contains `status: abandoned` +- WHEN the milestone store reads the file +- THEN validation succeeds and the parsed milestone reports status `abandoned` + +### Scenario: Pre-existing open and closed files are unaffected +- GIVEN milestone files on disk carrying `status: open` and `status: closed` written before the enum extension +- WHEN any milestone command reads them +- THEN both files validate without error and produce the same parsed status values as before the change + + +## ADDED: Requirement: Milestone store update applies validated patches + +`MilestonesStore` MUST expose an `update(slug, patch)` method that reads the existing milestone file at `spec/milestones/.md`, applies the patch, and writes the result back through the state store. The patch MUST support: changing `name`, setting or changing `target`, clearing `target` (removing the field from frontmatter), replacing the description body, and setting `status` to any value in the enum. Fields absent from the patch MUST be preserved unchanged. Before any write, the full resulting frontmatter MUST be re-validated through `MilestoneFrontmatterSchema`; a patch whose result fails validation MUST be rejected with a clear validation error and MUST leave the file on disk byte-identical to its pre-call state — no unvalidated state may reach disk. Updating a slug with no matching milestone file MUST fail with a clear not-found error naming the slug, and MUST NOT create a file. The existing `create`, `list`, `show`, and `exists` behaviors MUST be unchanged. (Traces: US-1, US-2; intent proposal §1.) + +### Scenario: Status patch preserves untouched fields +- GIVEN `spec/milestones/m1.md` with frontmatter `name: M1`, `target: 2026-09-30`, `status: open` and a non-empty description body +- WHEN `update('m1', { status: 'closed' })` is called +- THEN the rewritten file carries `status: closed`, retains `name: M1`, `target: 2026-09-30`, and the identical description body, and the written frontmatter passed `MilestoneFrontmatterSchema` validation before the write + +### Scenario: Target is cleared from frontmatter +- GIVEN a milestone whose frontmatter includes a `target` field +- WHEN `update` is called with a patch that clears the target +- THEN the rewritten frontmatter contains no `target` key (not `target: null`) and still passes schema validation + +### Scenario: Invalid patch is rejected and the file is untouched +- GIVEN an existing milestone file and a byte snapshot of its content +- WHEN `update` is called with a patch producing invalid frontmatter (e.g. `target: '2026-02-30'` or an empty `name`) +- THEN the call throws a validation error identifying the offending field, and the file on disk is byte-identical to the snapshot + +### Scenario: Updating a missing milestone fails without side effects +- GIVEN no file exists at `spec/milestones/ghost.md` +- WHEN `update('ghost', { status: 'closed' })` is called +- THEN the call throws an error stating milestone `ghost` was not found, and no file is created under `spec/milestones/` + + +## ADDED: Requirement: Milestone close CLI verb transitions to a terminal state + +The CLI MUST provide `metta milestone close `, which transitions an `open` milestone to `status: closed`, or to `status: abandoned` when the `--abandoned` flag is passed. The write MUST go through the validated store update path. On success the command MUST auto-commit the milestone file with the conventional message `chore: close milestone `, following the `milestone create` commit pattern (git failure is swallowed, reported as uncommitted). The command MUST respect the same main-branch guard as `milestone create` (`--on-branch ` acknowledgment required off the configured main branch). Closing a milestone that is already `closed` or `abandoned` MUST fail with exit code 4 and a clear conflict error — rendered via the standard JSON error envelope (`{ error: { code, type, message } }`) under `--json`, plain stderr otherwise — leaving the file unmodified. Closing a slug with no matching milestone file MUST fail with exit code 4 and a not-found error, creating no file. Under `--json`, a successful close MUST emit a JSON result object including the slug, the resulting status, and commit information, consistent with `milestone create` output conventions. (Traces: US-1, US-3; intent proposal §3.) + +### Scenario: Open milestone is closed and auto-committed +- GIVEN `spec/milestones/m1.md` with `status: open` in a git repository on the main branch +- WHEN the user runs `metta milestone close m1` +- THEN the file's frontmatter reads `status: closed`, a commit exists with message `chore: close milestone m1`, and the command exits 0 reporting the closure + +### Scenario: Abandoned flag writes the abandoned state +- GIVEN `spec/milestones/m6.md` with `status: open` +- WHEN the user runs `metta milestone close m6 --abandoned` +- THEN the file's frontmatter reads `status: abandoned`, the result passed schema validation before write, and the transition is auto-committed with `chore: close milestone m6` + +### Scenario: Closing an already-terminal milestone is a conflict +- GIVEN `spec/milestones/m1.md` with `status: closed` and a byte snapshot of the file +- WHEN the user runs `metta milestone close m1 --json` +- THEN the command exits 4, stdout carries a JSON error envelope with a conflict-typed error naming the milestone's current status, and the file is byte-identical to the snapshot + +### Scenario: Closing a missing milestone reports not found +- GIVEN no file exists at `spec/milestones/ghost.md` +- WHEN the user runs `metta milestone close ghost --json` +- THEN the command exits 4 with a JSON error envelope of type `not_found`, and no file is created or modified + +### Scenario: Main-branch guard applies to close +- GIVEN the repository checkout is on a branch other than the configured main branch +- WHEN the user runs `metta milestone close m1` without `--on-branch` +- THEN the command refuses with the same branch-guard error behavior as `metta milestone create`, and the milestone file is unmodified + + +## ADDED: Requirement: Milestone update CLI verb edits mutable fields + +The CLI MUST provide `metta milestone update `, which patches a milestone's mutable fields via options: `--name ` (rename display name), `--target ` (set or change target), `--clear-target` (remove target), `--description ` (replace the description body), and `--status ` (explicitly set status, including reopening a terminal milestone). Only fields named by the provided options may change; all other fields MUST be preserved. Invoking the command with no field options MUST fail with a clear error stating that at least one field option is required, leaving the file untouched. The write MUST go through the validated store update path: an update whose resulting frontmatter fails `MilestoneFrontmatterSchema` MUST exit 4 with a clear validation error (standard JSON error envelope under `--json`) and leave the file byte-identical. Updating a slug with no matching milestone file MUST exit 4 with a not-found error. The command MUST respect the same main-branch guard as `milestone create` and MUST auto-commit successful updates with the conventional message `chore: update milestone `, following the `create` commit pattern. Under `--json`, success MUST emit a JSON result object including the slug, the fields changed, and commit information. (Traces: US-2; intent proposal §4.) + +### Scenario: Description is replaced without touching other fields +- GIVEN `spec/milestones/m1.md` with `status: open`, a `target`, and a stale body reading "In flight as PR #24" +- WHEN the user runs `metta milestone update m1 --description "Shipped in v0.5.0"` +- THEN the body reads "Shipped in v0.5.0", the frontmatter `name`, `target`, and `status` values are unchanged, and the change is auto-committed with `chore: update milestone m1` + +### Scenario: Clear-target removes the field +- GIVEN a milestone with `target: 2026-09-30` +- WHEN the user runs `metta milestone update --clear-target` +- THEN the rewritten frontmatter contains no `target` key and passes schema validation + +### Scenario: A mistakenly closed milestone is reopened +- GIVEN a milestone with `status: closed` +- WHEN the user runs `metta milestone update --status open` +- THEN the frontmatter reads `status: open`, the write passed validation, and the change is auto-committed + +### Scenario: Invalid field value fails validation and leaves the file untouched +- GIVEN an existing milestone and a byte snapshot of its file +- WHEN the user runs `metta milestone update --target 2026-02-30 --json` +- THEN the command exits 4, stdout carries a JSON error envelope with a validation message naming `target`, and the file is byte-identical to the snapshot + +### Scenario: Updating a missing milestone reports not found +- GIVEN no file exists at `spec/milestones/ghost.md` +- WHEN the user runs `metta milestone update ghost --name "Ghost"` +- THEN the command exits 4 with a not-found error, and no file is created + +### Scenario: No field options is an error +- GIVEN an existing milestone +- WHEN the user runs `metta milestone update ` with no field options +- THEN the command exits non-zero with an error stating at least one field option is required, and the file is unmodified + + +## ADDED: Requirement: Renderers and rollups handle the abandoned state + +Milestone rendering surfaces MUST handle `status: abandoned` as a terminal state grouped with `closed`. `computeMilestoneRollups` MUST sort open milestones first and terminal milestones (`closed` and `abandoned`) after them, slug-ascending within each group. `metta milestone list` MUST render a distinct marker for `abandoned` milestones (visually distinguishable from both the open marker `▸` and the closed marker `✓`). `metta milestone show` MUST report `Status: abandoned` (and the `--json` `status` field as `abandoned`) rather than mislabeling the state. The milestone sections of `metta status` and `metta progress`, which inherit the shared rollup path, MUST render `abandoned` milestones without crashing or mislabeling them. When only `open` and `closed` milestones exist, human and `--json` output of `milestone list`, `milestone show`, `metta status`, and `metta progress` MUST be byte-identical to pre-change behavior. (Traces: US-3, US-4; intent proposal §5.) + +### Scenario: List sorts terminal states after open with distinct markers +- GIVEN milestones in `open`, `closed`, and `abandoned` states +- WHEN the user runs `metta milestone list` +- THEN open milestones appear before both terminal milestones, each of the three states renders its own marker, and the `abandoned` row's marker differs from `▸` and `✓` + +### Scenario: Show reports the abandoned state accurately +- GIVEN an `abandoned` milestone +- WHEN the user runs `metta milestone show ` (human and `--json`) +- THEN the human output contains `Status: abandoned` and the JSON output carries `"status": "abandoned"` + +### Scenario: Status and progress render abandoned without crashing +- GIVEN at least one `abandoned` milestone exists alongside open milestones +- WHEN `metta status` or `metta progress` renders its milestone section +- THEN the command exits 0 and the abandoned milestone appears in the rollup sorted after open milestones, labeled with its abandoned state + +### Scenario: Open and closed output stays byte-compatible +- GIVEN a project containing only `open` and `closed` milestones +- WHEN `metta milestone list`, `metta milestone show`, `metta status`, and `metta progress` run in both human and `--json` modes +- THEN the output of each is byte-identical to the output produced before this change + + +## ADDED: Requirement: Guard authorization for milestone close and update + +The `metta-guard-bash` hook's allow-list MUST authorize the scoped two-word forms `milestone close` and `milestone update` under the same Tier 2 (session-tier) trust rules as the existing `milestone create` verb, with no change to the two-tier trust model itself. Contexts not authorized for existing milestone mutating verbs MUST be blocked from the new verbs identically. (Traces: US-5; intent proposal §6.) + +### Scenario: Authorized skill context may invoke the new verbs +- GIVEN a skill context holding a valid session credential that authorizes `metta milestone create` +- WHEN it invokes `metta milestone close ` or `metta milestone update ` +- THEN the guard hook permits both commands + +### Scenario: Unauthorized context is blocked identically +- GIVEN a context without a valid session credential +- WHEN it attempts `metta milestone close ` or `metta milestone update ` +- THEN the guard blocks the commands with the same denial behavior it applies to `metta milestone create` diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/stories.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/stories.md new file mode 100644 index 00000000..26323e3b --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/stories.md @@ -0,0 +1,81 @@ +# fix-milestone-status-write-once-dead-field-no-close — User Stories + +## US-1: Close a completed milestone from the CLI + +**As a** developer using metta milestones as a system of record +**I want to** run `metta milestone close ` when a milestone's work has shipped +**So that** the milestone stops reporting a stale `open` status and my project's recorded state matches reality + +**Priority:** P1 +**Independent Test Criteria:** Running `metta milestone close ` against an open milestone transitions its frontmatter to `status: closed` on disk via a validated write, and the transition is auto-committed with a conventional `chore:` message. + +**Acceptance Criteria:** +- **Given** a milestone file with `status: open` **When** the user runs `metta milestone close ` **Then** the file's frontmatter is rewritten to `status: closed`, the result passes `MilestoneFrontmatterSchema` validation before write, and the change is auto-committed with a conventional `chore:` message +- **Given** a milestone that is already `closed` **When** the user runs `metta milestone close ` **Then** the command fails with a clear conflict error using the standard JSON error envelope and the file is not modified +- **Given** a slug with no matching milestone file **When** the user runs `metta milestone close ` **Then** the command fails with a clear not-found error and no file is created or modified +- **Given** the repository is on the main branch **When** the user runs `metta milestone close ` without `--on-branch` acknowledgment **Then** the command respects the same main-branch guard behavior as `metta milestone create` + +--- + +## US-2: Update a milestone's mutable fields without hand-editing YAML + +**As a** developer maintaining milestone descriptions and targets +**I want to** run `metta milestone update ` with options like `--name`, `--target`, `--clear-target`, and `--description` +**So that** I can correct stale bodies (e.g. "In flight as PR #24") and outdated targets through a validated, auto-committed path instead of hand-editing frontmatter that bypasses Zod validation + +**Priority:** P1 +**Independent Test Criteria:** Running `metta milestone update ` with field options patches exactly the specified fields, re-validates the full frontmatter before write, auto-commits, and leaves unspecified fields untouched. + +**Acceptance Criteria:** +- **Given** an existing milestone **When** the user runs `metta milestone update --description ""` **Then** the description body is replaced, all other fields are unchanged, and the write is validated and auto-committed +- **Given** an existing milestone with a `target` set **When** the user runs `metta milestone update --clear-target` **Then** the target is removed and the resulting frontmatter still passes schema validation +- **Given** a closed milestone that was closed by mistake **When** the user explicitly requests a status change via `metta milestone update ` **Then** the milestone is reopened through the same validated update path +- **Given** an update whose resulting frontmatter would fail `MilestoneFrontmatterSchema` **When** the command runs **Then** the write is rejected with a clear validation error and no unvalidated state reaches disk +- **Given** a slug with no matching milestone file **When** the user runs `metta milestone update ` **Then** the command fails with a clear not-found error + +--- + +## US-3: Distinguish abandoned milestones from completed ones + +**As a** developer whose plans change +**I want to** mark a dropped milestone as `abandoned` (e.g. `metta milestone close --abandoned`) +**So that** milestones that were scrapped are recorded distinctly from milestones that were achieved, keeping the project history honest + +**Priority:** P2 +**Independent Test Criteria:** Closing a milestone with the abandoned flag writes `status: abandoned`, which the schema accepts, and existing files with `open`/`closed` continue to validate unchanged. + +**Acceptance Criteria:** +- **Given** an open milestone **When** the user runs `metta milestone close --abandoned` **Then** the frontmatter is written as `status: abandoned` and passes validation +- **Given** existing milestone files carrying `status: open` or `status: closed` **When** any milestone command reads them **Then** they validate and behave exactly as before this change +- **Given** an `abandoned` milestone **When** the user views it via `milestone show` **Then** the status line clearly reports the abandoned state rather than mislabeling it as open or closed + +--- + +## US-4: Accurate milestone status in dashboards and lists + +**As a** developer (or AI orchestrator) checking project state via `metta status`, `metta progress`, or `metta milestone list` +**I want to** see closed and abandoned milestones rendered with correct markers and ordered after open milestones +**So that** dashboards reflect shipped and dropped work truthfully instead of reporting everything as forever open + +**Priority:** P2 +**Independent Test Criteria:** `milestone list`, `milestone show`, and the `status`/`progress` milestone sections render `closed` and `abandoned` states with distinct, sensible output, with terminal-state milestones sorted after open ones, and byte-identical output for `open`/`closed` milestones compared to the prior release. + +**Acceptance Criteria:** +- **Given** milestones in `open`, `closed`, and `abandoned` states **When** the user runs `metta milestone list` **Then** each state renders a sensible marker and terminal-state milestones (closed and abandoned) sort after open ones +- **Given** an `abandoned` milestone **When** `metta status` or `metta progress` renders the milestone section **Then** the rollup renders the state without crashing or mislabeling it +- **Given** only `open` and `closed` milestones **When** any milestone rendering runs **Then** human and `--json` output is unchanged from pre-change behavior + +--- + +## US-5: New milestone verbs usable from authorized AI sessions + +**As an** AI orchestrator session working through metta skills +**I want to** invoke `milestone close` and `milestone update` from authorized skill contexts without the guard hook blocking them +**So that** milestone lifecycle maintenance can happen inside the normal AI-driven workflow rather than requiring a human to drop to a terminal + +**Priority:** P2 +**Independent Test Criteria:** The `metta-guard-bash` allow-list authorizes `milestone close` and `milestone update` under the same trust rules as existing milestone verbs, with no change to the trust-tier model. + +**Acceptance Criteria:** +- **Given** an authorized skill context that can already invoke existing milestone verbs **When** it invokes `metta milestone close ` or `metta milestone update ` **Then** the guard hook permits the command +- **Given** an unauthorized context **When** it attempts the new verbs **Then** the guard blocks them exactly as it blocks existing milestone verbs diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/summary.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/summary.md new file mode 100644 index 00000000..b9460801 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/summary.md @@ -0,0 +1,36 @@ +# Summary: fix-milestone-status-write-once-dead-field-no-close + +## What changed + +Milestones gained a validated write-back lifecycle. Previously `metta milestone create` wrote `status: open` permanently — no CLI path could close a milestone, edit its body, or change its target (issue `milestone-status-is-a-write-once-dead-field-with-no-close-or`). + +## Implementation (7 tasks, 4 batches) + +- **Schema** (`src/schemas/milestone-frontmatter.ts`): status enum extended to `open | closed | abandoned`; default and `.strict()` unchanged. Commit `3864badc5`. +- **Store** (`src/milestones/milestones-store.ts`): exported `MilestonePatch` and `update(slug, patch)` — read → patch → full-frontmatter Zod re-validation **before any I/O** → write; failing patches provably leave the file byte-identical; `clearTarget` removes the key entirely. Commit `3864badc5`. +- **Rollup** (`src/milestones/milestone-rollup.ts`): two-state sort replaced with a rank comparator (open first, terminal group slug-ascending — behavior-identical for open/closed-only inputs); exported shared `MILESTONE_MARKERS` (`▸`/`✓`/`✗`). Commit `959e2805d`. +- **CLI** (`src/cli/commands/milestone.ts`): new `milestone close [--abandoned]` (conflict pre-check, `chore: close milestone ` auto-commit) and `milestone update ` (`--name/--target/--clear-target/--description/--status`, Commander `conflicts`/`choices`, `chore: update milestone `); shared `commitMilestones` helper extracted from `create`; all failures exit 4 with typed JSON envelopes (`branch_guard`/`not_found`/`milestone_conflict`/`milestone_error`). Commit `5a4e3406d`. +- **Renderers** (`src/cli/commands/status.ts`, `progress.ts`): abandoned milestones render red `✗` via `MILESTONE_MARKERS`; open/closed output byte-identical to pre-change. Commits `e0063d78b`, `6429c7b88`. +- **Guard/mint hooks** (deployed + `src/templates/hooks/` mirrors, byte-identical): `milestone close`/`update` join the Tier-2 blocked set; `SKILL_SCOPES['metta-backlog']` gains `milestone:close`/`milestone:update`; 7 new guard test cases close the previously-empty milestone coverage gap. Commit `8541e2f2b`. +- **Skill** (`.claude/skills/metta-backlog/SKILL.md` + template mirror): milestone actions now `create | list | show | close | update` with dispatch branches for both new verbs. Commit `df57b4692`. + +## Verification during implementation + +Every task ran `npx tsc --noEmit` (clean) and its focused vitest suites (all green), including 309 guard-hook tests, byte-identity pins for hook mirrors, byte-compat ordering pins for the rollup sort, and byte-identical-file assertions for all failure paths. + +## Notable deviations + +- Task 1.1 pre-widened `MilestoneRollup.status` (one line) because the enum extension broke compilation — work the design assigned to the rollup component anyway. +- Task 3.3's test landed in `tests/cli-status.test.ts` (the file actually covering progress milestone rendering) rather than the plan's speculative `progress-secondary-line.test.ts`; a brief mid-batch file overlap between tasks 3.2/3.3 was reconciled with no lost work. + +## Risks + +- `status: abandoned` files fail validation under older metta builds (accepted one-way door, documented in intent). +- `update` re-serializes frontmatter via YAML.stringify — hand-edited key order/comments are normalized (accepted; hand-editing is the workflow this change eliminates). + +## Verification (3 parallel verifiers, 2 iterations) + +- **Tests**: iteration 1 found 3 failures — pre-existing scope-pin suites (`tests/metta-session-mint.test.ts`, `tests/cli-metta-guard-bash-integration.test.ts`) still expected the old `metta-backlog` scope list without `milestone:close`/`milestone:update`. Expectations updated (commit `4a9b24382`); iteration 2: **135 files, 2802 passed, 2 skipped, 0 failed**. +- **Typecheck/lint/build**: `npx tsc --noEmit` clean; `npm run lint` clean; `npm run build` (tsc + copy-templates + emit-build-stamp) succeeded. +- **Spec coverage**: all 22 scenarios across the 6 spec.md requirements have cited passing tests (per-scenario evidence table produced by the verifier). One noted caveat: R5's byte-compatibility scenario is verified via unit-level pins (legacy-comparator reproduction, marker map, envelope shapes) rather than a literal pre/post output byte-diff — the guarantee is derived, since pre-change binaries are not available in-tree. +- Review follow-up: the quality reviewer's single warning (no direct CLI test for `show` on an abandoned milestone) was closed by commit `26e0703f0`. diff --git a/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/tasks.md b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/tasks.md new file mode 100644 index 00000000..8b9e1d56 --- /dev/null +++ b/spec/archive/2026-08-26-fix-milestone-status-write-once-dead-field-no-close/tasks.md @@ -0,0 +1,57 @@ +# Tasks for fix-milestone-status-write-once-dead-field-no-close + +## Batch 1 (no dependencies) + +- [x] **Task 1.1: Schema enum extension + validated store update path** + - **Files**: `src/schemas/milestone-frontmatter.ts`, `src/milestones/milestones-store.ts`, `tests/milestones-store.test.ts` + - **Action**: (1) Extend the status enum at `milestone-frontmatter.ts:26` to `z.enum(['open', 'closed', 'abandoned']).default('open')`; leave `.strict()`, the `target` regex, and the real-calendar-date refinement untouched. (2) In `milestones-store.ts`: replace the `Milestone.status` literal union (line 14) with the schema-derived type `MilestoneFrontmatter['status']`; add exported `interface MilestonePatch { name?: string; target?: string; clearTarget?: boolean; status?: Milestone['status']; description?: string }`; add `async update(slug: string, patch: MilestonePatch): Promise` implementing the design flow exactly: `assertSafeSlug` → throw `clearTarget and target are mutually exclusive` when both set → exists check throwing `` `Milestone '${slug}' not found` `` (exact text of `show`, never creating a file) → `readRaw` + `parseMilestone` → build next frontmatter (`name`/`status` via `??` fallback; `target` key present iff `!patch.clearTarget` and `patch.target ?? current.target` is defined) → `validateFrontmatter(next, relPath)` **before any I/O** → `writeRaw(relPath, formatMilestone(validated, patch.description ?? current.description))` → return the updated `Milestone`. `create`/`list`/`show`/`exists` untouched. No barrel edit needed (`export *` already covers the module). (3) Extend `tests/milestones-store.test.ts` with the design's cases: status patch preserves name/target/body; `clearTarget` removes the key (assert raw file has no `target:` line, not `target: null`); invalid patch (`target: '2026-02-30'`, empty name) throws naming the field AND file byte-identical to a pre-call snapshot; `target`+`clearTarget` throws; not-found throws with no file created under `milestones/`; `abandoned` round-trips through `show`; seeded `status: abandoned` file validates; seeded `status: shipped` rejects naming the allowed values; seeded pre-change `open`/`closed` files parse identically (back-compat pin). + - **Verify**: `npx tsc --noEmit` clean; `npx vitest run tests/milestones-store.test.ts` passes (all new cases plus pre-existing ones). + - **Done**: Enum accepts `abandoned` with unchanged default; `update()` matches the design signature and error table (all plain `Error`, exact messages); a failing patch provably leaves the file byte-identical (validation precedes all I/O); untouched fields preserved; empty patch is a validated no-op at store level; tree compiles. + +## Batch 2 (depends on Batch 1) + +- [x] **Task 2.1: Rollup rank comparator + shared MILESTONE_MARKERS** + - **Depends on**: Task 1.1 + - **Files**: `src/milestones/milestone-rollup.ts`, `tests/milestone-rollup.test.ts` + - **Action**: (1) Widen `MilestoneRollup.status` (line 7) to `Milestone['status']`. (2) Replace the two-state comparator at lines 77-80 with the design's exact rank comparator (`rank = s === 'open' ? 0 : 1`, then slug ascending) and update the doc comment to "sorted open-first, then terminal, then slug ascending". (3) Add `export const MILESTONE_MARKERS = { open: '▸', closed: '✓', abandoned: '✗' } as const` as the single glyph source for all three render sites. (4) Extend `tests/milestone-rollup.test.ts`: mixed open/closed/abandoned sorts open-first then terminal slug-ascending; open/closed-only ordering unchanged (byte-compat pin proving the rank comparator reproduces the old comparator for two-state inputs); `abandoned` passes through the rollup row; `MILESTONE_MARKERS` covers all three statuses. + - **Verify**: `npx tsc --noEmit` clean; `npx vitest run tests/milestone-rollup.test.ts` passes including the byte-compat ordering pin. + - **Done**: `abandoned` sorts in the terminal group; open/closed-only inputs order identically to pre-change; `MILESTONE_MARKERS` exported and flowing through the existing `src/index.ts` barrel with no barrel edit. + +## Batch 3 (depends on Batch 2) — tasks 3.1–3.3 touch disjoint files and run in parallel + +- [x] **Task 3.1: CLI `milestone close` / `milestone update` verbs + commitMilestones helper + list marker** + - **Depends on**: Task 1.1, Task 2.1 + - **Files**: `src/cli/commands/milestone.ts`, `tests/cli-milestone.test.ts` + - **Action**: (1) Extract module-private `async function commitMilestones(projectRoot, message): Promise<{ committed: boolean; commitSha?: string }>` verbatim from `create`'s commit block (lines 77-87: `git add spec/milestones` → `commit -m` → `rev-parse HEAD`, whole block swallow-on-failure); refactor `create` onto it passing `chore: create milestone ${slug}` (behavior-preserving). (2) Register `close` after `create` with `--abandoned` and `--on-branch `, following the design action flow: context/config/`assertOnMainBranch` preamble → `show(slug)` → conflict pre-check `current.status !== 'open'` → exit 4 `milestone_conflict` with message `` `Milestone '${slug}' is already ${current.status}` `` and **no store call** → `update(slug, { status: options.abandoned ? 'abandoned' : 'closed' })` → `commitMilestones(..., `chore: close milestone ${slug}`)` (one message for both variants) → JSON `{ slug, status, committed, commit_sha }` / text `Closed milestone:`/`Abandoned milestone:` + committed line. Catch maps `Refusing to write` → `branch_guard`, `not found` → `not_found`, else `milestone_error`; envelope `{ error: { code: 4, type, message } }` under `--json`, plain stderr otherwise; `process.exit(4)`. (3) Register `update` with `--name`, `--target` via `new Option(...).conflicts('clearTarget')`, `--clear-target`, `--description`, `--status` via `new Option(...).choices(['open', 'closed', 'abandoned'])`, `--on-branch`; import ripple `import { Command, Option } from 'commander'`. Flow: same preamble → no-field-options pre-check (exit 4 `milestone_error`, message `At least one field option is required (--name, --target, --clear-target, --description, --status)`, no store call) → build `MilestonePatch` by conditional spread of provided options only → `update(slug, patch)` → `commitMilestones(..., `chore: update milestone ${slug}`)` → JSON `{ slug, changed, committed, commit_sha }` (`clearTarget` reports as `'target'` in `changed`) / text `Updated milestone: ()`. Note in both descriptions that `update --status` is the explicit override/reopen path. (4) `list` renderer (line 121): ternary → `MILESTONE_MARKERS[r.status]` (uncolored). `show` unchanged. (5) Extend `tests/cli-milestone.test.ts` with the design's cases: close happy path (frontmatter `status: closed`, `git log` shows `chore: close milestone `, JSON shape); `close --abandoned` writes `abandoned` with the same commit message; close already-closed → exit 4 `milestone_conflict` naming current status + byte-identical file; close missing slug → exit 4 `not_found`, no file created; branch-guard refusal without `--on-branch`; `update --description` replaces body only; `--clear-target` removes the key; `--status open` reopens a closed milestone; `--target 2026-02-30 --json` → exit 4, envelope names `target`, byte-identical file; update missing slug → `not_found`; zero field options → exit 4, file untouched; `--target` + `--clear-target` rejected by Commander (its own non-zero exit); text-mode list alignment assertion covering the `✗` glyph column width. + - **Verify**: `npx tsc --noEmit` clean; `npx vitest run tests/cli-milestone.test.ts` passes. + - **Done**: Both verbs match the design's exit-code/envelope table (all failures exit 4 with types `branch_guard`/`not_found`/`milestone_conflict`/`milestone_error`); conflict and no-op paths provably never touch the file; `create` behavior unchanged after the helper refactor. + +- [x] **Task 3.2: status.ts abandoned marker rendering** + - **Depends on**: Task 2.1 + - **Files**: `src/cli/commands/status.ts`, `tests/cli-status.test.ts` + - **Action**: In `printMilestoneSection` (line 33) replace the two-state ternary with a lookup over `MILESTONE_MARKERS` glyphs plus a local color-code map `{ open: 36, closed: 32, abandoned: 31 }` (cyan `▸`, green `✓`, red `✗` — colors stay local to the site per design). Extend `tests/cli-status.test.ts` with one case: an abandoned milestone renders exit 0 with `✗`, sorted after open milestones; existing open/closed assertions stand unmodified as byte-compat pins. + - **Verify**: `npx tsc --noEmit` clean; `npx vitest run tests/cli-status.test.ts` passes with existing assertions untouched. + - **Done**: `metta status` renders abandoned milestones without crashing, `✗` in red, open/closed output byte-identical to pre-change. + +- [x] **Task 3.3: progress.ts abandoned marker rendering** + - **Depends on**: Task 2.1 + - **Files**: `src/cli/commands/progress.ts`, `tests/progress-secondary-line.test.ts` (or the sibling progress test file if milestone rendering is covered there) + - **Action**: At line 217 apply the identical treatment to Task 3.2: `MILESTONE_MARKERS` glyph lookup with the local color map `{ open: 36, closed: 32, abandoned: 31 }` (red — not grey — for `✗`, distinct from the adjacent 90-grey target text). Add one test case: an abandoned milestone renders exit 0 with `✗`, sorted after open; existing open/closed assertions stand as byte-compat pins. + - **Verify**: `npx tsc --noEmit` clean; `npx vitest run` on the edited progress test file passes with existing assertions untouched. + - **Done**: `metta progress` renders abandoned milestones without crashing or mislabeling; open/closed-only output byte-identical to pre-change. + +## Batch 4 (depends on Batch 3) — tasks 4.1–4.2 touch disjoint files and run in parallel + +- [x] **Task 4.1: Guard + mint hook authorization for milestone close/update (deployed + template mirrors)** + - **Depends on**: Task 3.1 + - **Files**: `.claude/hooks/metta-guard-bash.mjs`, `src/templates/hooks/metta-guard-bash.mjs`, `.claude/hooks/metta-session-mint.mjs`, `src/templates/hooks/metta-session-mint.mjs`, `tests/metta-guard-bash.test.ts` + - **Action**: (1) Guard, line 81: `['milestone', new Set(['create'])]` → `['milestone', new Set(['create', 'close', 'update'])]`; extend the adjacent comment (lines 79-80) to name all three Tier-2 scope keys as minted only by the metta-backlog skill. No other guard logic changes — scope keys auto-derive at lines 902-905, and `milestone list`/`show` stay in `ALLOWED_TWO_WORD`. (2) Mint hook, line 35: append `'milestone:close', 'milestone:update'` to `SKILL_SCOPES['metta-backlog']`, with a comment noting the mint boundary (future ship/finalize closers need their own scope extension). (3) Mirror BOTH edits byte-identically into the `src/templates/hooks/` counterparts (`tests/hooks-byte-identity.test.ts` pins the pairs — no edit to it). (4) Extend `tests/metta-guard-bash.test.ts` (currently zero milestone-specific cases): `metta milestone close x` / `metta milestone update x` blocked without credential (exit 2, mirroring the `backlog add` case); allowed with a minted metta-backlog token covering `milestone:close`/`milestone:update` (existing token-fixture helpers); `milestone list`/`show` still allowed credential-free. + - **Verify**: `npx vitest run tests/metta-guard-bash.test.ts tests/hooks-byte-identity.test.ts` passes; `npx tsc --noEmit` clean. + - **Done**: New verbs ride the existing Tier-2 machinery identically to `milestone create`; deployed hooks and template mirrors are byte-identical; read-only verbs remain credential-free; guard test gap for milestone verbs closed. + +- [x] **Task 4.2: metta-backlog skill close/update branches (deployed + template mirror)** + - **Depends on**: Task 3.1 + - **Files**: `.claude/skills/metta-backlog/SKILL.md`, `src/templates/skills/metta-backlog/SKILL.md` + - **Action**: In both copies (same edit — keep them in sync; no byte-identity test pins skills, so verify the sync manually with `diff`): line 28 milestone action choices `create | list | show` → `create | list | show | close | update`, plus two dispatch branches per the design: **close** — run `metta milestone list --json`, present open-milestone slugs via `AskUserQuestion`, ask achieved (`closed`) vs dropped (`abandoned`), run `metta milestone close ` (append `--abandoned` for dropped), echo resulting status and commit line; **update** — present slugs the same way, collect fields to change (`name` / `target` / clear target / `description` / `status`) via `AskUserQuestion`, run `metta milestone update ` with only the matching flags, noting `--status open` as the reopen path. + - **Verify**: `diff .claude/skills/metta-backlog/SKILL.md src/templates/skills/metta-backlog/SKILL.md` on the edited sections shows the same content; `npx vitest run tests/skill-template-anchoring.test.ts` passes (lints both trees, no edit to it). + - **Done**: Both skill copies carry the five-action choice list and both dispatch branches; anchoring lint green; full suite (`npx vitest run`) and `npx tsc --noEmit` green across the finished change. diff --git a/spec/specs/issue-logging/spec.lock b/spec/specs/issue-logging/spec.lock index 13c8bdbb..5ae72849 100644 --- a/spec/specs/issue-logging/spec.lock +++ b/spec/specs/issue-logging/spec.lock @@ -1,6 +1,6 @@ -version: 30 -hash: sha256:43f91d9f2103 -updated: 2026-08-18T02:28:41.403Z +version: 31 +hash: sha256:0a7ed91dd6b4 +updated: 2026-08-26T01:06:03.015Z status: draft source: change requirements: @@ -75,11 +75,13 @@ requirements: - shipped-in-stamp-survives-the-new-archive-path - done-commits-only-the-archived-pair-of-paths - id: milestone-store-with-zod-validated-frontmatter-and-cli - hash: sha256:49b88f48b187 + hash: sha256:765a64cdca7f scenarios: - milestone-created-with-defaults - creating-a-duplicate-milestone-is-refused - invalid-milestone-status-is-rejected + - abandoned-status-validates-through-the-schema + - pre-existing-open-and-closed-files-are-unaffected - id: milestone-and-priority-assignment-via-issue-frontmatter hash: sha256:c64f15b00189 scenarios: @@ -163,3 +165,39 @@ requirements: - config-get-escapes-c1-controls-identically-to-outputjson - tasks-json-rendering-routes-through-the-shared-helper - helper-is-idempotent-and-precise-at-range-boundaries + - id: milestone-store-update-applies-validated-patches + hash: sha256:17dbd846f285 + scenarios: + - status-patch-preserves-untouched-fields + - target-is-cleared-from-frontmatter + - invalid-patch-is-rejected-and-the-file-is-untouched + - updating-a-missing-milestone-fails-without-side-effects + - id: milestone-close-cli-verb-transitions-to-a-terminal-state + hash: sha256:301e6616455d + scenarios: + - open-milestone-is-closed-and-auto-committed + - abandoned-flag-writes-the-abandoned-state + - closing-an-already-terminal-milestone-is-a-conflict + - closing-a-missing-milestone-reports-not-found + - main-branch-guard-applies-to-close + - id: milestone-update-cli-verb-edits-mutable-fields + hash: sha256:809066e704a6 + scenarios: + - description-is-replaced-without-touching-other-fields + - clear-target-removes-the-field + - a-mistakenly-closed-milestone-is-reopened + - invalid-field-value-fails-validation-and-leaves-the-file-untouched + - updating-a-missing-milestone-reports-not-found + - no-field-options-is-an-error + - id: renderers-and-rollups-handle-the-abandoned-state + hash: sha256:f13e879a6fd5 + scenarios: + - list-sorts-terminal-states-after-open-with-distinct-markers + - show-reports-the-abandoned-state-accurately + - status-and-progress-render-abandoned-without-crashing + - open-and-closed-output-stays-byte-compatible + - id: guard-authorization-for-milestone-close-and-update + hash: sha256:f2615db1215a + scenarios: + - authorized-skill-context-may-invoke-the-new-verbs + - unauthorized-context-is-blocked-identically diff --git a/spec/specs/issue-logging/spec.md b/spec/specs/issue-logging/spec.md index 97d23afe..3c433237 100644 --- a/spec/specs/issue-logging/spec.md +++ b/spec/specs/issue-logging/spec.md @@ -213,7 +213,7 @@ Fulfills: US-1 ## Requirement: Milestone store with Zod-validated frontmatter and CLI -Milestones MUST be stored as one markdown file per milestone at `spec/milestones/.md`. Each file MUST carry YAML frontmatter validated by a Zod schema with fields: `name` (string, required), `target` (ISO 8601 date string `YYYY-MM-DD`, optional), and `status` (enum `open` | `closed`, defaulting to `open`); the body below the frontmatter is the free-form description. The CLI MUST provide `metta milestone create --name [--target ] [--description ]`, `metta milestone list`, and `metta milestone show `. `create` MUST refuse to overwrite an existing milestone file. Invalid frontmatter values (e.g., a malformed `target` date or unknown `status`) MUST produce a clear validation error on read or write. The mutating subcommand `milestone create` MUST be registered with the orchestration guard as a Tier 2 (session-tier) scoped two-word form, consistent with `backlog add/done/promote`; the read-only `milestone list` and `milestone show` subcommands MUST be permitted without a session credential, consistent with other read-only commands. (Traces: US-3; intent proposal §2.) +Milestones MUST be stored as one markdown file per milestone at `spec/milestones/.md`. Each file MUST carry YAML frontmatter validated by a Zod schema with fields: `name` (string, required), `target` (ISO 8601 date string `YYYY-MM-DD`, optional), and `status` (enum `open` | `closed` | `abandoned`, defaulting to `open`); the body below the frontmatter is the free-form description. The CLI MUST provide `metta milestone create --name [--target ] [--description ]`, `metta milestone list`, `metta milestone show `, `metta milestone close `, and `metta milestone update `. `create` MUST refuse to overwrite an existing milestone file. Invalid frontmatter values (e.g., a malformed `target` date or unknown `status`) MUST produce a clear validation error on read or write. Existing milestone files carrying `status: open` or `status: closed` MUST continue to validate and behave exactly as before the enum extension. The mutating subcommands `milestone create`, `milestone close`, and `milestone update` MUST be registered with the orchestration guard as Tier 2 (session-tier) scoped two-word forms, consistent with `backlog add/done/promote`; the read-only `milestone list` and `milestone show` subcommands MUST be permitted without a session credential, consistent with other read-only commands. (Traces: US-3, US-5; intent proposal §2, §6.) ### Scenario: Milestone created with defaults - GIVEN no milestone `v0-6` exists @@ -228,8 +228,17 @@ Milestones MUST be stored as one markdown file per milestone at `spec/milestones ### Scenario: Invalid milestone status is rejected - GIVEN a milestone file whose frontmatter contains `status: shipped` - WHEN the milestone store reads the file -- THEN validation fails with an error naming the `status` field and the allowed values `open`, `closed` +- THEN validation fails with an error naming the `status` field and the allowed values `open`, `closed`, `abandoned` +### Scenario: Abandoned status validates through the schema +- GIVEN a milestone file whose frontmatter contains `status: abandoned` +- WHEN the milestone store reads the file +- THEN validation succeeds and the parsed milestone reports status `abandoned` + +### Scenario: Pre-existing open and closed files are unaffected +- GIVEN milestone files on disk carrying `status: open` and `status: closed` written before the enum extension +- WHEN any milestone command reads them +- THEN both files validate without error and produce the same parsed status values as before the change ## Requirement: Milestone and priority assignment via issue frontmatter @@ -513,3 +522,133 @@ Fulfills: US-3 - GIVEN inputs consisting of already-escaped JSON text, an empty string, and strings containing the boundary code points U+007E, U+007F, U+009F, and U+00A0 - WHEN the escape helper is invoked on each input - THEN already-escaped text is returned unchanged, the empty string is returned unchanged, U+007F and U+009F are escaped as backslash + `u007f` and backslash + `u009f` respectively, and U+007E and U+00A0 are left intact + + +## Requirement: Milestone store update applies validated patches + +`MilestonesStore` MUST expose an `update(slug, patch)` method that reads the existing milestone file at `spec/milestones/.md`, applies the patch, and writes the result back through the state store. The patch MUST support: changing `name`, setting or changing `target`, clearing `target` (removing the field from frontmatter), replacing the description body, and setting `status` to any value in the enum. Fields absent from the patch MUST be preserved unchanged. Before any write, the full resulting frontmatter MUST be re-validated through `MilestoneFrontmatterSchema`; a patch whose result fails validation MUST be rejected with a clear validation error and MUST leave the file on disk byte-identical to its pre-call state — no unvalidated state may reach disk. Updating a slug with no matching milestone file MUST fail with a clear not-found error naming the slug, and MUST NOT create a file. The existing `create`, `list`, `show`, and `exists` behaviors MUST be unchanged. (Traces: US-1, US-2; intent proposal §1.) + +### Scenario: Status patch preserves untouched fields +- GIVEN `spec/milestones/m1.md` with frontmatter `name: M1`, `target: 2026-09-30`, `status: open` and a non-empty description body +- WHEN `update('m1', { status: 'closed' })` is called +- THEN the rewritten file carries `status: closed`, retains `name: M1`, `target: 2026-09-30`, and the identical description body, and the written frontmatter passed `MilestoneFrontmatterSchema` validation before the write + +### Scenario: Target is cleared from frontmatter +- GIVEN a milestone whose frontmatter includes a `target` field +- WHEN `update` is called with a patch that clears the target +- THEN the rewritten frontmatter contains no `target` key (not `target: null`) and still passes schema validation + +### Scenario: Invalid patch is rejected and the file is untouched +- GIVEN an existing milestone file and a byte snapshot of its content +- WHEN `update` is called with a patch producing invalid frontmatter (e.g. `target: '2026-02-30'` or an empty `name`) +- THEN the call throws a validation error identifying the offending field, and the file on disk is byte-identical to the snapshot + +### Scenario: Updating a missing milestone fails without side effects +- GIVEN no file exists at `spec/milestones/ghost.md` +- WHEN `update('ghost', { status: 'closed' })` is called +- THEN the call throws an error stating milestone `ghost` was not found, and no file is created under `spec/milestones/` + + +## Requirement: Milestone close CLI verb transitions to a terminal state + +The CLI MUST provide `metta milestone close `, which transitions an `open` milestone to `status: closed`, or to `status: abandoned` when the `--abandoned` flag is passed. The write MUST go through the validated store update path. On success the command MUST auto-commit the milestone file with the conventional message `chore: close milestone `, following the `milestone create` commit pattern (git failure is swallowed, reported as uncommitted). The command MUST respect the same main-branch guard as `milestone create` (`--on-branch ` acknowledgment required off the configured main branch). Closing a milestone that is already `closed` or `abandoned` MUST fail with exit code 4 and a clear conflict error — rendered via the standard JSON error envelope (`{ error: { code, type, message } }`) under `--json`, plain stderr otherwise — leaving the file unmodified. Closing a slug with no matching milestone file MUST fail with exit code 4 and a not-found error, creating no file. Under `--json`, a successful close MUST emit a JSON result object including the slug, the resulting status, and commit information, consistent with `milestone create` output conventions. (Traces: US-1, US-3; intent proposal §3.) + +### Scenario: Open milestone is closed and auto-committed +- GIVEN `spec/milestones/m1.md` with `status: open` in a git repository on the main branch +- WHEN the user runs `metta milestone close m1` +- THEN the file's frontmatter reads `status: closed`, a commit exists with message `chore: close milestone m1`, and the command exits 0 reporting the closure + +### Scenario: Abandoned flag writes the abandoned state +- GIVEN `spec/milestones/m6.md` with `status: open` +- WHEN the user runs `metta milestone close m6 --abandoned` +- THEN the file's frontmatter reads `status: abandoned`, the result passed schema validation before write, and the transition is auto-committed with `chore: close milestone m6` + +### Scenario: Closing an already-terminal milestone is a conflict +- GIVEN `spec/milestones/m1.md` with `status: closed` and a byte snapshot of the file +- WHEN the user runs `metta milestone close m1 --json` +- THEN the command exits 4, stdout carries a JSON error envelope with a conflict-typed error naming the milestone's current status, and the file is byte-identical to the snapshot + +### Scenario: Closing a missing milestone reports not found +- GIVEN no file exists at `spec/milestones/ghost.md` +- WHEN the user runs `metta milestone close ghost --json` +- THEN the command exits 4 with a JSON error envelope of type `not_found`, and no file is created or modified + +### Scenario: Main-branch guard applies to close +- GIVEN the repository checkout is on a branch other than the configured main branch +- WHEN the user runs `metta milestone close m1` without `--on-branch` +- THEN the command refuses with the same branch-guard error behavior as `metta milestone create`, and the milestone file is unmodified + + +## Requirement: Milestone update CLI verb edits mutable fields + +The CLI MUST provide `metta milestone update `, which patches a milestone's mutable fields via options: `--name ` (rename display name), `--target ` (set or change target), `--clear-target` (remove target), `--description ` (replace the description body), and `--status ` (explicitly set status, including reopening a terminal milestone). Only fields named by the provided options may change; all other fields MUST be preserved. Invoking the command with no field options MUST fail with a clear error stating that at least one field option is required, leaving the file untouched. The write MUST go through the validated store update path: an update whose resulting frontmatter fails `MilestoneFrontmatterSchema` MUST exit 4 with a clear validation error (standard JSON error envelope under `--json`) and leave the file byte-identical. Updating a slug with no matching milestone file MUST exit 4 with a not-found error. The command MUST respect the same main-branch guard as `milestone create` and MUST auto-commit successful updates with the conventional message `chore: update milestone `, following the `create` commit pattern. Under `--json`, success MUST emit a JSON result object including the slug, the fields changed, and commit information. (Traces: US-2; intent proposal §4.) + +### Scenario: Description is replaced without touching other fields +- GIVEN `spec/milestones/m1.md` with `status: open`, a `target`, and a stale body reading "In flight as PR #24" +- WHEN the user runs `metta milestone update m1 --description "Shipped in v0.5.0"` +- THEN the body reads "Shipped in v0.5.0", the frontmatter `name`, `target`, and `status` values are unchanged, and the change is auto-committed with `chore: update milestone m1` + +### Scenario: Clear-target removes the field +- GIVEN a milestone with `target: 2026-09-30` +- WHEN the user runs `metta milestone update --clear-target` +- THEN the rewritten frontmatter contains no `target` key and passes schema validation + +### Scenario: A mistakenly closed milestone is reopened +- GIVEN a milestone with `status: closed` +- WHEN the user runs `metta milestone update --status open` +- THEN the frontmatter reads `status: open`, the write passed validation, and the change is auto-committed + +### Scenario: Invalid field value fails validation and leaves the file untouched +- GIVEN an existing milestone and a byte snapshot of its file +- WHEN the user runs `metta milestone update --target 2026-02-30 --json` +- THEN the command exits 4, stdout carries a JSON error envelope with a validation message naming `target`, and the file is byte-identical to the snapshot + +### Scenario: Updating a missing milestone reports not found +- GIVEN no file exists at `spec/milestones/ghost.md` +- WHEN the user runs `metta milestone update ghost --name "Ghost"` +- THEN the command exits 4 with a not-found error, and no file is created + +### Scenario: No field options is an error +- GIVEN an existing milestone +- WHEN the user runs `metta milestone update ` with no field options +- THEN the command exits non-zero with an error stating at least one field option is required, and the file is unmodified + + +## Requirement: Renderers and rollups handle the abandoned state + +Milestone rendering surfaces MUST handle `status: abandoned` as a terminal state grouped with `closed`. `computeMilestoneRollups` MUST sort open milestones first and terminal milestones (`closed` and `abandoned`) after them, slug-ascending within each group. `metta milestone list` MUST render a distinct marker for `abandoned` milestones (visually distinguishable from both the open marker `▸` and the closed marker `✓`). `metta milestone show` MUST report `Status: abandoned` (and the `--json` `status` field as `abandoned`) rather than mislabeling the state. The milestone sections of `metta status` and `metta progress`, which inherit the shared rollup path, MUST render `abandoned` milestones without crashing or mislabeling them. When only `open` and `closed` milestones exist, human and `--json` output of `milestone list`, `milestone show`, `metta status`, and `metta progress` MUST be byte-identical to pre-change behavior. (Traces: US-3, US-4; intent proposal §5.) + +### Scenario: List sorts terminal states after open with distinct markers +- GIVEN milestones in `open`, `closed`, and `abandoned` states +- WHEN the user runs `metta milestone list` +- THEN open milestones appear before both terminal milestones, each of the three states renders its own marker, and the `abandoned` row's marker differs from `▸` and `✓` + +### Scenario: Show reports the abandoned state accurately +- GIVEN an `abandoned` milestone +- WHEN the user runs `metta milestone show ` (human and `--json`) +- THEN the human output contains `Status: abandoned` and the JSON output carries `"status": "abandoned"` + +### Scenario: Status and progress render abandoned without crashing +- GIVEN at least one `abandoned` milestone exists alongside open milestones +- WHEN `metta status` or `metta progress` renders its milestone section +- THEN the command exits 0 and the abandoned milestone appears in the rollup sorted after open milestones, labeled with its abandoned state + +### Scenario: Open and closed output stays byte-compatible +- GIVEN a project containing only `open` and `closed` milestones +- WHEN `metta milestone list`, `metta milestone show`, `metta status`, and `metta progress` run in both human and `--json` modes +- THEN the output of each is byte-identical to the output produced before this change + + +## Requirement: Guard authorization for milestone close and update + +The `metta-guard-bash` hook's allow-list MUST authorize the scoped two-word forms `milestone close` and `milestone update` under the same Tier 2 (session-tier) trust rules as the existing `milestone create` verb, with no change to the two-tier trust model itself. Contexts not authorized for existing milestone mutating verbs MUST be blocked from the new verbs identically. (Traces: US-5; intent proposal §6.) + +### Scenario: Authorized skill context may invoke the new verbs +- GIVEN a skill context holding a valid session credential that authorizes `metta milestone create` +- WHEN it invokes `metta milestone close ` or `metta milestone update ` +- THEN the guard hook permits both commands + +### Scenario: Unauthorized context is blocked identically +- GIVEN a context without a valid session credential +- WHEN it attempts `metta milestone close ` or `metta milestone update ` +- THEN the guard blocks the commands with the same denial behavior it applies to `metta milestone create` diff --git a/src/cli/commands/milestone.ts b/src/cli/commands/milestone.ts index 4b4dc97a..f36b7eb5 100644 --- a/src/cli/commands/milestone.ts +++ b/src/cli/commands/milestone.ts @@ -1,15 +1,36 @@ -import { Command } from 'commander' +import { Command, Option } from 'commander' import { execFile } from 'node:child_process' import { join } from 'node:path' import { promisify } from 'node:util' import { assertOnMainBranch, createCliContext, outputJson, getErrorMessage } from '../helpers.js' import type { CliContext } from '../helpers.js' -import { computeMilestoneRollups } from '../../milestones/milestone-rollup.js' +import { computeMilestoneRollups, MILESTONE_MARKERS } from '../../milestones/milestone-rollup.js' import { stripControlSequences, stripControlSequencesMultiline } from '../../util/sanitize-text.js' import type { MilestoneRollup } from '../../milestones/milestone-rollup.js' +import type { MilestonePatch } from '../../milestones/milestones-store.js' const execAsync = promisify(execFile) +/** + * Auto-commit `spec/milestones/` with the given message. Swallows all git + * failures (git unavailable, nothing to commit) — commits are best-effort at + * the CLI edge, exactly as `create` behaved before extraction. + */ +async function commitMilestones( + projectRoot: string, + message: string, +): Promise<{ committed: boolean; commitSha?: string }> { + try { + await execAsync('git', ['add', join('spec', 'milestones')], { cwd: projectRoot }) + await execAsync('git', ['commit', '-m', message], { cwd: projectRoot }) + const { stdout } = await execAsync('git', ['rev-parse', 'HEAD'], { cwd: projectRoot }) + return { committed: true, commitSha: stdout.trim() } + } catch { + // git unavailable or nothing to commit — swallow silently + return { committed: false } + } +} + /** * Shared wiring helper for milestone rollups, reused by `status`/`progress`. * Composes `MilestonesStore` + `IssuesStore` + the pure @@ -74,17 +95,7 @@ export function registerMilestoneCommand(program: Command): void { description: options.description, }) - let committed = false - let commitSha: string | undefined - try { - await execAsync('git', ['add', join('spec', 'milestones')], { cwd: ctx.projectRoot }) - await execAsync('git', ['commit', '-m', `chore: create milestone ${slug}`], { cwd: ctx.projectRoot }) - const { stdout } = await execAsync('git', ['rev-parse', 'HEAD'], { cwd: ctx.projectRoot }) - committed = true - commitSha = stdout.trim() - } catch { - // git unavailable or nothing to commit — swallow silently - } + const { committed, commitSha } = await commitMilestones(ctx.projectRoot, `chore: create milestone ${slug}`) if (json) { outputJson({ slug, created: true, committed, commit_sha: commitSha }) @@ -100,6 +111,116 @@ export function registerMilestoneCommand(program: Command): void { } }) + milestone + .command('close') + .argument('', 'Milestone slug') + .option('--abandoned', 'Mark abandoned instead of closed') + .option('--on-branch ', 'Acknowledge non-main branch and proceed') + .description('Close (or abandon) an open milestone; reopen via `update --status open`') + .action(async (slug, options) => { + const json = program.opts().json + const ctx = createCliContext() + try { + const config = await ctx.configLoader.load() + const mainBranch = config.git?.pr_base ?? 'main' + await assertOnMainBranch(ctx.projectRoot, mainBranch, options.onBranch) + + const current = await ctx.milestonesStore.show(slug) + if (current.status !== 'open') { + // Conflict check before any store call — the file stays untouched. + const message = `Milestone '${slug}' is already ${current.status}` + if (json) { outputJson({ error: { code: 4, type: 'milestone_conflict', message } }) } else { console.error(message) } + process.exit(4) + } + + const status = options.abandoned ? 'abandoned' : 'closed' + await ctx.milestonesStore.update(slug, { status }) + const { committed, commitSha } = await commitMilestones(ctx.projectRoot, `chore: close milestone ${slug}`) + + if (json) { + outputJson({ slug, status, committed, commit_sha: commitSha }) + } else { + console.log(`${options.abandoned ? 'Abandoned' : 'Closed'} milestone: ${slug}`) + if (committed) { console.log(` Committed: ${commitSha?.slice(0, 7)}`) } + } + } catch (err) { + const message = getErrorMessage(err) + const type = message.startsWith('Refusing to write') + ? 'branch_guard' + : message.includes('not found') + ? 'not_found' + : 'milestone_error' + if (json) { outputJson({ error: { code: 4, type, message } }) } else { console.error(message) } + process.exit(4) + } + }) + + milestone + .command('update') + .argument('', 'Milestone slug') + .option('--name ', 'Rename display name') + .addOption(new Option('--target ', 'Set or change target date (YYYY-MM-DD)').conflicts('clearTarget')) + .option('--clear-target', 'Remove the target date') + .option('--description ', 'Replace the description body') + .addOption(new Option('--status ', 'Set status explicitly (reopen with --status open)').choices(['open', 'closed', 'abandoned'])) + .option('--on-branch ', 'Acknowledge non-main branch and proceed') + .description('Edit milestone fields (name, target, description, status); `--status` is the explicit override/reopen path') + .action(async (slug, options) => { + const json = program.opts().json + const ctx = createCliContext() + try { + const config = await ctx.configLoader.load() + const mainBranch = config.git?.pr_base ?? 'main' + await assertOnMainBranch(ctx.projectRoot, mainBranch, options.onBranch) + + const hasField = options.name !== undefined + || options.target !== undefined + || options.clearTarget !== undefined + || options.description !== undefined + || options.status !== undefined + if (!hasField) { + const message = 'At least one field option is required (--name, --target, --clear-target, --description, --status)' + if (json) { outputJson({ error: { code: 4, type: 'milestone_error', message } }) } else { console.error(message) } + process.exit(4) + } + + // Conditional spread: absent options never enter the patch, so + // untouched fields are preserved by the store. + const patch: MilestonePatch = { + ...(options.name !== undefined ? { name: options.name } : {}), + ...(options.target !== undefined ? { target: options.target } : {}), + ...(options.clearTarget !== undefined ? { clearTarget: true } : {}), + ...(options.description !== undefined ? { description: options.description } : {}), + ...(options.status !== undefined ? { status: options.status } : {}), + } + await ctx.milestonesStore.update(slug, patch) + + const changed: string[] = [] + if (options.name !== undefined) { changed.push('name') } + if (options.target !== undefined || options.clearTarget !== undefined) { changed.push('target') } + if (options.description !== undefined) { changed.push('description') } + if (options.status !== undefined) { changed.push('status') } + + const { committed, commitSha } = await commitMilestones(ctx.projectRoot, `chore: update milestone ${slug}`) + + if (json) { + outputJson({ slug, changed, committed, commit_sha: commitSha }) + } else { + console.log(`Updated milestone: ${slug} (${changed.join(', ')})`) + if (committed) { console.log(` Committed: ${commitSha?.slice(0, 7)}`) } + } + } catch (err) { + const message = getErrorMessage(err) + const type = message.startsWith('Refusing to write') + ? 'branch_guard' + : message.includes('not found') + ? 'not_found' + : 'milestone_error' + if (json) { outputJson({ error: { code: 4, type, message } }) } else { console.error(message) } + process.exit(4) + } + }) + milestone .command('list') .description('List milestones with rollup counts') @@ -118,7 +239,7 @@ export function registerMilestoneCommand(program: Command): void { for (const warning of warnings) { process.stderr.write(`Warning: ${warning}\n`) } if (rollups.length === 0) { console.log('No milestones.') } else { for (const r of rollups) { - const marker = r.status === 'closed' ? '✓' : '▸' + const marker = MILESTONE_MARKERS[r.status] const target = r.target !== undefined ? ` target ${r.target}` : '' console.log(` ${marker} ${r.slug.padEnd(30)} ${r.resolved}/${r.total} resolved (${r.percent}%)${target}`) } diff --git a/src/cli/commands/progress.ts b/src/cli/commands/progress.ts index 015db644..d209e7f0 100644 --- a/src/cli/commands/progress.ts +++ b/src/cli/commands/progress.ts @@ -7,6 +7,7 @@ import { getGitLogTimings } from '../../util/git-log-timings.js' import { getCeremonyCommitRatio, getArtifactsPerSmallChange, getModelEscalationRate, getAvgTokensPerChangeByTier, getLatestTag } from '../../util/ceremony-metrics.js' import { isArchivedChangeDir } from '../../util/archive-dirs.js' import { loadMilestoneRollups, toMilestoneCountsRow } from './milestone.js' +import { MILESTONE_MARKERS } from '../../milestones/milestone-rollup.js' import type { ArtifactTiming, ArtifactTokens } from '../../schemas/change-metadata.js' export function registerProgressCommand(program: Command): void { @@ -209,12 +210,14 @@ export function registerProgressCommand(program: Command): void { } // Milestones — section omitted entirely when no milestone files - // exist (closed milestones marked ✓, sorted after open by the - // rollup function). + // exist (terminal milestones marked ✓/✗, sorted after open by the + // rollup function). Abandoned renders red — distinct from the + // adjacent 90-grey target text. + const MILESTONE_MARKER_COLORS = { open: 36, closed: 32, abandoned: 31 } as const if (milestoneSection !== null) { console.log(color(' Milestones:', 36)) for (const r of milestoneSection.rollups) { - const marker = r.status === 'closed' ? color('✓', 32) : color('▸', 36) + const marker = color(MILESTONE_MARKERS[r.status], MILESTONE_MARKER_COLORS[r.status]) const target = r.target !== undefined ? ` ${color(`target ${r.target}`, 90)}` : '' console.log(` ${r.slug.padEnd(30)} ${marker} ${r.resolved}/${r.total} resolved (${r.percent}%)${target}`) } diff --git a/src/cli/commands/status.ts b/src/cli/commands/status.ts index 0f6911ee..cdbe1364 100644 --- a/src/cli/commands/status.ts +++ b/src/cli/commands/status.ts @@ -3,6 +3,7 @@ import { createCliContext, outputJson, color, getErrorMessage } from '../helpers import { renderStatusLine } from '../../complexity/index.js' import { checkFinalizeLockStale } from '../../finalize/finalize-lock.js' import { loadMilestoneRollups, toMilestoneCountsRow } from './milestone.js' +import { MILESTONE_MARKERS } from '../../milestones/milestone-rollup.js' import type { ChangeMetadata, ComplexityScore } from '../../schemas/change-metadata.js' type MilestoneSection = Awaited> @@ -24,13 +25,16 @@ function milestoneJsonKeys(section: MilestoneSection): Record { return keys } +/** ANSI color per milestone status — local to this render site by design. */ +const MILESTONE_MARKER_COLORS = { open: 36, closed: 32, abandoned: 31 } as const + /** Text `Milestones:` section — omitted entirely when no milestones exist. */ function printMilestoneSection(section: MilestoneSection): void { if (section === null) return console.log('') console.log('Milestones:') for (const r of section.rollups) { - const marker = r.status === 'closed' ? color('✓', 32) : color('▸', 36) + const marker = color(MILESTONE_MARKERS[r.status], MILESTONE_MARKER_COLORS[r.status]) const target = r.target !== undefined ? ` target ${r.target}` : '' console.log(` ${r.slug.padEnd(30)} ${marker} ${r.resolved}/${r.total} resolved (${r.percent}%)${target}`) } diff --git a/src/milestones/milestone-rollup.ts b/src/milestones/milestone-rollup.ts index 3e031807..f546e344 100644 --- a/src/milestones/milestone-rollup.ts +++ b/src/milestones/milestone-rollup.ts @@ -1,10 +1,16 @@ import type { Milestone } from './milestones-store.js' import type { IssueRecord } from '../issues/issues-store.js' +/** + * Single glyph source for milestone status markers across all render sites + * (milestone list, status, progress) so glyphs cannot drift. + */ +export const MILESTONE_MARKERS = { open: '▸', closed: '✓', abandoned: '✗' } as const + export interface MilestoneRollup { slug: string name: string - status: 'open' | 'closed' + status: Milestone['status'] target?: string open: number resolved: number @@ -20,7 +26,8 @@ export interface MilestoneRollup { * slug has no milestone file produce a warning string (naming the issue and * the unknown slug), never a failure. Issues without a `milestone` field * contribute to no bucket and no warning. Milestones with zero issues roll up - * 0/0/0 at 0%. Rollups are sorted open-first, then slug ascending. + * 0/0/0 at 0%. Rollups are sorted open-first, then terminal, then slug + * ascending. */ export function computeMilestoneRollups( milestones: Milestone[], @@ -74,10 +81,10 @@ export function computeMilestoneRollups( rollup.percent = rollup.total === 0 ? 0 : Math.round((rollup.resolved / rollup.total) * 100) } - rollups.sort((a, b) => { - if (a.status !== b.status) return a.status === 'open' ? -1 : 1 - return a.slug < b.slug ? -1 : a.slug > b.slug ? 1 : 0 - }) + const rank = (s: Milestone['status']): number => (s === 'open' ? 0 : 1) + rollups.sort( + (a, b) => rank(a.status) - rank(b.status) || (a.slug < b.slug ? -1 : a.slug > b.slug ? 1 : 0), + ) return { rollups, warnings } } diff --git a/src/milestones/milestones-store.ts b/src/milestones/milestones-store.ts index d9b5421d..f3d2fb80 100644 --- a/src/milestones/milestones-store.ts +++ b/src/milestones/milestones-store.ts @@ -11,10 +11,18 @@ export interface Milestone { slug: string name: string target?: string - status: 'open' | 'closed' + status: MilestoneFrontmatter['status'] description: string } +export interface MilestonePatch { + name?: string + target?: string + clearTarget?: boolean + status?: Milestone['status'] + description?: string +} + // Frontmatter block at offset 0: opening fence, YAML lines, closing fence on // its own line (file may end right at the closing fence). CRLF tolerated. const FRONTMATTER_RE = /^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/ @@ -136,6 +144,44 @@ export class MilestonesStore { return parseMilestone(content, slug, relPath) } + async update(slug: string, patch: MilestonePatch): Promise { + assertSafeSlug(slug) + + if (patch.target !== undefined && patch.clearTarget) { + throw new Error('clearTarget and target are mutually exclusive') + } + + const relPath = join('milestones', `${slug}.md`) + if (!(await this.state.exists(relPath))) { + throw new Error(`Milestone '${slug}' not found`) + } + + const content = await this.state.readRaw(relPath) + const current = parseMilestone(content, slug, relPath) + + const nextTarget = patch.clearTarget ? undefined : (patch.target ?? current.target) + const next = { + name: patch.name ?? current.name, + ...(nextTarget !== undefined ? { target: nextTarget } : {}), + status: patch.status ?? current.status, + } + + // Full resulting frontmatter re-validated before any I/O — a failing + // patch throws here and the file stays byte-identical by construction. + const validated = validateFrontmatter(next, relPath) + + const description = patch.description ?? current.description + await this.state.writeRaw(relPath, formatMilestone(validated, description)) + + return { + slug, + name: validated.name, + target: validated.target, + status: validated.status, + description: description.trim(), + } + } + async exists(slug: string): Promise { assertSafeSlug(slug) return this.state.exists(join('milestones', `${slug}.md`)) diff --git a/src/schemas/milestone-frontmatter.ts b/src/schemas/milestone-frontmatter.ts index 964d9688..aabd2ffe 100644 --- a/src/schemas/milestone-frontmatter.ts +++ b/src/schemas/milestone-frontmatter.ts @@ -23,7 +23,7 @@ export const MilestoneFrontmatterSchema = z.object({ .regex(ISO_DATE_RE) .refine(isRealCalendarDate, { message: 'target must be a real calendar date (YYYY-MM-DD)' }) .optional(), - status: z.enum(['open', 'closed']).default('open'), + status: z.enum(['open', 'closed', 'abandoned']).default('open'), }).strict() export type MilestoneFrontmatter = z.infer diff --git a/src/templates/hooks/metta-guard-bash.mjs b/src/templates/hooks/metta-guard-bash.mjs index 70a203ad..f7276016 100755 --- a/src/templates/hooks/metta-guard-bash.mjs +++ b/src/templates/hooks/metta-guard-bash.mjs @@ -76,9 +76,10 @@ const BLOCKED_SUBCOMMANDS = new Set([ const BLOCKED_TWO_WORD = new Map([ ['backlog', new Set(['add', 'done', 'promote', 'migrate'])], ['changes', new Set(['abandon'])], - // `milestone create` mutates state (writes spec/milestones/.md) — Tier-2 scope - // key 'milestone:create', minted only by the metta-backlog skill. - ['milestone', new Set(['create'])], + // `milestone create`/`close`/`update` mutate state (write spec/milestones/.md) — + // Tier-2 scope keys 'milestone:create', 'milestone:close', 'milestone:update', minted + // only by the metta-backlog skill. + ['milestone', new Set(['create', 'close', 'update'])], ['roadmap', new Set(['add', 'reorder', 'next', 'remove'])], // `release cut` mutates state (version bump, tag, release commit) — Tier-2 scope // key 'release:cut', minted only by the metta-release skill. diff --git a/src/templates/hooks/metta-session-mint.mjs b/src/templates/hooks/metta-session-mint.mjs index a0ac90dd..21633b3c 100755 --- a/src/templates/hooks/metta-session-mint.mjs +++ b/src/templates/hooks/metta-session-mint.mjs @@ -32,7 +32,9 @@ const SKILL_SCOPES = { 'metta-refresh': ['refresh'], 'metta-import': ['import'], 'metta-init': ['init', 'refresh'], - 'metta-backlog': ['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create'], + // Milestone mutation scopes mint only via metta-backlog; future ship/finalize-driven + // closers need their own scope extension here rather than reusing this one. + 'metta-backlog': ['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create', 'milestone:close', 'milestone:update'], 'metta-fix-gap': ['fix-gap', 'complete', 'finalize'], 'metta-roadmap': ['roadmap:add', 'roadmap:reorder', 'roadmap:next', 'roadmap:remove'], 'metta-release': ['release:cut'], diff --git a/src/templates/skills/metta-backlog/SKILL.md b/src/templates/skills/metta-backlog/SKILL.md index c6c45d97..c94c0e4a 100644 --- a/src/templates/skills/metta-backlog/SKILL.md +++ b/src/templates/skills/metta-backlog/SKILL.md @@ -25,10 +25,12 @@ Drive the `metta backlog` CLI. The backlog is a view over `spec/issues/` frontma - **promote** → run `metta backlog list --json`, parse `.backlog[].slug` from the output, present the slugs via `AskUserQuestion`, then run `metta backlog promote `. The CLI prints the `/metta-fix-issues ` handoff command; echo that back to the user. - **done** → run `metta backlog list --json`, parse `.backlog[].slug` from the output to build the list of available slugs. Present the slugs via `AskUserQuestion`. Then ask, via `AskUserQuestion`, for an optional change name to record as `--change ` (free-form; if the user skips or leaves blank, omit the flag). Run `metta backlog done ` or `metta backlog done --change ` as appropriate. Echo the archived path (`spec/issues/resolved/.md`) printed by the CLI back to the user. - **migrate** → run `metta backlog migrate --json`. Report the converted counts (`converted.active`, `converted.done`), any `collisions` (each with `slug`, `legacy_path`, `existing_path` — collisions are reported, never overwritten), and the `archived_to` location. If `nothing_to_do` is true, tell the user there were no legacy `spec/backlog/` files to migrate. - - **milestone** → ask via `AskUserQuestion` which milestone action to take: `create | list | show`. + - **milestone** → ask via `AskUserQuestion` which milestone action to take: `create | list | show | close | update`. - **create** → ask for `slug`, `name`, and optional `target` (date) and `description`. First run `metta milestone list --json` (allow-listed; lets the session-credential mint hook complete a prior Bash cycle — output can be ignored), then run `metta milestone create --name ""` plus `--target ` / `--description ` when supplied. - **list** → run `metta milestone list` and report the rollups (open/resolved counts and percent per milestone) plus any warnings. - **show** → run `metta milestone list --json`, present the milestone slugs via `AskUserQuestion`, then run `metta milestone show ` and report the per-issue breakdown. + - **close** → run `metta milestone list --json` (allow-listed; lets the session-credential mint hook complete a prior Bash cycle), present the open-milestone slugs via `AskUserQuestion`, then ask via `AskUserQuestion` whether the milestone was achieved (`closed`) or dropped (`abandoned`). Run `metta milestone close `, appending `--abandoned` when the user chose dropped. Echo the resulting status and the commit line printed by the CLI. + - **update** → run `metta milestone list --json`, present the milestone slugs via `AskUserQuestion`, then collect which fields to change via `AskUserQuestion`: `name` / `target` / clear target / `description` / `status`. Run `metta milestone update ` with only the flags matching the chosen fields — `--name ""`, `--target `, `--clear-target`, `--description ""`, `--status `. `--status open` is the reopen path for a closed or abandoned milestone. Echo the resulting fields and the commit line printed by the CLI. 3. Echo the slug / path / next command printed by the CLI. diff --git a/tests/cli-metta-guard-bash-integration.test.ts b/tests/cli-metta-guard-bash-integration.test.ts index 64bc2ea2..4aa1b05d 100644 --- a/tests/cli-metta-guard-bash-integration.test.ts +++ b/tests/cli-metta-guard-bash-integration.test.ts @@ -584,7 +584,7 @@ describe('metta-guard-bash integration', { timeout: 60_000 }, () => { expect(stderr).toContain('unknown') }) - it('mint hook scope for metta-backlog grants backlog:add/done/promote/migrate and milestone:create', () => { + it('mint hook scope for metta-backlog grants backlog:add/done/promote/migrate and milestone:create/close/update', () => { const mint = spawnSync('node', [MINT_TEMPLATE_PATH, 'metta-backlog'], { input: JSON.stringify(bashEvent('metta backlog list', { cwd: tempDir })), encoding: 'utf8', @@ -612,6 +612,8 @@ describe('metta-guard-bash integration', { timeout: 60_000 }, () => { 'backlog:promote', 'backlog:migrate', 'milestone:create', + 'milestone:close', + 'milestone:update', ]) // The minted credential authorizes the new Tier-2 mutations… diff --git a/tests/cli-milestone.test.ts b/tests/cli-milestone.test.ts index 74bb0fbf..d0c6f4a0 100644 --- a/tests/cli-milestone.test.ts +++ b/tests/cli-milestone.test.ts @@ -141,6 +141,32 @@ describe('CLI: milestone create / list / show', { timeout: 60000 }, () => { expect(stdout).toContain('target 2026-09-30') }) + it('text mode keeps columns aligned across ▸ / ✓ / ✗ markers', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'a-open', '--name', 'Open one'], tempDir) + await runCli(['milestone', 'create', 'b-closed', '--name', 'Closed one'], tempDir) + await runCli(['milestone', 'create', 'c-abandoned', '--name', 'Abandoned one'], tempDir) + await runCli(['milestone', 'close', 'b-closed'], tempDir) + await runCli(['milestone', 'close', 'c-abandoned', '--abandoned'], tempDir) + + const { stdout, code } = await runCli(['milestone', 'list'], tempDir) + expect(code).toBe(0) + const lines = stdout.split('\n').filter((l) => l.trim().length > 0) + expect(lines).toHaveLength(3) + + const markers = lines.map((l) => l.trim()[0]) + expect(markers).toContain('▸') + expect(markers).toContain('✓') + expect(markers).toContain('✗') + + // Every marker is a single UTF-16 unit at the same column, and the + // padEnd(30) slug column keeps the counts column aligned. + const slugStarts = lines.map((l) => l.search(/[a-z]/)) + expect(new Set(slugStarts).size).toBe(1) + const countStarts = lines.map((l) => l.search(/\d+\/\d+ resolved/)) + expect(new Set(countStarts).size).toBe(1) + }) + it('surfaces dangling milestone references as warnings with exit 0', async () => { await installFixture(tempDir) await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) @@ -173,6 +199,212 @@ describe('CLI: milestone create / list / show', { timeout: 60000 }, () => { }) }) + describe('metta milestone close', () => { + it('closes an open milestone: frontmatter, commit message, JSON shape', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + + const { stdout, code } = await runCli(['--json', 'milestone', 'close', 'v0-6'], tempDir) + expect(code).toBe(0) + const data = JSON.parse(stdout) + expect(data.slug).toBe('v0-6') + expect(data.status).toBe('closed') + expect(data.committed).toBe(true) + expect(data.commit_sha).toBeTruthy() + + const file = await readFile(join(tempDir, 'spec', 'milestones', 'v0-6.md'), 'utf8') + expect(file).toContain('status: closed') + const { stdout: log } = await execAsync('git', ['log', '--format=%s'], { cwd: tempDir }) + expect(log).toContain('chore: close milestone v0-6') + }) + + it('--abandoned writes abandoned with the same commit message', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + + const { stdout, code } = await runCli(['--json', 'milestone', 'close', 'v0-6', '--abandoned'], tempDir) + expect(code).toBe(0) + const data = JSON.parse(stdout) + expect(data.status).toBe('abandoned') + + const file = await readFile(join(tempDir, 'spec', 'milestones', 'v0-6.md'), 'utf8') + expect(file).toContain('status: abandoned') + const { stdout: log } = await execAsync('git', ['log', '--format=%s'], { cwd: tempDir }) + expect(log).toContain('chore: close milestone v0-6') + }) + + it('text mode reports Closed / Abandoned respectively', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'a-ms', '--name', 'A'], tempDir) + await runCli(['milestone', 'create', 'b-ms', '--name', 'B'], tempDir) + + const closed = await runCli(['milestone', 'close', 'a-ms'], tempDir) + expect(closed.code).toBe(0) + expect(closed.stdout).toContain('Closed milestone: a-ms') + + const abandoned = await runCli(['milestone', 'close', 'b-ms', '--abandoned'], tempDir) + expect(abandoned.code).toBe(0) + expect(abandoned.stdout).toContain('Abandoned milestone: b-ms') + }) + + it('already-closed milestone exits 4 with milestone_conflict and a byte-identical file', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + await runCli(['milestone', 'close', 'v0-6'], tempDir) + + const milestonePath = join(tempDir, 'spec', 'milestones', 'v0-6.md') + const bytesBefore = await readFile(milestonePath) + + const { stdout, code } = await runCli(['--json', 'milestone', 'close', 'v0-6'], tempDir) + expect(code).toBe(4) + const data = JSON.parse(stdout) + expect(data.error.code).toBe(4) + expect(data.error.type).toBe('milestone_conflict') + expect(data.error.message).toBe("Milestone 'v0-6' is already closed") + + const bytesAfter = await readFile(milestonePath) + expect(bytesAfter.equals(bytesBefore)).toBe(true) + }) + + it('missing slug exits 4 with not_found and creates no file', async () => { + await installFixture(tempDir) + const { stdout, code } = await runCli(['--json', 'milestone', 'close', 'does-not-exist'], tempDir) + expect(code).toBe(4) + const data = JSON.parse(stdout) + expect(data.error.type).toBe('not_found') + expect(data.error.message).toContain('does-not-exist') + expect(existsSync(join(tempDir, 'spec', 'milestones', 'does-not-exist.md'))).toBe(false) + }) + + it('blocks on a feature branch with branch_guard without --on-branch', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + await execAsync('git', ['checkout', '-b', 'metta/feature'], { cwd: tempDir }) + const { stdout, code } = await runCli(['--json', 'milestone', 'close', 'v0-6'], tempDir) + expect(code).toBe(4) + const data = JSON.parse(stdout) + expect(data.error.type).toBe('branch_guard') + }) + }) + + describe('metta milestone update', () => { + it('--description replaces the body only', async () => { + await installFixture(tempDir) + await runCli( + ['milestone', 'create', 'v0-6', '--name', 'v0.6', '--target', '2026-09-30', '--description', 'Old body'], + tempDir, + ) + + const { stdout, code } = await runCli( + ['--json', 'milestone', 'update', 'v0-6', '--description', 'New body'], + tempDir, + ) + expect(code).toBe(0) + const data = JSON.parse(stdout) + expect(data.slug).toBe('v0-6') + expect(data.changed).toEqual(['description']) + expect(data.committed).toBe(true) + + const file = await readFile(join(tempDir, 'spec', 'milestones', 'v0-6.md'), 'utf8') + expect(file).toContain('name: v0.6') + expect(file).toContain('target: 2026-09-30') + expect(file).toContain('status: open') + expect(file).toContain('New body') + expect(file).not.toContain('Old body') + const { stdout: log } = await execAsync('git', ['log', '--format=%s'], { cwd: tempDir }) + expect(log).toContain('chore: update milestone v0-6') + }) + + it('--clear-target removes the target key and reports changed: [target]', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6', '--target', '2026-09-30'], tempDir) + + const { stdout, code } = await runCli(['--json', 'milestone', 'update', 'v0-6', '--clear-target'], tempDir) + expect(code).toBe(0) + const data = JSON.parse(stdout) + expect(data.changed).toEqual(['target']) + + const file = await readFile(join(tempDir, 'spec', 'milestones', 'v0-6.md'), 'utf8') + expect(file).not.toContain('target:') + }) + + it('--status open reopens a closed milestone', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + await runCli(['milestone', 'close', 'v0-6'], tempDir) + + const { stdout, code } = await runCli(['--json', 'milestone', 'update', 'v0-6', '--status', 'open'], tempDir) + expect(code).toBe(0) + const data = JSON.parse(stdout) + expect(data.changed).toEqual(['status']) + + const show = await runCli(['--json', 'milestone', 'show', 'v0-6'], tempDir) + expect(JSON.parse(show.stdout).status).toBe('open') + }) + + it('invalid target date exits 4 naming target with a byte-identical file', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6', '--target', '2026-09-30'], tempDir) + const milestonePath = join(tempDir, 'spec', 'milestones', 'v0-6.md') + const bytesBefore = await readFile(milestonePath) + + const { stdout, code } = await runCli( + ['--json', 'milestone', 'update', 'v0-6', '--target', '2026-02-30'], + tempDir, + ) + expect(code).toBe(4) + const data = JSON.parse(stdout) + expect(data.error.code).toBe(4) + expect(data.error.type).toBe('milestone_error') + expect(data.error.message).toContain('target') + + const bytesAfter = await readFile(milestonePath) + expect(bytesAfter.equals(bytesBefore)).toBe(true) + }) + + it('missing slug exits 4 with not_found', async () => { + await installFixture(tempDir) + const { stdout, code } = await runCli( + ['--json', 'milestone', 'update', 'does-not-exist', '--name', 'New'], + tempDir, + ) + expect(code).toBe(4) + const data = JSON.parse(stdout) + expect(data.error.type).toBe('not_found') + expect(data.error.message).toContain('does-not-exist') + }) + + it('zero field options exits 4 with the file untouched', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + const milestonePath = join(tempDir, 'spec', 'milestones', 'v0-6.md') + const bytesBefore = await readFile(milestonePath) + + const { stdout, code } = await runCli(['--json', 'milestone', 'update', 'v0-6'], tempDir) + expect(code).toBe(4) + const data = JSON.parse(stdout) + expect(data.error.type).toBe('milestone_error') + expect(data.error.message).toBe( + 'At least one field option is required (--name, --target, --clear-target, --description, --status)', + ) + + const bytesAfter = await readFile(milestonePath) + expect(bytesAfter.equals(bytesBefore)).toBe(true) + }) + + it('--target with --clear-target is rejected by Commander', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + const { stderr, code } = await runCli( + ['milestone', 'update', 'v0-6', '--target', '2026-09-30', '--clear-target'], + tempDir, + ) + expect(code).not.toBe(0) + expect(code).not.toBe(4) + expect(stderr).toContain('cannot be used with') + }) + }) + describe('metta milestone show', () => { it('JSON carries per-issue detail with states', async () => { await installFixture(tempDir) @@ -252,6 +484,22 @@ describe('CLI: milestone create / list / show', { timeout: 60000 }, () => { expect(bytesAfter.equals(bytesBefore)).toBe(true) }) + it('reports the abandoned state accurately in both modes', async () => { + await installFixture(tempDir) + await runCli(['milestone', 'create', 'v0-6', '--name', 'v0.6'], tempDir) + const closed = await runCli(['--json', 'milestone', 'close', 'v0-6', '--abandoned'], tempDir) + expect(closed.code).toBe(0) + expect(JSON.parse(closed.stdout).status).toBe('abandoned') + + const text = await runCli(['milestone', 'show', 'v0-6'], tempDir) + expect(text.code).toBe(0) + expect(text.stdout).toContain('Status: abandoned') + + const json = await runCli(['--json', 'milestone', 'show', 'v0-6'], tempDir) + expect(json.code).toBe(0) + expect(JSON.parse(json.stdout).status).toBe('abandoned') + }) + it('unknown slug exits 4 with not_found', async () => { await installFixture(tempDir) const { stdout, code } = await runCli(['--json', 'milestone', 'show', 'does-not-exist'], tempDir) diff --git a/tests/cli-status.test.ts b/tests/cli-status.test.ts index 1fc48faa..66578cab 100644 --- a/tests/cli-status.test.ts +++ b/tests/cli-status.test.ts @@ -611,7 +611,7 @@ describe("CLI: status / next / changes / doctor / gate / validate-stories", { ti async function seedMilestone( slug: string, name: string, - opts: { target?: string; status?: 'open' | 'closed' } = {}, + opts: { target?: string; status?: 'open' | 'closed' | 'abandoned' } = {}, ): Promise { const dir = join(tempDir, 'spec', 'milestones') await mkdir(dir, { recursive: true }) @@ -788,6 +788,36 @@ describe("CLI: status / next / changes / doctor / gate / validate-stories", { ti expect(human.stdout).toContain('1/1 resolved (100%)') expect(human.stdout.indexOf('v0-6')).toBeLessThan(human.stdout.indexOf('v0-5')) }) + + it('status text renders abandoned milestone with ✗ after open milestones', async () => { + await installFixture(tempDir) + await disableWorktrees(tempDir) + await seedMilestone('v0-7', 'v0.7') + await seedMilestone('v0-4', 'v0.4', { status: 'abandoned' }) + await seedIssue('open-one', 'Open one', 'v0-7') + + const human = await runCli(['status'], tempDir) + expect(human.code).toBe(0) + expect(human.stdout).toContain('Milestones:') + expect(human.stdout).toContain('✗') + // Open sorts before abandoned (terminal statuses sort last). + expect(human.stdout.indexOf('v0-7')).toBeLessThan(human.stdout.indexOf('v0-4')) + }) + + it('progress text renders abandoned milestone with ✗ after open milestones', async () => { + await installFixture(tempDir) + await disableWorktrees(tempDir) + await seedMilestone('v0-7', 'v0.7') + await seedMilestone('v0-4', 'v0.4', { status: 'abandoned' }) + await seedIssue('open-one', 'Open one', 'v0-7') + + const human = await runCli(['progress'], tempDir) + expect(human.code).toBe(0) + expect(human.stdout).toContain('Milestones:') + expect(human.stdout).toContain('✗') + // Open sorts before abandoned (terminal statuses sort last). + expect(human.stdout.indexOf('v0-7')).toBeLessThan(human.stdout.indexOf('v0-4')) + }) }) }) diff --git a/tests/metta-guard-bash.test.ts b/tests/metta-guard-bash.test.ts index f2d0b3ec..383db7d6 100644 --- a/tests/metta-guard-bash.test.ts +++ b/tests/metta-guard-bash.test.ts @@ -105,6 +105,27 @@ describe('metta-guard-bash hook', { timeout: 30_000 }, () => { expect(code).toBe(2) }) + it('blocks `metta milestone close x` two-word without credential (exit 2)', () => { + const { code } = runHook(hookPath, bashEvent('metta milestone close x')) + expect(code).toBe(2) + }) + + it('blocks `metta milestone update x` two-word without credential (exit 2)', () => { + const { code } = runHook(hookPath, bashEvent('metta milestone update x')) + expect(code).toBe(2) + }) + + // Read-only milestone verbs stay credential-free. + it('allows `metta milestone list` two-word (exit 0)', () => { + const { code } = runHook(hookPath, bashEvent('metta milestone list')) + expect(code).toBe(0) + }) + + it('allows `metta milestone show x` two-word (exit 0)', () => { + const { code } = runHook(hookPath, bashEvent('metta milestone show x')) + expect(code).toBe(0) + }) + // ----- Unknown subcommands (conservative-block) ----- it('blocks unknown single-word `metta unknowncmd` conservatively (exit 2)', () => { const { code, stderr } = runHook(hookPath, bashEvent('metta unknowncmd')) @@ -988,6 +1009,49 @@ describe('metta-guard-bash hook', { timeout: 30_000 }, () => { expect(stderr).toBe('') }) + it('allows `metta milestone close x` with a metta-backlog token covering milestone:close (exit 0)', () => { + const cwd = makeTempCwd() + seedToken(cwd, { + skill: 'metta-backlog', + subcommands: [ + 'backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', + 'milestone:create', 'milestone:close', 'milestone:update', + ], + }) + const { code, stderr } = runHook(hookPath, bashEvent('metta milestone close x', { cwd }), { cwd }) + expect(code).toBe(0) + expect(stderr).toBe('') + }) + + it('allows `metta milestone update x` with a metta-backlog token covering milestone:update (exit 0)', () => { + const cwd = makeTempCwd() + seedToken(cwd, { + skill: 'metta-backlog', + subcommands: [ + 'backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', + 'milestone:create', 'milestone:close', 'milestone:update', + ], + }) + const { code, stderr } = runHook(hookPath, bashEvent('metta milestone update x', { cwd }), { cwd }) + expect(code).toBe(0) + expect(stderr).toBe('') + }) + + it('blocks `metta milestone close x` when the token scope lacks milestone:close (exit 2)', () => { + const cwd = makeTempCwd() + seedToken(cwd, { + skill: 'metta-backlog', + subcommands: ['backlog:add', 'backlog:done', 'backlog:promote', 'milestone:create'], + }) + const { code } = runHook(hookPath, bashEvent('metta milestone close x', { cwd }), { cwd }) + expect(code).toBe(2) + const entries = readAuditEntries(cwd) + const last = entries[entries.length - 1] + expect(last.verdict).toBe('block') + expect(last.reason).toBe('subcommand-not-in-scope') + expect(last.tier).toBe('session') + }) + it('blocks `metta roadmap remove x` when the token scope does not include roadmap:remove (exit 2)', () => { const cwd = makeTempCwd() seedToken(cwd, { diff --git a/tests/metta-session-mint.test.ts b/tests/metta-session-mint.test.ts index d101336b..27818724 100644 --- a/tests/metta-session-mint.test.ts +++ b/tests/metta-session-mint.test.ts @@ -33,7 +33,7 @@ const EXPECTED_SCOPES: Record = { 'metta-refresh': ['refresh'], 'metta-import': ['import'], 'metta-init': ['init', 'refresh'], - 'metta-backlog': ['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create'], + 'metta-backlog': ['backlog:add', 'backlog:done', 'backlog:promote', 'backlog:migrate', 'milestone:create', 'milestone:close', 'milestone:update'], 'metta-fix-gap': ['fix-gap', 'complete', 'finalize'], } diff --git a/tests/milestone-rollup.test.ts b/tests/milestone-rollup.test.ts index 33d6f94e..4fdd06da 100644 --- a/tests/milestone-rollup.test.ts +++ b/tests/milestone-rollup.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { computeMilestoneRollups } from '../src/milestones/milestone-rollup.js' +import { computeMilestoneRollups, MILESTONE_MARKERS } from '../src/milestones/milestone-rollup.js' import type { Milestone } from '../src/milestones/milestones-store.js' import type { IssueRecord } from '../src/issues/issues-store.js' @@ -137,6 +137,68 @@ describe('computeMilestoneRollups', () => { expect(rollups.map(r => r.slug)).toEqual(['v0-6', 'v0-8', 'v0-1', 'v0-5']) }) + it('sorts mixed open/closed/abandoned open-first, then terminal slug-ascending', () => { + const { rollups } = computeMilestoneRollups( + [ + milestone({ slug: 'v0-3', status: 'abandoned' }), + milestone({ slug: 'v0-5', status: 'closed' }), + milestone({ slug: 'v0-8' }), + milestone({ slug: 'v0-1', status: 'abandoned' }), + milestone({ slug: 'v0-4', status: 'closed' }), + milestone({ slug: 'v0-6' }), + ], + [], + [], + ) + // Open group first (slug ascending), then the terminal group (closed and + // abandoned interleaved) slug ascending — no closed/abandoned sub-ordering. + expect(rollups.map(r => r.slug)).toEqual(['v0-6', 'v0-8', 'v0-1', 'v0-3', 'v0-4', 'v0-5']) + }) + + it('orders open/closed-only inputs identically to the pre-rank comparator (byte-compat pin)', () => { + // The old comparator was: a.status !== b.status ? (a.status === 'open' ? -1 : 1) : slug cmp. + // Pin its output on a two-state permutation so the rank comparator provably reproduces it. + const input: Milestone[] = [ + milestone({ slug: 'z-closed', status: 'closed' }), + milestone({ slug: 'a-open' }), + milestone({ slug: 'a-closed', status: 'closed' }), + milestone({ slug: 'z-open' }), + milestone({ slug: 'm-open' }), + milestone({ slug: 'm-closed', status: 'closed' }), + ] + const legacyComparator = (a: Milestone, b: Milestone): number => { + if (a.status !== b.status) return a.status === 'open' ? -1 : 1 + return a.slug < b.slug ? -1 : a.slug > b.slug ? 1 : 0 + } + const expected = [...input].sort(legacyComparator).map(m => m.slug) + + const { rollups } = computeMilestoneRollups(input, [], []) + expect(rollups.map(r => r.slug)).toEqual(expected) + expect(rollups.map(r => r.slug)).toEqual([ + 'a-open', + 'm-open', + 'z-open', + 'a-closed', + 'm-closed', + 'z-closed', + ]) + }) + + it('passes abandoned through the rollup row', () => { + const { rollups, warnings } = computeMilestoneRollups( + [milestone({ slug: 'v0-2', status: 'abandoned' })], + [record({ slug: 'open-a', milestone: 'v0-2' })], + [record({ slug: 'done-a', milestone: 'v0-2' })], + ) + + expect(warnings).toEqual([]) + expect(rollups).toHaveLength(1) + expect(rollups[0].status).toBe('abandoned') + expect(rollups[0].open).toBe(1) + expect(rollups[0].resolved).toBe(1) + expect(rollups[0].total).toBe(2) + }) + it('returns empty rollups and no warnings for empty inputs', () => { expect(computeMilestoneRollups([], [], [])).toEqual({ rollups: [], warnings: [] }) }) @@ -146,3 +208,14 @@ describe('computeMilestoneRollups', () => { expect('target' in rollups[0]).toBe(false) }) }) + +describe('MILESTONE_MARKERS', () => { + it('covers all three statuses with distinct glyphs', () => { + expect(MILESTONE_MARKERS).toEqual({ open: '▸', closed: '✓', abandoned: '✗' }) + const statuses: Array = ['open', 'closed', 'abandoned'] + for (const status of statuses) { + expect(MILESTONE_MARKERS[status]).toBeTruthy() + } + expect(new Set(Object.values(MILESTONE_MARKERS)).size).toBe(3) + }) +}) diff --git a/tests/milestones-store.test.ts b/tests/milestones-store.test.ts index 7f5018c5..659f249e 100644 --- a/tests/milestones-store.test.ts +++ b/tests/milestones-store.test.ts @@ -81,15 +81,43 @@ describe('MilestonesStore', () => { expect(after).toBe(before) }) - it('rejects invalid status naming the allowed values open/closed', async () => { + it('rejects invalid status naming the allowed values open/closed/abandoned', async () => { await seedMilestoneFile('v0-6', '---\nname: v0.6\nstatus: shipped\n---\nbody\n') await expect(store.show('v0-6')).rejects.toThrow( - /status: .*'open' \| 'closed'.*received 'shipped'/, + /status: .*'open' \| 'closed' \| 'abandoned'.*received 'shipped'/, ) await expect(store.list()).rejects.toThrow(/status/) }) + it('accepts a seeded status: abandoned file', async () => { + await seedMilestoneFile('v0-6', '---\nname: v0.6\nstatus: abandoned\n---\ndropped\n') + + const milestone = await store.show('v0-6') + expect(milestone.status).toBe('abandoned') + expect(milestone.description).toBe('dropped') + }) + + it('parses seeded pre-change open and closed files identically (back-compat)', async () => { + await seedMilestoneFile('v0-6', '---\nname: v0.6\ntarget: "2026-09-30"\nstatus: open\n---\nrelease\n') + await seedMilestoneFile('v0-5', '---\nname: v0.5\nstatus: closed\n---\nshipped release\n') + + expect(await store.show('v0-6')).toEqual({ + slug: 'v0-6', + name: 'v0.6', + target: '2026-09-30', + status: 'open', + description: 'release', + }) + expect(await store.show('v0-5')).toEqual({ + slug: 'v0-5', + name: 'v0.5', + target: undefined, + status: 'closed', + description: 'shipped release', + }) + }) + it('rejects a malformed target naming the field', async () => { await seedMilestoneFile('v0-6', '---\nname: v0.6\ntarget: sometime soon\n---\n') await expect(store.show('v0-6')).rejects.toThrow(/target/) @@ -125,6 +153,109 @@ describe('MilestonesStore', () => { await expect(store.show('bare')).rejects.toThrow(/missing YAML frontmatter/) }) + describe('update', () => { + it('patches status while preserving name, target, and body', async () => { + await store.create('v0-6', { + name: 'v0.6', + target: '2026-09-30', + description: 'Backlog/milestone unification release.', + }) + + const updated = await store.update('v0-6', { status: 'closed' }) + expect(updated).toEqual({ + slug: 'v0-6', + name: 'v0.6', + target: '2026-09-30', + status: 'closed', + description: 'Backlog/milestone unification release.', + }) + + const shown = await store.show('v0-6') + expect(shown).toEqual(updated) + }) + + it('clearTarget removes the target key entirely from the raw file', async () => { + await store.create('v0-6', { name: 'v0.6', target: '2026-09-30', description: 'body' }) + + const updated = await store.update('v0-6', { clearTarget: true }) + expect(updated.target).toBeUndefined() + + const raw = await readFile(join(tempDir, 'milestones', 'v0-6.md'), 'utf-8') + expect(raw).not.toContain('target:') + expect(raw).toContain('name: v0.6') + expect(raw).toContain('body') + }) + + it('rejects an invalid target date naming the field and leaves the file byte-identical', async () => { + await store.create('v0-6', { name: 'v0.6', target: '2026-09-30', description: 'body' }) + const path = join(tempDir, 'milestones', 'v0-6.md') + const before = await readFile(path, 'utf-8') + + await expect(store.update('v0-6', { target: '2026-02-30' })).rejects.toThrow( + /target: .*real calendar date/, + ) + + expect(await readFile(path, 'utf-8')).toBe(before) + }) + + it('rejects an empty name naming the field and leaves the file byte-identical', async () => { + await store.create('v0-6', { name: 'v0.6', description: 'body' }) + const path = join(tempDir, 'milestones', 'v0-6.md') + const before = await readFile(path, 'utf-8') + + await expect(store.update('v0-6', { name: '' })).rejects.toThrow(/name/) + + expect(await readFile(path, 'utf-8')).toBe(before) + }) + + it('rejects target and clearTarget together', async () => { + await store.create('v0-6', { name: 'v0.6' }) + await expect( + store.update('v0-6', { target: '2026-09-30', clearTarget: true }), + ).rejects.toThrow(/clearTarget and target are mutually exclusive/) + }) + + it('throws not-found for a missing slug without creating a file', async () => { + await expect(store.update('never-created', { status: 'closed' })).rejects.toThrow( + /Milestone 'never-created' not found/, + ) + expect(await store.exists('never-created')).toBe(false) + }) + + it('round-trips abandoned status through show', async () => { + await store.create('v0-6', { name: 'v0.6', description: 'dropped scope' }) + + const updated = await store.update('v0-6', { status: 'abandoned' }) + expect(updated.status).toBe('abandoned') + + const shown = await store.show('v0-6') + expect(shown.status).toBe('abandoned') + expect(shown.description).toBe('dropped scope') + }) + + it('treats an empty patch as a validated no-op', async () => { + await store.create('v0-6', { name: 'v0.6', target: '2026-09-30', description: 'body' }) + const path = join(tempDir, 'milestones', 'v0-6.md') + const before = await readFile(path, 'utf-8') + + const updated = await store.update('v0-6', {}) + expect(updated).toEqual({ + slug: 'v0-6', + name: 'v0.6', + target: '2026-09-30', + status: 'open', + description: 'body', + }) + expect(await readFile(path, 'utf-8')).toBe(before) + }) + + it('rejects path-traversal slugs', async () => { + await expect(store.update('../escape', { status: 'closed' })).rejects.toThrow( + /Invalid milestone slug/, + ) + }) + }) + it('rejects path-traversal slugs on create/show/exists', async () => { const bad = ['../escape', '..\\escape', '/abs/path', 'a/b', 'Foo', ''] for (const slug of bad) {