Commit Graph

104 Commits

Author SHA1 Message Date
Tom Boucher
0f98952a3d refactor(sdk): extract GSDTools transport seam + policy (#3058)
* refactor(sdk): extract gsdtools transport seam with per-command policy

* fix(sdk): address CodeRabbit transport policy and timeout findings

* fix(sdk): harden raw transport formatting and raw-path coverage
2026-05-03 08:20:05 -04:00
Tom Boucher
e1d661ece0 feat(#3024): dynamic routing with failure-tier escalation (#3031)
* 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.
2026-05-02 14:26:35 -04:00
Tom Boucher
d812c66020 feat(#3023): per-phase-type model map in .planning/config.json (#3030)
* 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.
2026-05-02 13:19:15 -04:00
Tom Boucher
e3b64b39f8 fix(#3019): query --help reaches handler instead of short-circuiting (#3026)
* 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.
2026-05-02 11:45:33 -04:00
Tom Boucher
ffeeb92c14 fix(#2997): mask SECRET_CONFIG_KEYS in SDK config-set/get and init responses (#2999)
* fix(#2997): mask SECRET_CONFIG_KEYS in SDK config-set/get and init responses

The CJS→TS port at sdk/src/query/config-mutation.ts:240,243 and
config-query.ts:122,128,132 dropped the masking layer that secrets.cjs
spec defines for brave_search/firecrawl/exa_search. Result: the SDK
echoed plaintext API keys into machine-readable JSON output (stdout,
transcripts, CI logs).

Adjacent leak in init.ts:673-675 / init.cjs:728-730: the init bundle
passed config.brave_search through raw, leaking the API key whenever
the user had stored one.

Fix:
- New sdk/src/query/secrets.ts ports SECRET_CONFIG_KEYS, isSecretKey,
  maskSecret, maskIfSecret. Exact CJS parity (verified by 17 tests
  in secrets.test.ts that import secrets.cjs and compare).
- config-set masks value + previousValue in response; on-disk plaintext
  intact (key stays usable).
- config-get masks read response. --default flows through unmasked
  (user's own input, not stored secret).
- init.ts/init.cjs mask string values only; booleans (availability
  flags) pass through unchanged so the typed contract is preserved.

Tests: 17 in secrets.test.ts (including CJS parity), 5 in
config-mutation.test.ts (#2997 block — covers on-disk-preserved,
previousValue masking, short-value, unset, non-secret pass-through),
4 in config-query.test.ts.

Closes #2997

* chore(#2997): add changeset fragment for PR #2999

* chore(#2997): add changeset fragment for PR #2999

* chore(#2999): 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.
2026-05-02 00:17:45 -04:00
javeroff
9d0d085a17 fix(query/agent-skills): emit raw <agent_skills> block instead of JSON-wrapped string (#2917)
* fix(query/agent-skills): emit raw <agent_skills> block instead of JSON-wrapped string

The CLI dispatcher (`cli.ts`) JSON-stringifies all query handler results via
`console.log(JSON.stringify(result.data, null, 2))`.  For the `agent-skills`
handler this produced a JSON-quoted string literal — e.g.
`"<agent_skills>\n…</agent_skills>"` — which workflows embedded verbatim via
`$(gsd-sdk query agent-skills gsd-planner)`, breaking all `<agent_skills>`
injection into spawned subagent prompts.

Fix: add an optional `format: 'json' | 'text'` field to `QueryResult`.  When a
handler returns `format: 'text'` and `--pick` is not active, the CLI writes the
string directly via `process.stdout.write` instead of JSON-stringifying it.
`agentSkills` sets `format: 'text'` for non-empty blocks.

Regression guard: two new CLI integration tests in `skills.test.ts` spawn the
CLI as a child process and assert that (a) a mapped agent type receives the raw
XML block on stdout and (b) an unmapped agent type produces the existing JSON
empty-string output.

Fixes #2914.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs(changelog): add #2917 entry under Unreleased Fixed

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-01 12:21:06 -04:00
Tom Boucher
8de8acee46 fix(workflows): assert HEAD on per-agent branch before worktree commits (#2924) (#2941)
* fix(workflows): assert HEAD on per-agent branch before worktree commits

Worktree-mode setup could leave HEAD attached to a protected branch (master),
causing agent commits to land there. The previous response was a destructive
self-recovery via 'git update-ref refs/heads/master <sha>', which silently
rewinds the protected branch and destroys concurrent commits in multi-active
scenarios (parallel agents, user committing while agent runs).

- Reorder <worktree_branch_check> in execute-phase.md and quick.md to assert
  HEAD via 'git symbolic-ref' BEFORE any 'git reset --hard'. HALT with a
  blocker if HEAD is on main/master/develop/trunk/release/* or detached.
- Add a per-commit HEAD assertion (step 0) to gsd-executor.md
  <task_commit_protocol>; HEAD attachment can drift after 'git checkout <sha>'.
- Forbid 'git update-ref refs/heads/<protected>' in
  <destructive_git_prohibition>; surface the blocker rather than self-heal.
- Remove '--no-verify' as the worktree-mode default in execute-phase.md,
  execute-plan.md, quick.md, and references/git-integration.md. Hooks now
  run on every executor commit; opt out only via workflow.worktree_skip_hooks.
- Add regression test that parses the worktree_branch_check blocks structurally
  and asserts the symbolic-ref check precedes the reset --hard, no workflow
  performs update-ref on a protected ref, and --no-verify is no longer the
  default in any parallel-execution prompt.

* fix(#2924): address CodeRabbit review findings on worktree HEAD PR

- Add positive worktree-agent-* allow-list to <task_commit_protocol> step 0
  in gsd-executor.md and to <worktree_branch_check> in execute-phase.md and
  quick.md. The deny-list (main|master|develop|trunk|release/*) silently
  allowed feature/* and other arbitrary branches outside the agent namespace.
- Register workflow.worktree_skip_hooks in both config schemas
  (sdk/src/query/config-schema.ts and get-shit-done/bin/lib/config-schema.cjs)
  and document it in docs/CONFIGURATION.md so config-set accepts it.
- Fix stash lifecycle in execute-phase.md post-wave hook validation: stash
  under a named ref and pop after the hook run; warn on pop failure.
- Pre-dispatch PLAN.md commit in quick.md: gate on git diff --cached --quiet
  for idempotency and exit 1 with a clear error on commit failure (both the
  --no-verify and the normal branches) — no more swallowing real errors.
- Test fixes (tests/bug-2924-worktree-head-attachment.test.cjs):
  - Parse the protected-branch alternation structurally and require
    main, master, develop, trunk, release/.* (release/* was previously
    skipped by the \\b...\\b regex).
  - Use fs.readdirSync(dir, { recursive: true }) so workflows in nested
    subdirectories are also asserted against the update-ref ban.
  - Add allow-list assertions for execute-phase.md, quick.md, and
    gsd-executor.md to lock in the new positive namespace check.

* test(#2924): assert sub-section end marker exists before slicing

* test(#2924): use section boundary instead of fixed window for parallel-agents slice
2026-05-01 09:23:02 -04:00
Tom Boucher
006cdafe8f ci(drift): enforce alias freshness checks in CI and contributor flow (#2910)
Merging alias-drift guardrails and local hook hardening.
2026-04-30 14:19:46 -04:00
Tom Boucher
8051bc4fd8 test(golden): expand phases/validate/roadmap parity matrix (#2909)
Merging parity-matrix expansion after stack foundation.
2026-04-30 14:10:28 -04:00
Tom Boucher
444db1714b refactor(query): manifest-backed routing seam + family adapters (#2908)
Merging validated command-seam foundation.
2026-04-30 14:04:50 -04:00
TÂCHES
8cbdbdd2de feat(sdk): add durable planning runtime (#2898) 2026-04-30 09:03:06 -06:00
Tom Boucher
5fdc950eb7 feat(#2792): namespace meta-skills + keyword-tag descriptions + context utilization guard (#2825)
* feat(#2792): namespace meta-skills retargeted at the post-#2790 surface

This branch is now based on #2790's HEAD (the consolidation PR) instead
of main, and every routing table targets the consolidated surface so a
user routed by a namespace meta-skill never lands at a deleted /
folded sub-skill.

Cross-PR inconsistencies the original PR #2825 carried (vs #2790):

  - ns-ideate routed to gsd-note / gsd-add-todo / gsd-add-backlog /
    gsd-plant-seed → all folded into gsd-capture by #2790. Now routes
    to gsd-capture (the parent picks the mode from the user's intent).
  - ns-context routed to gsd-scan and gsd-intel → folded into
    gsd-map-codebase --fast / --query by #2790. Now routes to those
    flag forms.
  - ns-manage routed all workspace intent to gsd-list-workspaces (a
    list-only entry) → CR also flagged the over-narrow target. #2790
    folds into gsd-workspace; routing now points there.
  - ns-workflow routed to gsd-research-phase → deleted outright by
    #2790. Removed.
  - ns-project routed to gsd-plan-milestone-gaps → deleted outright by
    #2790. Removed.
  - None of the namespaces previously surfaced #2790's new consolidated
    skills (gsd-capture, gsd-phase, gsd-config, gsd-workspace,
    gsd-progress). All five are now reachable through the routers.
  - extract_learnings → extract-learnings (canonicalized by #2858).

Defect fixes within the namespace skills:

  - Hyphen-form `name:` (gsd-workflow, …) per the canonical naming
    contract — the colon-form addressed CR's drift complaint.
  - `Skill` added to allowed-tools on every router. The body instructs
    "Invoke the matched skill directly using the Skill tool" — without
    Skill in the permission list the meta-skill cannot route at all.

New regression guard in tests/enh-2792-namespace-skills.test.cjs: every
gsd-* token in any namespace router's table column resolves to a
surviving commands/gsd/*.md file (or to a known consolidated parent for
flag-form targets like gsd-map-codebase --fast). This single test would
have caught every dead-end route the original PR shipped with.

Skill-count cap in tests/enh-2790-skill-consolidation.test.cjs now
filters out ns-*.md from its <= 63 cap. Namespace routers are
descriptor-only entries, not part of the consolidation surface that cap
is policing — they have their own contract in
tests/enh-2792-namespace-skills.test.cjs.

INVENTORY.md gains a "Namespace Meta-Skills" section with the 6 router
rows; INVENTORY-MANIFEST.json gains 6 entries; the headline count moves
59 → 65 to match.

Out of scope for this rebase: the gsd-health --context flag (PR #2825
advertised the contract but didn't implement it). That's a separate
feature concern and is left untouched here.

5908/5908 on `npm test`.

* feat(#2792): implement gsd-health --context utilization guard

The original PR #2825 advertised a `--context` flag on gsd-health with a
60%/70% utilization threshold table but never implemented the workflow
logic — CR caught it as a contract leak, the rebase deferred it. This
commit closes the gap with TDD red/green/refactor.

Math layer (pure):
  - get-shit-done/bin/lib/context-utilization.cjs
    classifyContextUtilization(tokensUsed, contextWindow) →
      { percent, state }
    State boundaries use the exact ratio:
      < 60% healthy / 60–70% warning / ≥ 70% critical (fracture point)
    Display percent rounded for humans. Throws TypeError on non-integer
    or out-of-range inputs.
  - STATES = Object.freeze({ HEALTHY, WARNING, CRITICAL }) exported
    so callers reference the names by symbol, not by literal string.

SDK CLI integration:
  - get-shit-done/bin/gsd-tools.cjs
    `validate context --tokens-used N --context-window M [--json]`
    routes to the classifier, owns the recommendation copy (the
    classifier intentionally does not — keeps the renderer free to
    evolve without touching the math layer or its tests), and uses
    core.output's rawValue path for the sync-flush guarantee.
  - sdk/src/query/validate.ts + sdk/src/query/index.ts
    TypeScript validateContext handler registered at 'validate.context'
    and 'validate context'. Mirrors the CJS classifier inline (15 lines
    of arithmetic; not worth a shared cross-language module).

User-facing wiring:
  - commands/gsd/health.md frontmatter advertises --context, body
    documents the three-state threshold table.
  - get-shit-done/workflows/health.md adds a `context_check` step
    that's reached only when --context is set. Step calls
    `gsd-sdk query validate.context` with self-reported tokensUsed and
    contextWindow, prints the SDK output verbatim, and ends. Includes
    a TEXT_MODE plain-text fallback for non-Claude runtimes per #2012.

Tests:
  - tests/context-utilization.test.cjs (17 tests) — pure-function
    contract: state thresholds at every boundary, percent rounding,
    input validation, return-shape (no recommendation field — that's
    the renderer's job).
  - tests/validate-context.test.cjs (9 tests) — SDK CLI plumbing:
    arg parsing errors, JSON vs human rendering, recommendation copy
    pinned per state.
  - tests/enh-2792-namespace-skills.test.cjs (4 new tests) — markdown
    contract: --context advertised in argument-hint, threshold table
    in command body, context_check step exists in workflow, step
    invokes gsd-sdk query validate.context with both flags.

Inventory bookkeeping:
  - docs/INVENTORY.md "CLI Modules" 31 → 32; new row for
    context-utilization.cjs.
  - docs/INVENTORY-MANIFEST.json mirror.

5939/5939 on `npm test`.
2026-04-30 01:04:41 -04:00
Tom Boucher
f9ed47ac8b fix(#2832): gsd-sdk auto detects Codex runtime correctly (#2844)
* fix(#2832): gsd-sdk auto detects Codex runtime correctly

Two-part fix for #2832 (gsd-sdk auto silently routing non-Claude runtime
projects through the Claude Agent SDK):

1. Runtime gate at the `auto` entry point. New `runtime-gate.ts` exports
   `assertRuntimeSupportsAutoMode(config)` which throws an actionable error
   when `GSD_RUNTIME` / `config.runtime` resolves to a non-Claude runtime
   (codex, gemini, opencode, etc.). The autonomous orchestrator only knows
   how to drive `@anthropic-ai/claude-agent-sdk` today; failing fast with a
   clear pointer at the in-session slash commands beats the previous instant
   `[FAILED] $0.00 0.1s` flake. Wired into `cli.ts` before the GSD/InitRunner
   construction.

2. Runtime-aware `resolveModel()` in `session-runner.ts`. The profile -> id
   map (`balanced -> claude-sonnet-4-6`, etc.) was applied unconditionally,
   so even with `runtime: codex` and `resolve_model_ids: omit` the SDK
   forced a Claude id into `query()`. Now the profile map only fires when
   the runtime is Claude and the explicit `resolve_model_ids: "omit"` knob
   short-circuits to undefined, mirroring `query/config-query.ts`.

Tests (vitest, sdk/src):
- runtime-gate.test.ts (8 cases): claude / unset / unknown pass; codex,
  gemini, opencode throw; GSD_RUNTIME wins over config.runtime; error
  message references #2832 and the slash-command workaround.
- session-runner.test.ts (4 new cases under "resolveModel runtime
  awareness (#2832)"): codex runtime + balanced profile -> no model
  injected; resolve_model_ids: omit -> no model; claude runtime still
  resolves to claude-sonnet-4-6 (no regression); explicit options.model
  wins on any runtime.

* fix(#2832): address CR — env-precedence in resolveModel + accurate source attribution

Two CodeRabbit findings on PR #2844:

1. session-runner.ts:resolveModel() (Major) — read runtime via detectRuntime()
   so GSD_RUNTIME env precedence is honored. Without this, a Codex run with
   a Claude-shaped config still fell into the Claude-only profile-id branch.

2. runtime-gate.ts:assertRuntimeSupportsAutoMode() (Minor) — when GSD_RUNTIME
   holds an unsupported value, detectRuntime() falls through to config but
   the source label still reported the discarded env value. Fix: validate
   env against SUPPORTED_RUNTIMES before attributing the source.

Tests added for both: env-precedence in session-runner, source attribution
in runtime-gate. 17/17 pass.
2026-04-29 08:03:32 -04:00
Tom Boucher
9e58c45ea1 fix(#2791): GSD_WORKSTREAM env var respected by gsd-sdk query + gsd-tools bin alias (#2821)
* fix(#2791): GSD_WORKSTREAM env var respected by gsd-sdk query + gsd-tools bin alias

Two fixes for gsd-sdk binary issues:

**Issue 1 — Binary name collision:**
Both `get-shit-done-cc` and `@gsd-build/sdk` declare `bin: { "gsd-sdk": ... }`.
Added `"gsd-tools": "bin/gsd-sdk.js"` to `package.json` bin so users with the
collision can invoke `gsd-tools query <cmd>` as a conflict-free alternative.

**Issue 2 — Query registry not workstream-aware:**
`gsd-sdk query` commands ignored `GSD_WORKSTREAM` env var, always reading from
the root `.planning/` even when a workstream was active. `gsd-tools.cjs` reads
`GSD_WORKSTREAM` via `planningDir()`, so all ~35 `gsd-sdk query` call sites in
workflow files were broken in workstream-scoped projects.

Fix: added env var fallback in `sdk/src/cli.ts` — when `--ws` is not provided,
`GSD_WORKSTREAM` is used (with name validation; invalid values are silently
ignored, matching CJS behaviour).

Regression test: `tests/bug-2791-sdk-workstream-env.test.cjs`

Closes #2791

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#2791): address CodeRabbit — precedence test, invalid env fallback assertion, bash fence

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-28 20:23:32 -04:00
Tom Boucher
a4e15d5616 fix(#2788): audit-uat reads human_verification items from frontmatter (#2814)
* fix(#2788): audit-uat reads frontmatter human_verification array

parseVerificationItems only searched the body for a '## Human Verification'
section. gsd-verifier writes items to the frontmatter human_verification:
YAML array, so audit-uat returned total_items: 0 for all such files.

Two fixes:
1. Read frontmatter human_verification: array first (via extractFrontmatter);
   return those items if present (primary path for gsd-verifier output).
2. Relax the body-section heading regex to accept underscore separators and
   parenthetical suffixes (e.g. '## human_verification (action required)').

Regression test: tests/bug-2788-audit-uat-frontmatter.test.cjs

* fix(#2788): address CodeRabbit — trim whitespace entries, support hyphenated headings, bash fence
2026-04-28 20:22:59 -04:00
Tom Boucher
fa78692167 fix(#2796): roadmap-update-plan-progress accepts --phase flag form (#2815)
* fix(#2796): roadmap update-plan-progress accepts --phase flag form

roadmap-update-plan-progress used positional-only arg parsing: args[0].
When execute-phase.md:228 calls it with --phase <N>, args[0] was the
literal string "--phase", which findPhase received as the phase number.
findPhase returned found:false, causing updated:false with no write.
ROADMAP.md plan checkboxes silently never advanced.

Fix: check for --phase <value> first; fall back to the first non-flag
positional argument for backward-compatible direct calls.

Regression test: tests/bug-2796-arg-parsing-regression.test.cjs

* fix(#2796): address CodeRabbit — guard --phase against flag-like values, bash fence
2026-04-28 20:22:30 -04:00
Tom Boucher
b959b1844f fix(#2803): config-get supports --default <value> fallback for missing keys (#2817)
* fix(#2803): honor --default flag in SDK config-get handler

The gsd-sdk query config-get handler ignored the --default <value> flag.
Missing keys always threw 'Key not found' (exit 1), making 8 workflow
sites that rely on config-get --default fall through to error paths.

The CJS path (gsd-tools.cjs) honored --default since #1893; this ports
that behavior to the SDK configGet handler.

Regression test: tests/bug-2803-config-get-default-flag.test.cjs

* fix(#2803): address CodeRabbit — require --default value, keep missing config.json as error, bash fence
2026-04-28 20:21:48 -04:00
Tom Boucher
f983c95ffc Release/1.39.0-rc.4 (#2797)
* chore: bump version to 1.39.0 for release

* chore: bump to 1.39.0-rc.1

* chore: bump to 1.39.0-rc.2

* chore: bump to 1.39.0-rc.3

* chore: bump to 1.39.0-rc.4

* docs: add v1.39.0-rc.4 release notes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-27 22:32:14 -04:00
Tom Boucher
936cf26706 fix(#2769): tolerate Requirements header with colon inside bold delimiters (#2782)
* fix(#2769): tolerate Requirements header with colon inside bold delimiters

extractReqIds in sdk/src/query/init.ts and the legacy init.cjs port only
matched `**Requirements**:` (colon outside bold), so phases declared with
the equally-valid markdown form `**Requirements:**` (colon inside bold,
which is what the project's own templates emit) returned phase_req_ids:
null for both `init plan-phase` and `init execute-phase`.

The mirror-image bug in `phase complete`'s REQUIREMENTS.md traceability
sweep at get-shit-done/bin/lib/phase.cjs:871 only matched the inside-bold
form, silently skipping the REQ-ID checkbox flips for any roadmap that
used the outside-bold form. Both parsers now share the same canonical
regex that accepts all three rendered-identical variants:

  **Requirements:**     (colon inside bold)
  **Requirements**:     (colon outside bold)
  **Requirements** :    (space before outside colon)

Tests:
- tests/init.test.cjs — parameterized over the three header variants for
  both init plan-phase and init execute-phase (6 new behavioral cases).
- sdk/src/query/init.test.ts — describe.each over the same variants
  exercising initPlanPhase through the SDK.
- tests/bug-2769-requirements-header-variants.test.cjs — phase complete
  flips REQ-001 in REQUIREMENTS.md across all three header variants.

Closes #2769

* refactor(#2769): centralize REQUIREMENTS_HEADER_RE constant per CodeRabbit
2026-04-27 12:31:49 -04:00
Tom Boucher
b8bbc74192 fix(sdk): preserve nested keys from globalDefaults in configNewProject (#2745)
When building nested config sections (workflow, git, hooks, agent_skills,
features), the deep merge was missing globalDefaults for those sections,
causing user values from ~/.gsd/defaults.json to be silently dropped.

Added globalDefaults spread at the correct precedence level (hardcoded <
globalDefaults < userChoices) for all five nested keys, and added three
test cases verifying the merge works end-to-end via HOME env var override.

Closes #2673

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 13:33:26 -04:00
Tom Boucher
2b95ccbddd Merge pull request #2743 from gsd-build/fix/2714-workstream-config-inheritance
feat(config): workstream config.json inherits from root .planning/config.json
2026-04-26 11:49:30 -04:00
Tom Boucher
4a05283bc8 Merge pull request #2742 from gsd-build/fix/2731-workstream-query-handler-threadthrough
fix(query): thread workstream through all query handlers
2026-04-26 11:49:25 -04:00
Tom Boucher
41787e361f Merge pull request #2740 from gsd-build/fix/2641-details-summary-milestone-detection
fix(phase-lifecycle): skip <details>-wrapped sections in milestone detection
2026-04-26 11:49:14 -04:00
Tom Boucher
bbf33b608e Merge pull request #2738 from gsd-build/fix/2644-milestone-complete-version-arg
test(query): Vitest regression guard for milestone.complete version arg (fix #2644)
2026-04-26 11:49:03 -04:00
Tom Boucher
9e63d14709 Merge pull request #2737 from gsd-build/fix/2726-phase-add-bullet-bold-roadmap
fix(phase-lifecycle): detect phases in bullet/bold ROADMAP formats for phase.add
2026-04-26 11:48:58 -04:00
Tom Boucher
5b4a239ead Merge pull request #2736 from gsd-build/fix/2728-phase-complete-roadmap-corruption
fix(phase-lifecycle): prevent plan-line corruption when **Plans:** has no inline value
2026-04-26 11:48:52 -04:00
Tom Boucher
274fc524cd fix(tests): derive STATE.md phase number from phaseDir in setupProject fixture
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 11:46:57 -04:00
Tom Boucher
b7b6f89776 fix(phase-lifecycle): fast-path replaceInCurrentMilestone only when pattern matches after
The previous guard `if (after.trim().length > 0)` incorrectly triggered
when `after` contained only footer text (e.g. `---\n*Last updated*`).
In that case `after.replace(pattern, replacement)` is a no-op and the
function returned unchanged content instead of falling through to the
slow path that searches inside the last `<details>` block.

Fix: capture the replaced string first, then only take the fast path
when the replacement actually changed `after`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 11:46:40 -04:00
Tom Boucher
76f1d20d80 test(phase-lifecycle): add regression for active <details> with trailing footer
Adds a case where the active milestone is the last <details> block but
footer text (--- / *Last updated*) follows </details>, triggering the
fast-path to replace in the footer instead of inside the block.

Closes #2743
2026-04-26 11:36:46 -04:00
Tom Boucher
493e251bab refactor(tests): extract assertMilestoneSuccess helper in decomposed-handlers.test.ts
Centralizes repeated result.data cast + common success-field checks
per coderabbit review suggestion on PR #2738.
2026-04-26 11:24:31 -04:00
Tom Boucher
cfc79d211f fix(config-query): thread workstream through resolveModel handler
resolveModel ignored _workstream, unlike configGet/configPath which both
forward it to planningPaths/loadConfig. Different workstreams may have
different model_profile settings.

Addresses coderabbit finding on PR #2742.
2026-04-26 11:23:51 -04:00
Tom Boucher
8270f17773 fix(phase-lifecycle): handle project-code-prefixed dirs in phaseAdd fallback scan
Filesystem fallback regex /^(\d+)-/ missed directories like CK-45-foundation
when project_code is configured. Updated to /^(?:[A-Z][A-Z0-9]*-)?(\d+)-/i.

Addresses coderabbit finding on PR #2737.
2026-04-26 11:21:33 -04:00
Tom Boucher
c85e65ec03 fix(roadmap-update-plan-progress): propagate Plans** regex hardening from phase-lifecycle
Applies the same [ \t]* + section-boundary lookahead fix that was applied
to planCountPattern in phase-lifecycle.ts. roadmap.update-plan-progress
shared the same corruption vector via \s* crossing newlines.

Addresses coderabbit finding on PR #2736.
2026-04-26 11:19:34 -04:00
Tom Boucher
3a623b1117 fix(query): thread workstream through all query handlers
All 18+ query handlers accepted _workstream but never forwarded it to
planningPaths/loadConfig/getMilestoneInfo. Remove _ prefix and pass
workstream to all internal helper calls so --ws flag actually scopes
path resolution.

Affected handlers: initNewProject, initProgress, initManager, configGet,
configPath, configSet, configSetModelProfile, configNewProject,
configEnsureSection, validateHealth, commit, checkCommit, commitToSubrepo.

Also fixes validate.ts to use paths.* fields from planningPaths instead
of hardcoded join(projectDir, '.planning') paths.

Closes #2731
2026-04-26 10:53:17 -04:00
Tom Boucher
f6cddc5b2f test(query): add failing tests for workstream path threading in init-complex
Demonstrates that initProgress and initManager ignore the workstream
parameter, reading from root .planning/ instead of the workstream
subdirectory.

Closes #2731
2026-04-26 10:38:10 -04:00
Tom Boucher
0f17cfc71d fix(phase-lifecycle): skip <details>-wrapped sections in milestone detection
replaceInCurrentMilestone's lastIndexOf('</details>') heuristic fails
when the active milestone itself is wrapped in a <details> block — the
after-slice is empty so the replacement is silently dropped.

Fix detects this case (after.trim().length === 0) and falls back to
locating the last complete <details>…</details> span and applying the
replacement only inside it, leaving all earlier archived-milestone
blocks untouched.

Closes #2641
2026-04-26 10:28:48 -04:00
Tom Boucher
a7d3bb948b test(phase-lifecycle): add failing test for active milestone in <details> block
Closes #2641
2026-04-26 10:23:41 -04:00
Tom Boucher
f8a0e6f145 test(query): add Vitest regression tests for milestone.complete version arg (fix #2644)
milestoneComplete was imported in decomposed-handlers.test.ts but had zero
test coverage. The original defect (6f79b1d) called phasesArchive([], ...)
instead of forwarding the positional version arg; the wrapping try/catch
swallowed the GSDError into { completed: false, reason: String(err) },
masking a programming error as a legitimate negative answer.

Add five Vitest tests that lock in the correct contract:
- positional version arg is extracted from args[0] and echoed in response
- missing version throws GSDError (not masked as completed: false)
- --archive-phases flag is processed
- --name flag sets milestone name
- response shape has version/date/phases/milestones_updated fields

Closes #2644

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-26 10:18:08 -04:00
Tom Boucher
54cbc2ad96 fix(phase-lifecycle): detect phases in bullet/bold ROADMAP formats
phaseAdd's phase-number regex only matched heading format (## Phase N:),
missing bullet checklist (- [x] Phase N:) and bold (**Phase N:**) entries.
When zero regex matches, newPhaseId defaulted to 1.

Fix: broaden regex to match all three formats, and add filesystem fallback
scanning .planning/phases/ when ROADMAP scan finds nothing.

Closes #2726
2026-04-26 10:00:39 -04:00
Tom Boucher
a9f49f8f9d fix(phase-lifecycle): prevent plan-line overwrite when **Plans:** is bare
\s* after **Plans:** matches newlines, causing [^\n]+ to consume the first
plan checkbox when the **Plans:** field has no value on the same line.
Additionally, the lazy [\s\S]*? could cross section boundaries when the
current section had no **Plans:** value, corrupting a later section.

Fix 1: replace \s* with [ \t]* to restrict post-colon match to horizontal
whitespace only.
Fix 2: replace [\s\S]*? with (?:(?!\n#{2,4})[\s\S])*? to prevent the
pattern from crossing into a new section heading.

Closes #2728
2026-04-26 09:46:57 -04:00
Tom Boucher
394403ae06 test(phase-lifecycle): add failing regression test for #2728
When **Plans:** appears on its own line with no inline value, the
planCountPattern regex crosses the newline and destroys the first
plan checkbox line by replacing it with the literal "N/N plans complete"
string.

This test documents the expected correct behavior and will fail until
the planCountPattern regex is fixed.
2026-04-26 09:46:51 -04:00
Lex Christopherson
2fafbd2753 fix(sdk): pass phaseDir to executor prompt so SUMMARY.md lands in .planning/
The SDK's buildExecutorPrompt told executors to "Create a SUMMARY.md file"
with no directory path, causing them to write it in cwd (project root)
instead of .planning/phases/{phase}/. Thread phaseDir from PhaseRunner
through PromptFactory and into the completion instructions so the executor
gets an explicit path like `.planning/phases/01-auth/01-01-SUMMARY.md`.

Backward compatible — buildExecutorPrompt still accepts a plain string
(agentDef) for existing callers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-25 17:56:06 -06:00
Lex Christopherson
f0953dec0c fix(sdk): prevent interactive tool calls in headless self-discuss mode
The discuss step loaded the full interactive workflow prompt which
instructs the agent to use AskUserQuestion, Skill(), and area selection
UIs. In headless auto mode, the agent followed these instructions and
tried to interact with a non-existent user.

Fix: prepend a mandatory headless override BEFORE the workflow prompt
that explicitly forbids interactive tools and instructs the agent to
make all decisions autonomously. Prepending (not appending) ensures
the override takes priority over conflicting instructions later in
the prompt.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-25 14:08:05 -06:00
Lex Christopherson
25d9763878 fix(sdk): fix executor plan loading, plan ID derivation, and verification outcome parsing
Bug 1: phasePlanIndex derived empty planId for bare PLAN.md files.
Fixed to use 'PLAN' as the ID, with matching SUMMARY.md detection.

Bug 2: executeSinglePlan passed null to buildPrompt instead of the
actual parsed plan. The executor needs the plan content (tasks,
objectives) to know what to build. Now loads and parses the plan
file before building the prompt.

Bug 3: parseVerificationOutcome checked session exit code, not what
the verifier wrote. A session that runs without errors but writes
status: gaps_found to VERIFICATION.md was treated as 'passed'. Now
queries check.verification-status to read the actual VERIFICATION.md
frontmatter status field.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-25 14:08:05 -06:00
Lex Christopherson
377a6d2c6e fix(sdk): use installed agent/workflow prompts instead of stripped-down bundled copies
The SDK bundled its own agents and workflows at ~17% the size of the real
ones, missing critical instructions like file naming conventions, scope
reduction rules, discovery protocols, and TDD integration. This caused
the planner to create a single PLAN.md instead of properly named
per-plan files (01-01-PLAN.md, 01-02-PLAN.md), breaking wave-based
parallel execution.

- Invert load priority: installed GSD agents/workflows first, SDK
  bundled as last-resort fallback
- Replace @-reference stripping with resolution (read + inline content)
- Use full agent definitions instead of extracting only the <role> block
- Delete sdk/prompts/agents/ and sdk/prompts/workflows/ (13 files)
- Delete headless-prompts.test.ts (validated deleted files)
- Thread projectDir through sanitizePrompt for @-reference resolution

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-25 14:08:05 -06:00
Tom Boucher
b40110111d feat(#2306): plan-review-convergence v2 — CYCLE_SUMMARY contract, config gate, local model reviewers (#2718)
* feat(#2306): plan-review-convergence v2 — CYCLE_SUMMARY contract, config gate, local model reviewers

Fixes the false-stall detection bug in the plan→review→replan convergence
loop. REVIEWS.md accumulates history across cycles so raw grep inflated
HIGH counts; HIGH count now comes from a per-cycle CYCLE_SUMMARY contract
emitted in the review agent's return message.

Key changes:
- workflow.plan_review_convergence config gate (disabled by default, same
  pattern as workflow.code_review / workflow.nyquist_validation)
- Review agent prompt defines CYCLE_SUMMARY: current_high=<N> contract with
  PARTIALLY RESOLVED / FULLY RESOLVED counting rules
- Orchestrator aborts on absent/malformed CYCLE_SUMMARY (distinguishes both)
- Warns when HIGH_COUNT > 0 but ## Current HIGH Concerns section is missing
- Stall detection and --ws forwarding preserved and tested
- Local model reviewers: --ollama, --lm-studio, --llama-cpp flags added to
  convergence workflow and review workflow; all three use OpenAI-compatible
  /v1/chat/completions endpoint with jq --rawfile for safe JSON encoding
- review.ollama_host / review.lm_studio_host / review.llama_cpp_host config
  keys registered and documented (default to localhost:11434/1234/8080)
- review.models.ollama / .lm_studio / .llama_cpp model-name config support
- 58 tests (up from 29 in PR #2339), all passing

Closes #2306
Closes #2339
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(ci): sync sdk/src/query/config-schema.ts with CJS schema (#2306)

Add workflow.plan_review_convergence, review.ollama_host,
review.lm_studio_host, and review.llama_cpp_host to the SDK-side
TypeScript mirror — required by the CJS↔SDK parity test (#2653).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#2306): resolve CodeRabbit review findings

- Anchor HIGH_COUNT extraction with head -1 to prevent multi-match when
  agent return message contains multiple CYCLE_SUMMARY lines (e.g. quoted
  back from prompt context)
- Replace hardcoded reviewers list in REVIEWS.md frontmatter template with
  runtime-derived placeholder — the static list did not reflect which
  reviewers were actually invoked
- Broaden workflow.plan_review_convergence docs to include local reviewers
  (Ollama, LM Studio, llama.cpp) alongside cloud reviewers

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(ci): restore reviewers frontmatter list with runtime note

The cursor-reviewer.test.cjs (and equivalent per-reviewer tests) assert
that each supported reviewer appears on the reviewers: line — these are
wiring tests that catch when a new reviewer is added to invocation but
not to the REVIEWS.md template. Replacing the list with a placeholder
broke those tests.

Restore the full static list and add an inline comment clarifying that
the actual committed frontmatter should be filtered to only the reviewers
invoked that run — satisfying both the per-reviewer tests and the
CodeRabbit correctness note.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-25 14:18:34 -04:00
Tom Boucher
8e21c9b1b7 fix(#2684,#2676): milestone.complete arg forwarding + parallel milestone phase routing (#2708)
* test(#2692): add behavioral --wave N test, annotate source-text assertions

Adds two behavioral tests for wave filtering via phase-plan-index:
- Verifies plans with wave frontmatter are correctly grouped by wave number
- Verifies plans with no wave field default to wave 1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#2684,#2676): milestone.complete version validation + parallel milestone phase routing

#2684: Confirms milestone.complete correctly validates and uses its version
argument end-to-end. The inline archive path in milestoneComplete already
forwarded version correctly; regression tests lock in that contract.

#2676: phase.complete applied getMilestonePhaseFilter unconditionally, using
STATE.md's primary milestone to scope the candidate set. When the completed
phase belongs to a parallel (secondary) milestone, the filter excluded all
phases from that milestone, leaving an empty candidate set and incorrectly
returning is_last_phase: true / next_phase: null.

Fix: before applying the milestone filter in Step E, check whether the
completed phase itself appears in the filtered set. If not, skip the filter
for both the directory scan and the ROADMAP.md fallback so phases from the
secondary milestone remain visible for next-phase detection.

Closes #2684
Closes #2676

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-25 12:10:01 -04:00
Tom Boucher
b1278f6fc3 fix(#2674): align initProgress with initManager ROADMAP [x] precedence (#2681)
initProgress computed phase status purely from disk (PLAN/SUMMARY counts),
consulting the ROADMAP `- [x] Phase N` checkbox only for phases with no
directory. initManager, by contrast, applied an explicit override: a
ROADMAP `[x]` forces status to `complete` regardless of disk state.

Result: a phase with a stub directory (no SUMMARY.md) and a ticked
ROADMAP checkbox reported `complete` from /gsd-manager and `pending`
from /gsd-progress — same data, different answer.

Apply ROADMAP-[x]-wins as the unified policy inside initProgress, mirroring
initManager's override. A user who typed `- [x] Phase 3` has made an
explicit assertion; a leftover stub dir is the weaker signal.

Adds sdk/src/query/init-progress-precedence.test.ts covering six cases
(stub dir + [x], full dir + [x], full dir + [ ], stub dir + [ ],
ROADMAP-only + [x], and completed_count parity). Pre-fix: cases 1 and 6
failed. Post-fix: all six pass. No existing tests were modified.

Closes #2674
2026-04-24 20:20:11 -04:00
Tom Boucher
303fd26b45 fix(#2662): add state.add-roadmap-evolution SDK handler; insert-phase uses it (#2683)
/gsd-insert-phase step 4 instructed the agent to directly Edit/Write
.planning/STATE.md to append a Roadmap Evolution entry. Projects that
ship a protect-files.sh PreToolUse hook (a recommended hardening
pattern) blocked the raw write, silently leaving STATE.md out of sync
with ROADMAP.md.

Adds a dedicated SDK handler state.add-roadmap-evolution (plus space
alias) that:

  - Reads STATE.md through the shared readModifyWriteStateMd lockfile
    path (matches sibling mutation handlers — atomic against
    concurrent writers).
  - Locates ### Roadmap Evolution under ## Accumulated Context, or
    creates both sections as needed.
  - Dedupes on exact-line match so idempotent retries are no-ops
    ({ added: false, reason: "duplicate" }).
  - Validates --phase / --action presence and action membership,
    throwing GSDError(Validation) for bad input (no silent
    { ok: false } swallow).

Workflow change (insert-phase.md step 4):

  - Replaces the raw Edit/Write instructions for STATE.md with
    gsd-sdk query state.patch (for the next-phase pointer) and
    gsd-sdk query state.add-roadmap-evolution (for the evolution
    log).
  - Updates success criteria to check handler responses.
  - Drops "Write" from commands/gsd/insert-phase.md allowed-tools
    (no step in the workflow needs it any more).

Tests (vitest, sdk/src/query/state-mutation.test.ts): subsection
creation when missing; append-preserving-order when present;
duplicate -> reason=duplicate; idempotence over two calls; three
validation cases covering missing --phase, missing --action, and
invalid action.

This is the first SDK handler dedicated to STATE.md Roadmap
Evolution mutations. Other workflows with similar raw STATE.md
edits (/gsd-pause-work, /gsd-resume-work, /gsd-new-project,
/gsd-complete-milestone, /gsd-add-phase) remain on raw Edit/Write
and will need follow-up issues to migrate — out of scope for this
fix.

Closes #2662
2026-04-24 20:20:02 -04:00
Tom Boucher
7b470f2625 fix(#2633): ROADMAP.md is the authority for current-milestone phase counts (#2665)
* fix(#2633): use ROADMAP.md as authority for current-milestone phase counts

initMilestoneOp (SDK + CJS) derives phase_count and completed_phases from
the current milestone section of ROADMAP.md instead of counting on-disk
`.planning/phases/` directories. After `phases clear` at the start of a new
milestone the on-disk set is a subset of the roadmap, causing premature
`all_phases_complete: true`.

validateHealth W002 now unions ROADMAP.md phase declarations (all milestones
— current, shipped, backlog) with on-disk dirs when checking STATE.md phase
refs. Eliminates false positives for future-phase refs in the current
milestone and history-phase refs from shipped milestones.

Falls back to legacy on-disk counting when ROADMAP.md is missing or
unparseable so no-roadmap fixtures still work.

Adds vitest regressions for both handlers; all 66 SDK + 118 CJS tests pass.

* fix(#2633): preserve full phase tokens in W002 + completion lookup

CodeRabbit flagged that the parseInt-based normalization collapses distinct
phase IDs (3, 3A, 3.1) into the same integer bucket, masking real
STATE/ROADMAP mismatches and miscounting completions in milestones with
inserted/sub-phases.

Index disk dirs and validate STATE.md refs by canonical full phase token —
strip leading zeros from the integer head only, preserve [A-Z] suffix and
dotted segments, and accept just the leading-zero variant of the integer
prefix as a tolerated alias. 3A and 3 never share a bucket.

Also widens the disk and STATE.md regexes to accept [A-Z]? suffix tokens.
2026-04-24 18:11:12 -04:00