* test(graphify): TDD-red design contract for #3170 commit-staleness signal
Captures the proposed extension to graphifyStatus() as 8 failing
assertions across 3 groups (git-aware, non-git, back-compat). Suite is
describe.skip()'d so npm test stays green on the branch — removing
.skip is the green-light moment when the enhancement is approved and
implementation lands.
Verified against safishamsi/graphify v0.7.0 release notes: the field
on graph.json is built_at_commit (full git HEAD), not commit_hash as
originally guessed in #3170. Tests assert against the verified name.
Design highlights captured in the file's docstring:
- Tri-state commit_stale (true/false/null) — null means "we don't
know" (pre-v0.7 graph or no git), distinct from false ("known fresh")
- Argument-injection fence /^[0-9a-f]{4,40}$/i validates built_at_commit
before it reaches `git` as an argv element
- Existing graphifyStatus() fields (node_count, edge_count, stale,
age_hours, etc.) are unchanged — back-compat fenced
Per the issue's enhancement template: no PR will be opened until the
issue is labeled `approved-enhancement`.
Refs #3170
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(graphify): surface commit-based staleness from graphify v0.7+ built_at_commit
Closes#3170
graphify v0.7+ embeds built_at_commit (full git HEAD) into graph.json at
write time. GSD's existing graphifyStatus() ignored it; staleness was
mtime-only, which is a poor proxy for "does this graph reflect the
current code." A CI-built graph rebuilt minutes ago against an old
checkout reads as FRESH on mtime but is materially stale.
graphifyStatus() now returns four additional fields on the success path:
built_at_commit short hash from graph.built_at_commit, or null
current_commit short hash of git HEAD, or null when no git
commits_behind git rev-list --count <built>..HEAD, or null
commit_stale true | false | null
Tri-state on commit_stale is load-bearing. null means "we don't know"
(pre-v0.7 graph, non-git cwd, unreachable commit) — semantically
distinct from false ("known fresh"). Agents reading null should fall
back to mtime; reading false can confidently skip a rebuild.
Security: built_at_commit is on-disk and user-influenceable. Without
validation, a hostile value (e.g. "--upload-pack=evil") would reach git
as an argv element and be interpreted as an option. The
/^[0-9a-f]{4,40}$/i fence rejects anything else as absent. spawnSync's
array args (no shell) is defense in depth, not the boundary.
Skill (commands/gsd/graphify.md) Step 2b renders one conditional line:
Source commit: abc1234 (5 commits behind HEAD)
Source commit: abc1234 (current)
Source commit: abc1234 (freshness unknown)
Pre-v0.7 graphs omit the line entirely — no confusing "Source commit:
unknown" rendered.
Also documents `graphify hook install` in docs/CONFIGURATION.md for
multi-dev teams who would otherwise hit graph.json merge conflicts on
parallel rebuilds (sub-enhancement 2 from #3170).
TDD red→green: tests/enh-3170-graphify-commit-staleness.test.cjs
(8 assertions across git-aware, non-git, back-compat) was committed
describe.skip()'d in c567f23d when the issue was filed; this commit
removes .skip and lands the implementation that makes them green.
Full suite 7503/7503 passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MAJOR (security/correctness):
- commands/gsd/debug.md: add Write to allowed-tools (session file creation
requires it — workflow explicitly says 'use Write tool, never heredoc')
- workflows/debug.md: add SLUG sanitization guard to steps 1b+1c (status/
continue subcommands used raw user input in file paths — path traversal)
- workflows/thread.md: sanitize $ARGUMENTS in RESUME mode before file path
construction (was bypassing the sanitization guard in CLOSE/STATUS modes)
MINOR (consistency/correctness):
- docs/INVENTORY-MANIFEST.json: remove stale top-level 'workflows' array
(duplicate of families.workflows introduced in earlier update)
- commands/gsd/resume-work.md: normalize process to 'Execute end-to-end.'
- commands/gsd/settings.md: normalize process to 'Execute end-to-end.'
- commands/gsd/update.md: normalize otherwise branch to 'execute end-to-end.'
- docs/adr/0002: add Status: Accepted + Date header (ADR convention)
- workflows/extract-learnings.md: rename step extract_learnings → extract-learnings
- tests/extract-learnings.test.cjs: tighten step-name assertion to exact name
ARCHITECTURE:
- scripts/command-contract-helpers.cjs: extract CANONICAL_TOOLS, parseFrontmatter,
executionContextRefs as shared module — single source of truth consumed by
both lint script and test suite (prevents silent lint/test disagreement)
- scripts/lint-command-contract.cjs: require() helpers instead of duplicating
- tests/command-contract.test.cjs: require() helpers; move readFileSync calls
inside test() callbacks (registration-time throws surface as named failures)
After extracting debug.md and thread.md implementations to workflow files
and renaming extract_learnings.md, existing tests still referenced the
old locations:
- debug-session-management.test.cjs: commands/gsd/debug.md → workflows/debug.md
- thread-session-management.test.cjs: commands/gsd/thread.md → workflows/thread.md
- extract-learnings.test.cjs: extract_learnings.md → extract-learnings.md
- enh-2430-learnings-consumption.test.cjs: extract_learnings.md → extract-learnings.md
Also adds <available_agent_types> block and TEXT_MODE fallback note to
get-shit-done/workflows/debug.md to satisfy the spawn-type-consistency
(#1357) and AskUserQuestion text-mode fallback (#2012) contract tests
that scan all workflow files.
* fix(#3121): implement commands verb in SDK native registry
- Add commandsList handler — returns sorted JSON array of all registered
verb strings; satisfies workstream-flag.md + agent tooling discoverability
- Register ['commands', commandsList] in DECISION_ROUTING_STATIC_CATALOG
- Add golden-policy exemption (SDK-only, no CJS mirror needed)
- check.decision-coverage-plan/verify were already registered; commands was the remaining gap
Closes#3121
* fix(#3135): restore workflows/add-backlog.md — capture --backlog had no workflow to load
Root cause: PR #2824 consolidated add-backlog into gsd-capture --backlog and
wired capture.md to delegate to workflows/add-backlog.md via execution_context.
The workflow file was never created (same gap class as reapply-patches.md which
was caught and fixed in the same PR). With no file to load, the agent had no
implementation steps to follow when --backlog was invoked.
Fix:
- Restore get-shit-done/workflows/add-backlog.md with full process from deleted
commands/gsd/add-backlog.md (phase.next-decimal, ROADMAP write, mkdir, commit)
- Preserve #2280 ordering invariant: ROADMAP entry written before directory
- Fix docs/INVENTORY.md: remove incorrect attribution of --backlog to add-todo.md,
add add-backlog.md row, bump workflow count 84→85
- Update docs/INVENTORY-MANIFEST.json
- Add regression test: every execution_context @-reference in commands/gsd/*.md
must resolve to an existing workflow file on disk
Closes#3135
* fix(#3127): make state.begin-phase idempotent on mid-flight phases
Root cause: cmdStateBeginPhase() unconditionally overwrote execution-
progress fields regardless of current phase status. When execute-phase
called it on a phase already mid-flight (--wave N resume), it regressed:
- Current Plan to 1 (from e.g. 3)
- Last Activity Description to 'context gathered; ready for plan-phase'
- Plan: N of M body line to 'Plan: 1 of M'
- last_updated timestamp to an older value
- progress.percent could decrease
Fix: read Status field before writing. If phase is already executing
(Status: Executing Phase N), skip execution-progress fields and only
update fields safe on resume:
- Last Activity date (always safe)
- Resume-specific 'execution resumed (wave continue)' activity line
First-time execution (Status != Executing Phase N) writes all fields
as before -- no regression on the normal path.
Regression test: 4 real unit tests using synthetic STATE.md files:
- mid-flight phase does not reset Current Plan (was the bug)
- mid-flight phase does not overwrite stopped_at narrative
- fresh phase sets Current Plan to 1 (normal path, no regression)
- both paths update Last Activity date (safe field)
Suite: 6990/6990. Closes#3127.
* fix(lint+state): allow-test-rule, escapeRegex phaseNumber in idempotency guard
* fix(#3097, #3099): add cwd-drift + absolute-path guards to executor worktree protocol
#3097 — cwd-drift sentinel (gsd-executor.md task_commit_protocol step 0a):
A Bash cd out of the worktree makes [ -f .git ] false, silently skipping
all HEAD/branch safety guards. Commits land on main's branch.
Fix: on first commit, capture spawn-time toplevel into sentinel file at
.git/worktrees/<name>/gsd-spawn-toplevel. Before every subsequent commit,
verify ACTUAL_TL matches EXPECTED_TL. Exits 1 with recovery instructions
if drift detected.
#3099 — absolute-path guard (gsd-executor.md task_commit_protocol step 0b):
Absolute paths constructed from the orchestrator's pwd (main repo root)
resolve to the main repo inside worktrees. Edit/Write lands in wrong dir;
git commit sees a clean worktree tree; work silently lost or leaks to main.
Fix: before any absolute-path Edit/Write, verify path starts with
WT_ROOT=/Users/thbouc/projects/get-shit-done. Prefer relative paths.
Both guards are documented in references/worktree-path-safety.md, which
is now loaded into every executor spawn prompt via <execution_context>.
The <worktree_branch_check> footnote references all three steps (0/0a/0b).
execute-phase.md: extracted worktree bash commands to reference file
(safe embed — @ files are inlined before the executor processes the prompt).
The blank line in <required_reading> was removed to stay at the XL=1700 line
budget after adding the @ reference.
Suite: 6986/6986. Closes#3097. Closes#3099.
* fix(lint+executor+docs): allow-test-rule, fix [ -f .git ] guard, fail-closed abs-path check, fix INVENTORY count
* fix(#3096): enforce sequential Steps 7+8 + Edit-only discipline in ai-integration-phase
Root cause: Steps 7 (gsd-ai-researcher) and 8 (gsd-domain-researcher)
were listed without an explicit sequential constraint. An orchestrator
optimizing for speed could parallelize them since sections appeared
disjoint. gsd-domain-researcher's Write at finalization replaced the
full AI-SPEC.md with its in-memory copy (pre-researcher state), losing
Sections 3/4. Confirmed at 40% incidence (2/5 agents on a real run).
Recovery cost: one extra ai-researcher dispatch, ~18 min wall.
Fix:
- Explicit 'MUST run sequentially' note on Step 7 (ordering note)
- 'Wait for Step 7 to complete before spawning Step 8' on Step 8
- Edit-only tool discipline injected into both agent prompts:
'Use Edit exclusively - NEVER use Write on this file'
prevents the last-writer-wins overwrite regardless of dispatch order
Suite: 7043/7043. Closes#3096.
* fix(lint): allow-test-rule for ai-integration-phase structural contract test
* fix(#3120): add register_authored_at_plan_time guard to secure-phase
Root cause: Step 3 short-circuit used threats_open: 0 as the sole
condition to skip directly to Step 6 (write clean SECURITY.md). It
did not distinguish empty-by-all-mitigated from empty-by-no-planning.
Legacy phases authored before <threat_model> blocks were canonical
received a rubber-stamped clean SECURITY.md with no audit performed.
Fix:
Step 2c: track register_authored_at_plan_time (true iff >=1 PLAN
file contained a parseable <threat_model> block)
Step 3: two-condition short-circuit:
- threats_open:0 AND register_authored_at_plan_time:true
-> skip to Step 6 (legitimate, all mitigated)
- threats_open:0 AND register_authored_at_plan_time:false
-> retroactive-STRIDE mode in Step 5 (build register
from implementation, then verify)
Step 5: auditor constraint varies by mode:
planned -> Verify mitigations exist, do not scan
retroactive -> Build STRIDE register first, then verify
Suite: 7039/7039. Closes#3120.
* fix(lint+changeset): allow-test-rule, drop dead regex branches, fix pr field to 3142
* fix(#3128): extend roadmap.cjs plan-count to match {N}-PLAN-{NN}-{slug}.md
Root cause: same regex flaw as #2893 (fixed in phase.cjs by #2896).
The manager-dashboard countPhasePlansAndSummaries() in roadmap.cjs was
not updated alongside the phase.cjs fix. Files like 5-PLAN-01-setup.md
end in -setup.md, not -PLAN.md, so plan_count returned 0.
Symptom: init manager returned plan_count=0 / disk_status=discussed for
fully-planned phases, triggering redundant background planner agents that
correctly detected existing plans and declined -- wasted runs.
Fix: apply the same looksLikePlanFile pattern from phase.cjs with
PLAN-OUTLINE and pre-bounce exclusions to countPhasePlansAndSummaries.
Regression test: tests/bug-3128-roadmap-plan-count-slug-layout.test.cjs
Suite: 6985/6985. Closes#3128.
* fix(lint): allow-test-rule for roadmap isPlanFile structural contract test
* fix(#3130): harden update.md npx invocations against cache-stale and token-routing
Two failure modes with the old form:
1. Cache-stale: npx serves a cached older version (no --package= flag)
2. Token-routing: Bash-tool wrapper misroutes @ token in package@tag spec
All three sibling invocations (local/global/unknown) now use:
npx -y --package=get-shit-done-cc@latest -- get-shit-done-cc $ARGS
--package= forces a fresh registry fetch; -- prevents token misrouting.
Also fixes the manual-update hint in the error-exit block.
Regression test: tests/bug-3130-update-npx-robust-invocation.test.cjs
Suite: 6973/6973 pass. Closes#3130.
* fix(lint): allow-test-rule for update.md structural contract test
- discuss-phase --assumptions → list-phase-assumptions.md
- pause-work --report → session-report.md
- manager --analyze-deps → analyze-dependencies.md
- import --from-gsd2 → gsd-tools.cjs from-gsd2 CLI
TDD: 8 new assertions in enh-2790-skill-consolidation.test.cjs
(argument-hint presence + body dispatch reference per flag).
Confirmed RED before wiring, GREEN after. Full suite 6971/6971.
help.md updated with all four new flag forms to satisfy
bug-2954-help-md-slash-command-stubs parity test.
Closes#3131
* feat(plan-phase): --research-phase flag absorbs deleted /gsd-research-phase + scrub stale refs (#3042, #3044)
#3042 (orphaned research-phase): /gsd-research-phase had a workflow file
but no slash-command stub. Rather than restore the orphan, the research-
only capability is now a flag on /gsd-plan-phase:
/gsd-plan-phase --research-phase <N>
When set, the workflow scopes to phase N, runs the research step (Section
5 of the existing plan-phase workflow), then early-exits before the
planner/plan-checker/verifier chain.
Per RCA against the deleted standalone, the flag adds two modifiers to
fully cover the original surface (Option B from the RCA discussion):
- --view : print existing RESEARCH.md to stdout, no spawn. Cheapest mode
for the correction-without-replanning loop the issue reporter
explicitly called out. Errors with a clear hint if RESEARCH.md is
missing.
- --research : reuse the existing "force re-research" semantics. In
research-only mode this skips the existing-RESEARCH.md prompt and
re-spawns unconditionally.
- Neither flag, RESEARCH.md exists : prompt update/view/skip. Mirrors
the deleted standalone's existing-artifact menu (#3042 RCA).
#3044 (stale slash-command refs): scrubbed five deleted commands from
all user-facing surfaces, including English docs, 4 localized doc sets
(ja-JP, ko-KR, zh-CN, pt-BR), workflows, templates, and references.
/gsd-check-todos → /gsd-capture --list
/gsd-new-workspace → /gsd-workspace --new
/gsd-status → /gsd-progress
/gsd-plan-milestone-gaps → table rows / orphan sections removed
(PR #3038 only scrubbed workflows/agent;
missed the docs surfaces this PR covers)
/gsd-research-phase → /gsd-plan-phase --research-phase
Includes a fix to docs/issue-driven-orchestration.md (PR #3036)
which itself referenced /gsd-new-workspace 4 times — self-correction.
Removed:
- get-shit-done/workflows/research-phase.md (orphan, capability
absorbed into --research-phase flag)
Tests:
- tests/bug-3042-3044-research-flag-and-stale-refs.test.cjs — 46
structural-IR tests across both bugs:
- argument-hint advertises --research-phase + --view
- workflow parses --research-phase, sets RESEARCH_ONLY,
early-exits before planner
- --view prints RESEARCH.md without spawning
- --research forces refresh in research-only mode
- existing-RESEARCH.md prompt path with update/view/skip
- workflows/research-phase.md is removed
- 5 deleted slash-commands absent from 17 English user-facing
surfaces + 16 localized doc surfaces (4 locales × 4 docs each)
- replacement command tokens present where deleted ones lived
6950/6950 full suite pass. Lints clean.
Closes#3042Closes#3044
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: address all 8 CR findings on PR #3045
Major (3):
- get-shit-done/workflows/plan-phase.md:344 — added explicit early-exit
guard at Section 5.1: "Skip if RESEARCH_ONLY=true". Without it, an LLM
could fall through "use existing, skip to step 6" → planner spawn,
violating the research-only contract. The guard makes the early-exit
unreachable from any non-research-only branch.
- get-shit-done/references/continuation-format.md (3 examples) +
zh-CN/.../continuation-format.md (3 examples) — pointed to
`/gsd-plan-phase --research-phase` but docs/COMMANDS.md didn't
document the flag. Added a full --research-phase + --view + --research
modifier section to the /gsd-plan-phase flag table in COMMANDS.md so
the canonical reference matches the continuation examples.
Minor (5):
- docs/FEATURES.md:1632 — `/gsd-plan-phase --research-phase` →
`/gsd-plan-phase --research-phase <N>` (include required arg).
- get-shit-done/templates/README.md:46 — NN-VALIDATION.md producer
reverted from `/gsd-plan-phase --research-phase` (Nyquist) to plain
`/gsd-plan-phase` (Nyquist). VALIDATION.md is created during normal
Nyquist flow, not research-only mode — the bulk replacement was
wrong for that line.
- get-shit-done/workflows/help.md:89 — signature line was missing
`--research`; added it alongside `--research-phase` and `--view`.
- tests/bug-3042-3044-...:197 — promptHasView/promptHasSkip were
tautological (matched anywhere in 1700-line workflow). Tightened
to a proximity check anchored on "RESEARCH.md already exists" prompt
header within a 600-char window. Updated workflow to emit that
literal phrase.
- tests/feat-2840-...:95 — workspace assertion used `/gsd-workspace`
but the documented replacement is `/gsd-workspace --new`. Tightened
to require both tokens (in 3 places: requiredCommands list, regex
in conceptPairs, error message).
6950/6950 full suite pass. Lint clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(workflows,docs): scrub stale /gsd-code-review-fix and /gsd-plan-milestone-gaps refs (#3029, #3034)
#2790 consolidated /gsd-code-review-fix into /gsd-code-review --fix and
deleted /gsd-plan-milestone-gaps in favor of inline gap planning as part
of /gsd-audit-milestone's output. The deletion was propagated through
some surfaces (#2950 covered help/do/settings/discuss-phase/etc.) but
several user-facing surfaces still emitted the old forms:
#3029 — /gsd-code-review-fix references in:
- agents/gsd-code-fixer.md (description, "Spawned by", recovery prose)
- get-shit-done/workflows/code-review.md (offer text)
- get-shit-done/workflows/execute-phase.md (offer text)
- get-shit-done/workflows/code-review-fix.md (internal retry hints)
- docs/INVENTORY.md (agent + workflow rows)
- docs/CONFIGURATION.md (workflow.code_review row)
- docs/USER-GUIDE.md (3 occurrences in walkthrough)
- docs/AGENTS.md (gsd-code-fixer agent stub)
- docs/FEATURES.md (commands list + REQ-REVIEW-04)
All replaced with /gsd-code-review --fix. Internal retry hints in the
workflow file itself updated to point at the new form. Release notes
(docs/RELEASE-*.md) and gsd-ns-review's "absorbed by" deletion note
left unchanged — historical/explanatory content.
#3034 — /gsd-plan-milestone-gaps references in:
- get-shit-done/workflows/audit-milestone.md (<offer_next> blocks for
gaps_found and tech_debt: lines 281, 323)
- commands/gsd/complete-milestone.md (gaps_found pre-flight: lines 46, 57)
Replaced with inline closure path:
/gsd-phase --insert <N> "Close gap: <REQ-ID> ..."
/gsd-discuss-phase <N>
/gsd-plan-phase <N>
/gsd-execute-phase <N>
Plus a Nyquist-coverage hint pointing at /gsd-validate-phase /
/gsd-secure-phase for retroactive audit-chain hygiene gaps. The
gsd-ns-project SKILL.md "deleted by #2790" note is preserved
(it's the canonical pointer for future readers asking what
happened to the command).
Tests:
- tests/bug-3029-3034-stale-command-routes.test.cjs — parser-based
assertions per fixed surface, plus a structural cross-check that
gsd-ns-project keeps the deletion note. 15 tests, all green.
- 6905/6905 full suite passes.
Closes#3029Closes#3034
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: address CR feedback on PR #3038 — argument order, structural tests, agent count
CR findings on PR #3038:
1. **docs/USER-GUIDE.md (Major)** — `--fix` examples used flag-first form
(`/gsd-code-review --fix 3`), but the supported CLI grammar is
phase-first (`/gsd-code-review 3 --fix`). The original sed-based
replacement preserved the position of the `gsd-code-review-fix`
token, producing the wrong order. Fixed in USER-GUIDE.md (3
occurrences) and the same drift in the workflow surfaces:
- get-shit-done/workflows/code-review-fix.md (2 retry hints)
- get-shit-done/workflows/code-review.md (offer text)
- get-shit-done/workflows/execute-phase.md (offer text)
2. **docs/AGENTS.md (Minor)** — internal count drift: line 483 said
"Ten additional agents" but line 725 said "12 advanced/specialized".
Filesystem reality: 33 agents total, 21 primary, 12 specialized
(count of `### ` stubs in the Advanced and Specialized section).
Updated lines 3, 13, 483 to use 12/33 and added the two missing
names (doc-classifier, doc-synthesizer) to the inline list at
line 13.
3. **tests:94 (Major refactor suggestion)** — `.includes()` token checks
were source-grep style. Refactored to a typed-IR pattern: extract
the SET of slash-command tokens via regex, assert membership on the
parsed Set instead of substring scanning the raw file text. Added
the `allow-test-rule` comment explaining the IR-build vs
IR-assertion split per scripts/lint-no-source-grep.cjs convention.
4. **tests:130 (Major)** — replacement-path assertion was file-wide and
could false-pass on generic mentions of "inline" elsewhere in the
file. Refactored: `extractOfferBlocks(content)` returns the typed
list of `<offer_next>` and "Pre-flight" blocks where the deleted
command previously lived, and the assertion runs against those
blocks specifically. Now requires `/gsd-phase --insert` or
inline-audit prose to appear in the same offer block, not just
somewhere in the file.
15/15 targeted tests pass. 6905/6905 full suite pass. Lints clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* docs(#3025): MCP tool schema as a context-budget concern
Adds documentation covering the largest GSD cost lever that GSD
itself does not own: MCP tool schema injection. Every enabled MCP
server adds its schema to every turn (often 20k+ tokens for
heavyweight servers like browser/playwright, mac-tools, etc.),
which can dwarf whatever `model_profile` tuning saves.
Two doc surfaces (per the bundle-docs-with-code skill depth gradient):
1. get-shit-done/references/context-budget.md
- New "MCP Tool Schema Cost (Harness Concern)" section.
- Explains schemas-per-turn cost framing.
- Names enabledMcpjsonServers / disabledMcpjsonServers and
.claude/settings.json explicitly.
- Pre-phase audit checklist: browser/playwright, platform-specific,
cross-project/stale, duplicate/shadow.
- Explicit "GSD does not manage MCP enablement — harness concern"
statement so users don't hunt for a GSD setting.
- Links to Anthropic Claude Code MCP docs as canonical reference.
- Notes compounding interaction with model_profile (additive levers).
2. docs/USER-GUIDE.md
- New task-oriented "Trim MCP servers to reduce per-turn cost"
section above "Using Non-Claude Runtimes".
- Same checklist condensed.
- Cross-link to context-budget.md for the full reference.
Tests:
- tests/feat-3025-mcp-token-budget-docs.test.cjs (12 cases) parses
both docs into typed semantic-flag records and asserts behavioral
invariants (mentions key, includes audit, names harness, etc.)
rather than substring-matching prose. Adheres to CONTRIBUTING.md
no-source-grep — section can be reworded freely as long as the
required semantics survive.
- Markdownlint pre-flight tests (MD040 fence language, MD056 table
column count) per the bundle-docs-with-code skill so CR can't
ratchet on prose nitpicks across multiple review rounds.
Verification:
- 12/12 pass on regression test
- 6857/6857 full suite (12 net new)
- lint-no-source-grep clean (377 test files)
Companion to #3023 (per-phase-type model map) and #3024 (dynamic
routing). Together they cover the three biggest cost levers users
ask about; this issue covers the one GSD does not own.
Closes#3025
* docs(#3025): batch 3 CR fixes — pr id, relative link, named flag
CodeRabbit on PR #3032 (3 minor — 2 inline + 1 nitpick), all in one
push per the bundle-docs-with-code skill (avoid per-round nitpick
ratchet):
1. Inline (Minor) — .changeset/mcp-token-budget-docs.md:3
`pr: TBD` → `pr: 3032` so changeset tooling can link the entry.
2. Inline (Minor) — docs/USER-GUIDE.md:1101
Used a hardcoded `https://github.com/.../blob/main/...` URL for the
cross-link to `context-budget.md`. Rest of USER-GUIDE.md uses
relative links. Switched to `../get-shit-done/references/context-
budget.md#mcp-tool-schema-cost-harness-concern` so feature-branch
work shows the right content and rename-resilience is preserved.
3. Nitpick — tests/feat-3025-mcp-token-budget-docs.test.cjs:234
The cross-link assertion used an inline `/context-budget/i.test(...)`
while every other invariant in the file lived as a named flag in
`parseMcpBudgetSection`. Per CONTRIBUTING.md no-source-grep, added
`crossLinksContextBudget` to the parser and asserted on
`parsed.crossLinksContextBudget` so the cross-link rule sits next
to its siblings.
Verification:
- 12/12 pass on regression test (no count change; refactor only)
- No source code changes, only docs + tests
* test(#3025): strip inline markdown before phrase-match (CR nitpick)
CodeRabbit caught that the `explainsHarnessNotGsd` primary regex
branch couldn't match "GSD does **not** manage" in
context-budget.md because the markdown bold markers (`**`) sit
between contiguous words. The test passed today only via the
fallback `harness (concern|setting|controlled)` branch — the
primary branch was effectively dead code.
Fix: strip inline markdown emphasis (`**`, `*`, `~~`) and inline-
code backticks before any phrase-matching in `parseMcpBudgetSection`.
All seven flag computations now run against the stripped text so
markdown formatting can't silently invalidate any invariant.
Underscores are intentionally NOT stripped — `model_profile` and
other snake_case identifiers must survive intact for the
mentionsModelProfileInteraction check to find them.
Verification: 12/12 still pass; primary branches now fire on
real markdown content rather than relying on fallbacks.
* test(#3025): guard markdownlint tests against null section (CR nitpick)
CodeRabbit caught that the MD040 and MD056 markdownlint pre-flight
tests called `section.match(...)` and `section.split('\n')`
directly on the value returned by `extractSection`, which returns
null when no matching header is found. If the MCP section is ever
removed (regression), both tests would throw `TypeError: Cannot
read properties of null` instead of producing a clean assertion
failure naming the actual problem.
The semantic tests above are protected because parseMcpBudgetSection
short-circuits to a typed-falsy record on null input. The
markdownlint tests bypassed that guard since they need raw section
text, not parsed flags. Added `assert.ok(section, ...)` preconditions
to both so a missing section produces a meaningful failure message.
No content changes; defensive programming only.
Verification: 12/12 still pass.
* feat(#3024): dynamic routing with failure-tier escalation
Adds a `dynamic_routing` block to .planning/config.json that lets
the resolver start agents on a cheap tier and escalate one tier up
when the orchestrator detects a soft failure (verification
inconclusive, plan-check FLAG, etc.). Solves the "pay Opus rates as
insurance" anti-pattern by making escalation observed-quality-driven.
Architecture:
- AGENT_DEFAULT_TIERS map (light/standard/heavy) — every agent in
MODEL_PROFILES declares a default tier; tests assert coverage
so adding a new agent without updating the map fails CI.
- nextTier(currentTier) helper — light → standard → heavy → heavy
(heavy stays at heavy; can't go further).
- resolveModelForTier(cwd, agentType, attempt) — new resolver. The
orchestrator tracks the attempt counter and passes 0 for the
first spawn, 1+ on escalation. The resolver caps internally at
max_escalations so the orchestrator can blindly bump the counter.
- Schema validation: dynamic_routing.enabled / escalate_on_failure /
max_escalations / tier_models.<light|standard|heavy>. Unknown
tiers and unknown sub-keys rejected at config-set time.
- SDK schema mirror updated to keep CJS/SDK in lockstep (#2653).
Resolution precedence (highest → lowest):
1. model_overrides[<agent>] (full IDs accepted)
2. dynamic_routing.tier_models[<tier>] (NEW; escalation-aware)
3. models[<phase_type>] (#3023 phase-type map)
4. model_profile (per-agent column)
5. Runtime default
Backward compatibility: dynamic_routing is disabled by default
(enabled: false or block omitted). resolveModelForTier short-
circuits to resolveModelInternal in that case, so callers can
adopt unconditionally without breaking existing behavior.
This PR delivers the JS-layer infrastructure: schema + tier map +
resolver. Orchestrator adoption (workflow markdown updates that
detect soft failures and call resolveModelForTier with attempt+1)
is incremental follow-up — verifier / plan-checker / integration-
checker each adopt the protocol when ready.
Tests (23 cases, all structural-IR — no stdout grep):
- Schema invariants: AGENT_DEFAULT_TIERS coverage, VALID_AGENT_TIERS
exact match, every assignment uses a valid tier
- nextTier helper: light→standard→heavy→heavy, null on invalid input
- Disabled mode: no block + enabled:false both no-op (back-compat)
- Enabled mode: attempt=0 returns default tier model, attempt=1
escalates, beyond max_escalations caps, heavy agents stay heavy,
default max_escalations=1 when omitted
- Precedence: per-agent override beats dynamic_routing,
dynamic_routing beats phase-type models
- Validation: every settings key accepted, unknown tiers/sub-keys
rejected, bare `dynamic_routing` rejected as config-set target
Documentation:
- get-shit-done/references/model-profiles.md — full reference section
- docs/CONFIGURATION.md — full settings table + escalation flow
- docs/USER-GUIDE.md — task-oriented "Cheap-by-default" section
- docs/FEATURES.md — config row cross-link
Verification:
- 23/23 pass on regression test
- 6843/6843 full suite (23 net new from 6820)
- lint-no-source-grep clean (376 test files)
- SDK schema mirror keeps CJS/SDK in sync per #2653 parity test
Closes#3024
* fix(#3024): honor escalate_on_failure:false + 3 CR follow-ups
CodeRabbit on PR #3031 (4 findings — 1 Major + 2 Minor + 1 Nitpick):
1. **Major (inline)** — get-shit-done/bin/lib/core.cjs:1668
resolveModelForTier ignored dynamic_routing.escalate_on_failure.
When the user set it to false, escalation should be disabled, but
the resolver only checked attempt/max_escalations. An orchestrator
that always passes attempt+1 on retry would silently escalate
despite the user opting out.
Fix: gate effectiveAttempt on `dr.escalate_on_failure !== false`
so false short-circuits every attempt back to the default tier.
2. **Minor (inline)** — docs/CONFIGURATION.md:123-126
The dynamic_routing rows in the Core Settings table had 4 cells
instead of 5 (missing the Options column), breaking the table
structure. Added explicit Options values for enabled / escalate_on_failure
/ max_escalations rows.
3. **Minor (outside-diff)** — references/model-profiles.md:179-195
"Resolution Logic" sketch was pre-#3024 and didn't include
dynamic_routing in the precedence ladder. Updated to a 6-step
block with dynamic_routing at step 3 (between override and
phase-type).
4. **Nitpick** — tests/feat-3024-dynamic-routing.test.cjs:189+
Tests used `if (lightAgent) { ... }` guards that silent-pass
when AGENT_DEFAULT_TIERS drifts. Replaced all 5 conditional
skips with `assert.ok(lightAgent, '...')` preconditions so a
tier-mapping change surfaces as a test failure.
Plus: 2 new regression tests for the Major fix:
- escalate_on_failure:false caps every attempt at default tier
- escalate_on_failure:true (explicit) still escalates normally
Verification:
- 25/25 pass on regression test (23 prior + 2 escalate_on_failure)
- 6845/6845 full suite (2 net new)
- lint-no-source-grep clean
* docs(#3024): align precedence + add fence language tags (CR follow-up)
CodeRabbit (3 minor):
1. docs/CONFIGURATION.md:691 — "Per-Phase-Type Models → Resolution
precedence" was a 4-step block written pre-#3024; readers got
contradictory rules between the per-phase-type section and the
later dynamic_routing section. Updated to the same 5-step ladder
with dynamic_routing at step 2, and noted that dynamic_routing
is disabled by default so this section's behavior is unchanged
when the kill-switch is off.
2. docs/CONFIGURATION.md:770 — escalation-flow code fence missing
language tag (MD040). Added `text`.
3. references/model-profiles.md:184 — resolution-ladder code fence
missing language tag (MD040). Added `text`.
No code changes; docs only. Verification: regression test still 25/25.
* docs(#3024): clarify precedence prose — five layers, not four (CR nitpick)
CodeRabbit nitpick: the "Per-Phase-Type Models → Resolution
precedence" prose said "The four layers compose..." but the ladder
above lists five (including Runtime default). Also "dynamic_routing
escalates per-attempt above all of them" misreads as suggesting
dynamic_routing wins over model_overrides — actually overrides still
win at step 1.
Reworded top-down so the precedence direction is unambiguous:
- model_profile = base
- models = phase-level override
- dynamic_routing = per-attempt escalation
- model_overrides = per-agent exception (top)
- runtime default = fallback
No code changes; docs only.
* docs(#3024): note escalate_on_failure:false in escalation-flow diagram (CR)
CodeRabbit nitpick: the escalation-flow diagram in
docs/CONFIGURATION.md described the soft-failure → respawn →
tier_models[next_tier_up] path, but didn't surface the
`dynamic_routing.escalate_on_failure: false` kill-switch right next
to it. Users reading the flow diagram (which is the canonical place
to understand attempt behavior) wouldn't see that the kill-switch
overrides the soft-failure branch.
Added a one-paragraph note immediately after the flow listing,
before the tier-sequence example, so the kill-switch is visible
exactly where users decide whether escalation will happen.
No code changes; docs only.
* feat(#3023): per-phase-type model map in .planning/config.json
Adds a new `models` block to .planning/config.json with six phase-type
slots (planning / discuss / research / execution / verification /
completion). Lets users express coarse tuning ("Opus for planning,
Sonnet for the rest") without learning the agent taxonomy.
Resolution precedence (highest → lowest):
1. Per-agent `model_overrides[agent]` (full IDs; targeted exception)
2. Phase-type `models[phase_type]` (NEW; tier alias)
3. Profile table (`model_profile`) (per-agent column)
4. Runtime default
The three layers compose: `models` defaults a phase, `model_overrides`
carves an exception. Phase-type values are tier aliases (opus/sonnet/
haiku/inherit) so the runtime-resolution chain (#2517) stays correct
end-to-end without further branching.
Implementation:
- model-profiles.cjs: new AGENT_TO_PHASE_TYPE map + VALID_PHASE_TYPES
set. Each agent in MODEL_PROFILES gets one phase-type assignment;
tests assert coverage so adding a new agent without updating the
table fails CI.
- core.cjs (resolveModelInternal): inserts phase-type tier lookup
between per-agent override and profile-derived tier. Skips runtime
resolution when the resolved tier is 'inherit' (was previously gated
only on profile === 'inherit'; phase-type can now produce inherit
independently).
- core.cjs (loadConfig): pass `parsed.models` through both code paths
so resolveModelInternal can read it.
- config-schema.cjs + sdk/src/query/config-schema.ts: dynamic-pattern
validator accepts only the six known phase-types; unknown slots
rejected at config-set time.
Backward compat: configs without `models` behave exactly as today.
Tests (15 cases, all structural-IR — no stdout grep):
- Schema: AGENT_TO_PHASE_TYPE coverage, VALID_PHASE_TYPES exact match
- Resolver: phase-type alone; per-agent override beats phase-type;
phase-type beats profile; issue's full example; "inherit"; empty
block is no-op; no block is no-op
- Validation: each of the 6 slots accepted; unknown slot rejected;
bare `models` (no slot) rejected
Verification:
- 15/15 pass on new regression test
- 6808/6808 full suite (5 net new), 0 fail
- lint-no-source-grep clean across 375 test files
Closes#3023
* docs(#3023): document `models` per-phase-type config in user-facing docs
Adds `models` block coverage to the three user-facing docs that ship
with each release:
1. docs/CONFIGURATION.md
- New "Per-Phase-Type Models" section between "Per-Agent Overrides"
and "Non-Claude Runtimes" with:
* full example mixing models + model_overrides
* phase-type → agent mapping table
* resolution-precedence pseudocode
* accepted values (tier alias only)
* "When to use which" decision matrix
* validation behavior + example error
- Added `"models": {}` to the Full Schema snippet
- Added a row for `models.<phase_type>` to the config keys table
(next to model_profile_overrides for adjacency)
2. docs/FEATURES.md
- Added a row for models.<phase_type> in the Configurable Settings
table (right under model_profile)
- Cross-link to CONFIGURATION.md for the full surface
3. docs/USER-GUIDE.md
- New task-oriented "Tuning model cost by phase" section above
"Using Non-Claude Runtimes" — leads with the concrete config
and shows the override pattern (one-shot phase + targeted exception)
- Cross-link to CONFIGURATION.md
Verification:
- 29/29 pass on config-schema-docs-parity + docs-update + new feature
test (parity-check passes, so the config-schema entry I added in the
feature commit is now matched by a docs row)
- 6808/6808 full suite pass
- lint-no-source-grep clean
Doc style follows the same pattern used by the existing model_profile,
model_overrides, and model_profile_overrides sections — example-led,
table-backed, cross-referenced. Each doc surfaces the feature at the
right depth (reference / settings table / task guide).
* fix(#3023): mirror phase-type tier in resolveReasoningEffortInternal (CR Major)
CodeRabbit caught a real Codex correctness bug + 3 minor docs/test issues:
1. **Major (outside-diff)** — resolveReasoningEffortInternal in core.cjs
derived its tier exclusively from the profile table, ignoring the
models.<phase_type> override added in #3023. Failure mode on Codex:
Config: model_profile=balanced, models.execution=opus, agent=gsd-executor
resolveModelInternal: tier=opus → gpt-5.4
resolveReasoningEffortInternal: tier=sonnet → reasoning_effort=medium
↑
WRONG — should be xhigh
(opus tier on Codex)
The runtime received a mismatched (model, effort) pair. Mirrored the
phase-type lookup from resolveModelInternal so both functions derive
from the same tier source. 'inherit' phase-type returns null effort
(no runtime entry maps to 'inherit'; let runtime decide).
2. Minor — .changeset/per-phase-type-models.md `pr: TBD` → `pr: 3030`.
3. Minor (outside-diff) — model-profiles.md "Resolution Logic" section
omitted the new phase-type tier. Updated the 4-step block to a 5-step
block including `models[phase_type]` between override and profile,
plus a paragraph noting that `model` and `reasoning_effort` derive
from the same tier source.
4. Nitpick — added 2 typo-safety tests:
- models.research = "haiku3" (typo) → falls through to profile
- models.research = "openai/gpt-5" (full ID) → falls through to profile
Plus 5 new reasoning_effort tests covering the Major fix:
- exported correctly
- phase-type override flips both model AND effort to same tier
- inherit phase-type returns null effort
- per-agent override still bypasses phase-type for effort
- claude runtime ignores models.* (no effort propagation)
Verification:
- 24/24 pass on regression test (15 original + 2 typo-safety + 5 effort + 2 outside-diff related)
- 6815/6815 full suite (7 net new from 6808)
- lint-no-source-grep clean
The reasoning_effort tests are written semantically (phase-type override
must produce the SAME effort as a profile-only opus config) rather than
hard-coding tier-specific effort strings, so changes to the runtime tier
map don't break them.
* fix(#3023): phase-type override beats profile=inherit (CR Major round 2)
CodeRabbit caught another precedence inversion: when
{ model_profile: 'inherit', models: { execution: 'opus' } }
both resolvers short-circuited on `profile === 'inherit'` BEFORE the
phase-type override could be honored. Result: model returned 'inherit'
and reasoning_effort returned null — both contradicting the documented
precedence where models[phase_type] wins over model_profile.
Fix in resolveModelInternal:
- Compute tier from phase-type FIRST. If phase-type is a valid alias,
it wins. Otherwise, fall back to profile-derived tier OR 'inherit'
(when profile === 'inherit').
- Gate the runtime-resolution branch on `tier !== 'inherit'` (was
`profile !== 'inherit'`) so phase-type=opus can flip runtime mapping
on even when profile=inherit.
- Gate the inherit-return on `tier === 'inherit'` (was
`profile === 'inherit'`).
Fix in resolveReasoningEffortInternal:
- Remove the `if (profile === 'inherit') return null;` early-return.
- Compute tier from phase-type first, fall back to profile. If
phase-type is explicitly 'inherit' OR the resolved tier is 'inherit',
return null (no runtime entry maps to inherit).
Tests added (5 new):
- model: phase-type wins over profile=inherit (with explicit opus, with
haiku for one phase + planner-without-slot still inheriting)
- model: profile=inherit + no models block → all agents inherit (no
regression on existing inherit semantics)
- model: profile=inherit + models block but agent has no slot → that
agent inherits, agents with slots get phase-type tier
- effort: phase-type opus + profile=inherit → produces opus-tier
effort, NOT null (the original bug)
Verification:
- 27/27 pass on regression test (24 prior + 3 model + 1 effort)
- 6820/6820 full suite (5 net new)
- lint-no-source-grep clean
The effort test reads the expected value by running a profile-only opus
config and comparing — semantic check, not hard-coded effort string. So
runtime tier map changes don't break the test.
* fix(#3019): query --help reaches handler instead of short-circuiting to top-level usage
The query argv parser in sdk/src/cli.ts harvested -h/--help as a global
flag and main() short-circuited dispatch when args.help was true. Net
effect: every `gsd-sdk query <anything> --help` printed top-level USAGE
instead of contextual subcommand help. There was no path for users to
discover what arguments a query subcommand accepts — they had to trigger
"required" errors by trial and error.
Two-layer fix:
1. sdk/src/cli.ts (parseCliArgsQueryPermissive)
- Push -h / --help onto queryArgv instead of consuming them silently,
so the registered handler / gsd-tools.cjs fallback gets to interpret
the flag and render contextual help.
- Only honor the global help flag when there is NO real subcommand to
dispatch to (i.e. queryArgv contains only help flags). Preserves
`gsd-sdk query --help` → top-level USAGE while letting
`gsd-sdk query phase add --help` reach the handler.
2. get-shit-done/bin/gsd-tools.cjs
- Render top-level usage on --help / -h / -? / --usage instead of
erroring with "Unknown flag". The discovery hint in the usage text
points users at the working method (run without args → error names
required arguments) and references #3019 for tracking subcommand-
level help printers.
- --version remains rejected (no discovery use-case).
#1818 anti-hallucination invariant preserved: the destructive command
NEVER executes when --help is present. The new shape returns success:true
+ usage on stdout instead of the old success:false + error on stderr —
both satisfy "destructive command did not run", and the new shape also
restores discoverability.
Tests:
- sdk/src/cli.test.ts: 4 new vitest cases covering #3019 — query argv
parser keeps --help with subcommand, parses -h short flag, preserves
bare `query --help` top-level behavior, preserves --help position when
intermixed with other query flags.
- tests/bug-3019-help-passthrough.test.cjs: 5 node:test cases on the
fallback — bare gsd-tools (no args) errors with usage; --help renders
usage on stdout exit 0; -h same; subcommand --help renders usage; usage
hint mentions discovery method (without prose substring matching —
parses into typed sections).
- tests/bug-1818-unknown-flags.test.cjs: rewritten to assert the new
invariant ("destructive command did not run" + "usage was rendered")
instead of the old shape ("--help is rejected with non-zero exit").
Each destructive test seeds a sentinel artifact (phase dir, slug
output) and asserts it survives.
Verification:
- 47/47 vitest pass on sdk/src/cli.test.ts
- 5/5 pass on tests/bug-3019-help-passthrough.test.cjs
- 8/8 pass on tests/bug-1818-unknown-flags.test.cjs (rewritten)
- 6763/6763 pass on full node:test suite
- lint-no-source-grep clean (0 violations)
Closes#3019
* fix(#3019): SDK fallback forwards plain-text help, broader usage list (CR)
CodeRabbit on PR #3026 (4 findings — 1 Major outside-diff, 2 inline,
1 nitpick):
1. **Major outside-diff** — sdk/src/cli.ts:442-454. The fallback path
that delegates to gsd-tools.cjs called parseCliQueryJsonOutput
(JSON.parse) on stdout. Now that gsd-tools renders plain-text usage
on --help, JSON.parse threw "Unexpected token 'U'". Wrapped the
parse in try/catch — on parse failure, forward the plain stdout
verbatim so subcommand help reaches the user. Regression test:
tests/bug-3019-help-passthrough.test.cjs spawns the built SDK and
asserts `gsd-sdk query phase --help` exits 0, stdout contains the
gsd-tools usage, and stderr does NOT contain a JSON-parse error.
2. .changeset/help-passthrough.md:3 — `pr: TBD` → `pr: 3026`.
3. gsd-tools.cjs:346 (TOP_LEVEL_USAGE):
- Removed self-referencing `#3019` link (immediately stale after
this PR merges).
- Expanded Commands list from 17 → all 47 dispatcher cases:
agent-skills, audit-open, audit-uat, check-commit, commit, …
phase, phases, roadmap, milestone, validate, progress, intel,
graphify, learnings, etc. — the bulk of the surface that was
previously unreachable via --help discovery.
4. Nitpick: `isUsageOutput` was duplicated in bug-1818 and
bug-3019-help-passthrough tests. Moved to tests/helpers.cjs with
structural-comment, removed both duplicates.
Verification: 47/47 vitest pass, 14/14 regression tests pass,
6764/6764 full suite, lint clean.
* test(#3019): use t.skip() instead of bare return when SDK not built (CR)
CodeRabbit follow-up on PR #3026:
The integration test guarded against missing sdk/dist/cli.js with a
bare `return;` — node:test counts that as a passing test (0 assertions
exercised, 0 failures). On a CI checkout that hasn't run the SDK build,
the #3026 regression test silently green-lit and no signal ever
surfaced that the integration check was skipped.
Switched to `t.skip(...)` via the test context parameter so the
omission shows up in the test report. The unit-level fix
(sdk/src/cli.ts) is still covered by vitest, so the skip only affects
the end-to-end spawn-built-SDK check.
Verification: 6/6 pass when SDK is built; 5 pass + 1 skip when not.
* fix(#3010): post-install message and docs use /gsd-update --reapply
PR #2824 consolidated 86 skills into ~58, removing the standalone
/gsd-reapply-patches command and folding it into a flag on /gsd-update
(/gsd-update --reapply). The 1.39.1 hotfix (#2954) updated help.md
but missed three other surfaces that still recommended the dead form:
1. bin/install.js reportLocalPatches() — runtime emitter shown after
every install with backed-up patches. All branches updated:
- claude/opencode/kilo/copilot: /gsd-update --reapply
- gemini: /gsd:update --reapply
- codex: $gsd-update --reapply
- cursor: gsd-update --reapply (mention the skill name)
2. get-shit-done/workflows/update.md — Step 4 prose and the
check_local_patches block both referenced /gsd-reapply-patches.
Replaced with /gsd-update --reapply (with backticks around the
command per CR feedback for copy/paste UX).
3. Localized docs (en/ja-JP/ko-KR/zh-CN) — 14 files across
ARCHITECTURE.md / COMMANDS.md / FEATURES.md / INVENTORY.md /
USER-GUIDE.md / manual-update.md still listed the removed command.
Tests:
- bug-3010-reapply-patches-references.test.cjs (4 tests): scans
bin/install.js's reportLocalPatches body, every workflow file, and
every doc (excluding CHANGELOG history and help.md's deprecation
notice) for the removed command form, and verifies each runtime
branch emits the consolidated form via captured console output.
- tests/copilot-install.test.cjs:1081-1115 — stale assertions that
hard-coded the removed string updated to assert /gsd-update --reapply.
Verification: 115/115 pass across both files.
Co-authored-by: Patrick Clery <patrick@patrickclery.com>
Closes#3010
* test(#3010): broaden dead-command scan + tighten runtime exact-match
CodeRabbit follow-up findings on #3012:
1. Workflow + docs scans only matched "/gsd-reapply-patches", missing
the gemini ("/gsd:reapply-patches") and codex ("$gsd-reapply-patches")
spellings. A regression that re-introduced either form in localized
docs would have passed silently. Extracted a DEAD_COMMAND_PATTERNS
array + findDeadCommands() helper used by both scans, so all three
removed forms are checked uniformly. Match output also reports which
spellings hit, for faster diagnosis.
2. reportLocalPatches runtime test asserted output.includes('update --reapply'),
which is too loose — a malformed prefix like '/gsd:update --reapply' on
the claude branch would have passed. Replaced with an exact
{runtime → expected token} map covering all 7 branches:
claude/opencode/kilo/copilot → /gsd-update --reapply
gemini → /gsd:update --reapply
codex → $gsd-update --reapply
cursor → gsd-update --reapply
Negative assertion also runs DEAD_COMMAND_PATTERNS against output for
every runtime, so dead forms can't slip in regardless of branch.
Verification: 4/4 pass on bug-3010-reapply-patches-references.test.cjs.
* test(#3010): add prefix-absence guard for cursor runtime (CR follow-up)
CodeRabbit (Minor): the cursor expected token "gsd-update --reapply" is
a substring of every prefixed form ("/gsd-update --reapply" for claude/
opencode/kilo/copilot, "\$gsd-update --reapply" for codex). The positive
output.includes(expectedToken) check therefore can't distinguish correct
cursor output from a regression where the installer emits a prefixed
form for cursor — both pass the substring check.
Add an explicit prefix-absence assertion for cursor that fails if any
of /, \$, or : appears immediately before "gsd-update --reapply" in
output. The gemini form ("/gsd:update --reapply") doesn't share the
substring (gsd:update vs gsd-update) so it's already caught by the
positive includes failing on cursor's expected bare token.
Verification: 4/4 pass.
---------
Co-authored-by: Patrick Clery <patrick@patrickclery.com>
* test(#2974): migrate 8 test files to typed-IR assertions
Replaces raw stdout/stderr substring matching with structured-field
assertions per CONTRIBUTING.md "Prohibited: Raw Text Matching on Test
Outputs". Adds shared infrastructure for typed error emission so this
pattern is the easy path going forward.
Shared infrastructure:
- core.cjs: ERROR_REASON frozen enum + setJsonErrorMode/getJsonErrorMode
- gsd-tools.cjs: --json-errors CLI flag, parsed before subcommand dispatch
- config.cjs: typed reasons at all 7 error sites
- graphify.cjs: GRAPHIFY_REASON enum + reason/timeout_ms in execGraphify result
- bin/install.js: pure buildSdkFailFastReport() IR builder + renderer
- hooks/gsd-session-state.sh, gsd-phase-boundary.sh: emit Claude Code
hookSpecificOutput JSON envelope with typed state_present/config_mode/
planning_modified/file_path fields (no-op when hooks.community is off)
Test migrations (all pass, 171 tests across the 8 files):
- bug-2649-sdk-fail-fast: assert on ir.reason / ir.context / ir.fix_command
- bug-2687-config-read-warning-parity: assert.equal stderr === ''
- bug-2796-arg-parsing-regression: assert on result.json.updated/.phase
- bug-2838-summary-rescue: parse rescue footer, assert mtime invariant
- bug-2943-config-get-context-window: parse JSON, assert ERROR_REASON.CONFIG_KEY_NOT_FOUND
- graphify: assert reason === GRAPHIFY_REASON.ENOENT/TIMEOUT
- hooks-opt-in: parse hookSpecificOutput, assert typed fields
- security-scan: reclassified as source-text-is-the-product (scan label
output and CI workflow YAML ARE the deployed contract)
Verification: lint-no-source-grep clean (0 violations), full suite
6741/6741 pass.
Closes#2974
* test(#2974): address CR feedback — typed code field, robust idempotency
Two CodeRabbit findings on #3016 addressed:
1. tests/hooks-opt-in.test.cjs:355 (Minor, inline) —
parsed.reason.includes('Conventional Commits') was still substring
matching after the typed-IR migration. Fixed at the source: the
gsd-validate-commit hook now emits a typed `code` field
('CONVENTIONAL_COMMITS_VIOLATION', 'COMMIT_SUBJECT_TOO_LONG')
alongside the human-readable `reason`. Test asserts strictEqual
on the code; the prose copy is no longer part of the test contract.
2. tests/bug-2838-summary-rescue-gitignored-planning.test.cjs:224-250
(Outside-diff) — mtimeMs alone can stay unchanged on coarse-grained
filesystems (HFS+, FAT) when two rewrites land within the same
timestamp tick, falsely passing the idempotency assertion.
Replaced with a full snapshot (mtimeMs, ctimeMs, size, ino, sha256
of contents) compared via assert.deepStrictEqual — the hash
catches any rewrite the timestamp would miss.
Verification: 30/30 pass on the two affected files; lint-no-source-grep
clean (0 violations across 368 test files).
* fix(#2973): /gsd-profile-user writes dev-preferences.md to skills/ not legacy commands/gsd/
v1.39.0's install summary claimed the legacy ~/.claude/commands/gsd/
directory had been removed in favor of skills-only architecture, but
the cmdGenerateDevPreferences writer at profile-output.cjs:781 still
defaulted to the legacy path. Every /gsd-profile-user --refresh
deterministically re-created the legacy directory.
Missed in PR #1540's migration because dev-preferences is a
runtime-generated user artifact, not a GSD-shipped command file.
Fix:
- Writer default: ~/.claude/skills/gsd-dev-preferences/SKILL.md
- profile-user.md Display message + artifact list reference new path
- New migrateLegacyDevPreferencesToSkill(targetDir, saved) installer
helper. Called at all 5 skills-aware install branches. Copies
preserved legacy dev-preferences.md into skills/gsd-dev-preferences/
SKILL.md, but ONLY if no SKILL.md already exists -- never clobbers
user-customized skill content.
Tests: bug-2973-profile-user-skills-path.test.cjs runs the writer in
a subprocess (core.cjs:output uses fs.writeSync(1, ...) which bypasses
in-process stubbing), asserts the writer's command_path field is the
skills location, the file is on disk at that path, the legacy path is
NOT created. Tests for migration helper assert it writes when no skill
exists and skips when one does.
Closes#2973
* chore(#2973): add changeset fragment for PR #3003
* fix(#2973): rephrase comment to avoid cline-install leaked-path lint
The new comment at line 780 of profile-output.cjs literally contained
the string '~/.claude/commands/gsd/' which the cline-install
leaked-path regression test (tests/cline-install.test.cjs:175)
correctly flagged.
Cline transforms .claude/skills/ -> .cline/skills/ in installed .cjs
files but does not transform .claude/commands/. The new comment talks
about the legacy 'commands/gsd' subdirectory without the ~/.claude/
prefix, so the lint passes. The path semantics are unchanged -- the
runtime construction at line 787 still uses path.join(os.homedir(),
'.claude', 'skills', ...) which the lint regex does not match.
* test(#2973): add timeout to spawnSync to prevent CI hangs (CR feedback)
CodeRabbit on PR #3003: without a timeout, a regression that hangs the
writer or dispatcher would block CI indefinitely. Added a 30s timeout
(generous for what should complete in <1s) and an explicit signal
assertion so a timeout trip surfaces as a clear test failure with
context rather than a hung worker.
* test(#2973): add allow-test-rule annotation for legitimate product-text parsing
The new var-binding lint from #2982/#2985 caught readFileSync(...).match()
and readFileSync(...).includes() calls in this test. Both are legitimate
structural assertions against the product workflow markdown, not source-grep:
- match() extracts the path from a structured Display: "..." line and
asserts on the typed path value (same pattern as bug-2470's installer
scanForLeakedPaths regex test).
- includes() asserts the absence of a legacy path literal.
profile-user.md IS the shipped workflow artifact, and its Display: line
IS what the user sees. Per the existing test-rigor convention, this is
the source-text-is-the-product justification category.
Annotated with allow-test-rule citing that category.
* chore(#3003): drop direct CHANGELOG.md edit; release entry now lives in .changeset/
The changeset-fragment workflow (#2975) renders fragments into
CHANGELOG.md at release time. Direct edits to [Unreleased] on
each PR caused merge conflicts on every concurrent PR. This commit
restores CHANGELOG.md to match origin/main; the release entry for
this fix is preserved in the .changeset/*.md fragment(s) on this
branch, which the release workflow consolidates.
* fix(#2973): preserve user-owned gsd-dev-preferences skill across wipe (CR)
CodeRabbit on PR #3003 caught a real bug: copyCommandsAsClaudeSkills()
wipes ALL gsd-* skill directories at the top of every install, then
reinstalls from the package source. Since gsd-dev-preferences is
user-generated (written by /gsd-profile-user --refresh) and NOT
shipped by the npm package, the wipe deletes the user's customized
SKILL.md with nothing to restore from.
Fix: USER_OWNED_SKILLS allow-list in copyCommandsAsClaudeSkills.
Snapshot files under skills/gsd-dev-preferences/ before the wipe,
restore after. Same preserve/restore pattern as PR #1924.
Tests: bug-2973 grows by 2 cases:
- user-customized SKILL.md survives the wipe
- non-user-owned gsd-* skills are still wiped (preservation is opt-in)
* fix(#2994): move verify-reapply-patches.cjs to get-shit-done/bin/ so installer ships it
scripts/verify-reapply-patches.cjs (added in #2972 to close the
verified-yes-without-checking gap from #2969) shipped in the npm tarball
but never reached user installs: bin/install.js copies get-shit-done/
recursively but does not copy the top-level scripts/ directory.
Effect: every fresh install hit `Cannot find module …/scripts/verify-reapply-patches.cjs`
on Step 5 of /gsd-reapply-patches. The whole point of moving
verification out of LLM-driven prose into a deterministic script is
undone if the script does not resolve at runtime.
Fix: move the script to get-shit-done/bin/verify-reapply-patches.cjs
(same pattern as gsd-tools.cjs and other runtime bin scripts that the
installer ships) and update reapply-patches.md Step 5 to invoke
${GSD_HOME}/get-shit-done/bin/verify-reapply-patches.cjs.
Tests:
- bug-2969 SCRIPT path updated to the new location
- New bug-2994-verify-reapply-patches-installed-path.test.cjs parses
reapply-patches.md into structured invocation records and asserts
every node ${GSD_HOME}/... reference lives under get-shit-done/
(the installed tree). Catches future regressions where someone moves
a runtime-needed script back to scripts/.
Closes#2994
* chore(#2994): add changeset fragment for PR #3000
* chore(#2994): add changeset fragment for PR #3000
* docs(#2994): update verifier-script-location comment to reflect new path (CR)
CodeRabbit on PR #3000: the parenthetical at line 278 still said the
script ships under scripts/, but this PR moved it to get-shit-done/bin/.
Updated the prose to reference the new location and the installer
target path.
* chore(#3000): drop direct CHANGELOG.md edit; release entry now lives in .changeset/
The changeset-fragment workflow (#2975) renders fragments into
CHANGELOG.md at release time. Direct edits to [Unreleased] on
each PR caused merge conflicts on every concurrent PR. This commit
restores CHANGELOG.md to match origin/main; the release entry for
this fix is preserved in the .changeset/*.md fragment(s) on this
branch, which the release workflow consolidates.
