CodeRabbit pointed out the post-creation guard is structurally
unreachable: immediately after `git checkout -b X origin/$DEFAULT_BRANCH`,
HEAD == origin/$DEFAULT_BRANCH, so both the merge-base form (`MB == DT`)
and the alternative "ahead-of" count form (`AHEAD == 0`) are sentinels
that always pass on a successful fresh checkout. With the explicit base
arg + fail-fast on the checkout, the guard cannot catch anything new.
Removing it (rather than swapping in another no-op that satisfies the
linter but adds no actual coverage) is the honest fix. Comment retained
to explain why no post-creation guard is needed: the explicit base
argument to `git checkout -b` is the single source of correctness for
#2916.
Same simplification mirrored in get-shit-done/workflows/quick.md.
Full suite: 6102/6102.
Two CodeRabbit findings on PR #2921 (review 4209533909 + comment
3171721073, both still unresolved):
A. Branch switch and create steps now abort on non-zero exit. Previously
`git switch "$BRANCH_NAME"` and `git checkout -b "$BRANCH_NAME"
"origin/$DEFAULT_BRANCH"` could fail (locked worktree, dirty tree
refusing the checkout, etc.) and the workflow would silently continue
on the wrong branch — sending the phase's later commits to the wrong
place. Both calls now `|| { echo "ERROR: …" >&2; exit 1; }`.
B. The fork-point base-warning is now scoped to the creation arm of
the if/else. Previously it ran for the resume path too, so a
legitimate resumed branch where origin/$DEFAULT_BRANCH had advanced
since first creation would falsely warn ("does not fork from
origin/<DEFAULT_BRANCH>"). Moving the check inside the else arm
means it only runs immediately after a fresh `git checkout -b`, when
the merge-base check is meaningful.
Same fix mirrored in get-shit-done/workflows/quick.md.
execute-phase.md stays at the 1700-line XL budget. Full suite: 6102/6102.
Two follow-ups on commit 80f14cac (which hardened quick-branching with a
trunk fixture):
1. quick-branching.test.cjs: add a `defaultBranch` parameter to
setupFixture and run the "branches off origin/HEAD" assertion against
both `main` and `trunk`. The wholesale switch to trunk in 80f14cac
removed coverage of the conventional `main` path; parameterizing
restores it without giving up the symbolic-ref guarantee.
2. bug-2916-handle-branching-default-base.test.cjs: apply the same
parameterization here. handle_branching has the same default-branch
detection logic as Step 2.5, so it deserves the same trunk regression
guard. Previously this file only exercised `main`.
A regression that silently defaults to `main` instead of consulting
`git symbolic-ref refs/remotes/origin/HEAD` now fails the `trunk`
variant in both files.
Tests: 10/10 in the touched suites.
- Restrict the "init parse list includes branch_name" assertion to
the bash blocks inside Step 2 (Initialize) so an unrelated step
that mentions branch_name cannot mask the contract.
- Switch the fixture's default branch from main to trunk so the
symbolic-ref code path is locked in: a regression that silently
defaults to "main" instead of consulting origin/HEAD now fails.
Addresses CodeRabbit review on PR #2921.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the "ahead-of" heuristic with a structural check that compares
the HEAD↔origin/$DEFAULT_BRANCH merge-base to origin/$DEFAULT_BRANCH
itself. The previous count-based warning fired on legitimate WIP that
was simply ahead of the default branch — the correct signal is that
the branch did not fork from the default branch in the first place.
Addresses CodeRabbit review on PR #2921.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CodeRabbit nitpick (per project policy `feedback_no_source_grep_tests`):
the prior `tests/quick-branching.test.cjs` asserted branching correctness
by `.includes()`-grepping the raw markdown content for literal command
substrings. Those assertions stayed green even when the underlying
behavior regressed (e.g. when `git checkout -b` was unconditionally run
from the wrong HEAD).
Replace with the same pattern as `bug-2916-handle-branching-default-base
.test.cjs`:
- Structurally extract the Step 2.5 bash block from quick.md by
walking the markdown for fenced ```bash blocks under the heading
(no regex on prose).
- Spin up a fixture git repo with a bare origin, a clone whose
`origin/HEAD` points at `main`, and a checked-out previous-task
branch carrying its own unmerged commit.
- Execute the extracted bash block via `bash -c` and assert that
the new branch's tip equals `origin/main` (0 commits inherited
from the previous-task HEAD).
- Add a reuse test that pre-creates the target branch with its own
commit and verifies the script switches back to it without a
rebase or reset.
The two informational tests (workflow file exists, branching runs
before task-directory creation) are retained, plus the `branch_name`
parsing assertion is rewritten to walk fenced bash blocks rather than
substring-grep arbitrary content.
Address CodeRabbit HIGH findings on PR #2921. The previous fix had three
unconditional code paths where `git checkout -b "$BRANCH_NAME"` would run
from the *current* HEAD when the upstream sync failed silently:
- the dirty-tree warn-and-continue path,
- the clean path where `git switch` / `git merge --ff-only` errors were
swallowed by `2>/dev/null` (still falling through to checkout -b),
- any case where `git fetch` failed but the script continued.
This rewrites both `execute-phase.md` (handle_branching) and `quick.md`
(Step 2.5) to:
1. Fetch origin/$DEFAULT_BRANCH; if fetch fails AND no local copy of
origin/$DEFAULT_BRANCH exists, abort with a clear ERROR (exit 1)
rather than create the branch off arbitrary HEAD.
2. Always create the new branch with an explicit start point:
`git checkout -b "$BRANCH_NAME" "origin/$DEFAULT_BRANCH"`. The base
is now deterministic regardless of which branch is currently
checked out, regardless of whether the optional local fast-forward
succeeded, and regardless of dirty-tree state.
3. Carry uncommitted changes onto the new (origin-pinned) branch
instead of inheriting the previous-phase HEAD as a fallback base.
The post-creation INHERITED check now references origin/$DEFAULT_BRANCH
rather than the (possibly-stale) local default branch, so the warning
fires accurately even when the local fast-forward was skipped.
The docstring coverage pre-merge check (default: warning at 80% threshold)
produces false-positive warnings on PRs whose new code is entirely test
files: it counts test(...) / beforeEach / afterEach arrow-function
callbacks as functions and reports 0% coverage because nothing has JSDoc.
CR's documented schema for reviews.pre_merge_checks.docstrings only
accepts `mode` and `threshold` — there is no per-check path filter that
would let us exclude tests/** while keeping the check active elsewhere.
The top-level path_filters approach would silence ALL CR review on test
files (security scans, out-of-scope checks, the substantive line-level
findings) which we want to keep.
Disabling the check entirely is the right call for this repo because:
- GSD ships a CLI + agent runtime, not a documented public library
- The internal helpers that warrant JSDoc already have it
- The other CR pre-merge checks (out-of-scope, security, title) are
meaningful for this codebase and stay enabled
Closes#2932
Two existing tests called extractBlockquotes(reportStep) without first
asserting reportStep was non-null. If the workflow file ever loses its
`<step name="report">` block, the test would fail with a confusing
TypeError on the destructuring inside extractBlockquotes instead of a
clear "report step must exist" assertion.
Add assert.ok(reportStep, ...) guards at the two missing call sites
(lines 100 and 130). The other two call sites (lines 75-83) already
had guards.
Addresses CodeRabbit comment on PR #2918.
Ports the pre-publish CI gates that release.yml applies into release-sdk.yml,
so the stopgap workflow ships releases at the same quality bar as the
canonical workflow (minus the @gsd-build/sdk publish, still intentionally
omitted, and the release-branch ceremony, intentionally omitted).
Changes (all mechanical copies of release.yml patterns):
- install-smoke as needs: dependency. The reusable workflow at
.github/workflows/install-smoke.yml runs the cross-platform install
matrix (Ubuntu 22/24, macOS 24, packed-vs-unpacked). Publish job
won't start until install-smoke passes for the dispatched ref.
- npm test → npm run test:coverage. Full coverage gate, matching
release.yml's pre-publish test step.
- Tolerant tag-existence check. The previous upfront "refuse if tag
exists" was too strict — operators re-running after a mid-flight
publish-step failure would be blocked by the tag they successfully
pushed last time. New behavior matches release.yml: skip the tag
step if the tag points at HEAD; error only if it points elsewhere.
- Tag-and-push step gets the same skip-if-at-HEAD pattern.
- New "Re-point next dist-tag at the new latest" step, gated on
tag=latest. Matches release.yml#finalize "Clean up next dist-tag" —
keeps @next from going stale relative to @latest.
- New "Create GitHub Release" step. Per-tag flag selection:
tag=dev, tag=next → --prerelease (won't be highlighted on repo home)
tag=latest → --latest (becomes the highlighted release)
All use --generate-notes so the release body auto-fills from commits.
- Summary updated to mention the GitHub Release and dist-tag re-point.
Out of scope per #2929:
- canary.yml, release.yml unchanged (verified by file diff)
- bin/install.js unchanged (install path already uses bundled SDK)
- No @gsd-build/sdk publish anywhere
- No release/X.Y.Z branch ceremony (this stopgap targets dispatched
ref directly)
Adds a workflow_dispatch-only release path that publishes get-shit-done-cc
to ONE chosen dist-tag per run (dev | next | latest), with the SDK
bundled inside the CC tarball both as the existing loose sdk/dist/ tree
and as a fresh sdk-bundle/gsd-sdk.tgz npm-installable artifact.
Why: @gsd-build/sdk publishes from canary.yml and release.yml fail because
the @gsd-build npm token is currently unavailable. CC users don't consume
@gsd-build/sdk directly — bin/gsd-sdk.js resolves sdk/dist/cli.js from
inside the installed CC package. This workflow ships only get-shit-done-cc
(which we hold the token for) and bundles the SDK two ways so any future
install path can pick whichever shape it needs.
The new sdk-bundle/ directory is added to the CC files whitelist in-tree
at build time only — never committed. Existing canary.yml and release.yml
are intentionally untouched; restore them to primary use once the
@gsd-build/sdk token is recovered.
Per-tag version derivation when the version input is empty:
- dev → <base>-dev.N (next sequential, scanning v<base>-dev.* tags)
- next → <base>-rc.N (matches release.yml convention)
- latest → <base> (clean, no suffix)
Refuses to publish when the version already exists on npm or has an
existing git tag (no accidental overwrites). Verifies the publish landed
on the registry and the dist-tag resolves correctly before marking the
run successful.
handle_branching in execute-phase.md (and the equivalent step in quick.md)
created the per-phase branch from whatever branch happened to be checked
out — typically the previous phase's still-unmerged feature branch — so
consecutive phases compounded on top of each other and stayed unpushed.
Detect the default branch via git symbolic-ref refs/remotes/origin/HEAD,
fast-forward it from origin, and fork the new phase branch off that tip.
Existing branches are still reused as-is. Dirty working trees fall back
to current HEAD with a loud warning, and a post-creation guard reports
any inherited commits.
Regression test extracts the bash from the <step name="handle_branching">
block structurally and runs it against a fixture repo where HEAD sits on
a previous-phase branch with extra commits.
Two bugs in the audit-open dispatch case in bin/gsd-tools.cjs:
1. Bare output(...) calls (only core.output is in scope) threw
ReferenceError: output is not defined on every invocation,
blocking the first step of /gsd-complete-milestone.
2. Even after switching to core.output(formattedReport, raw), the
human-readable branch JSON-stringified the formatted text because
core.output only bypasses JSON encoding when called as
core.output(null, true, rawValue).
Fix:
- --json path: core.output(result, raw) — pass the object,
let core.output JSON-stringify (don't pre-stringify).
- text path: core.output(null, true, formatAuditReport(result))
— use the rawValue form to emit verbatim section dividers and
item lists.
Adds tests/bug-2911-audit-open-output-shape.test.cjs which parses
both modes structurally — line-by-line for text mode (asserting the
report headers exist as standalone lines, not as escaped \n inside a
JSON quoted string), and JSON.parse + key-by-key shape assertions for
--json mode (matching the contract returned by auditOpenArtifacts).
The report step in workflows/progress.md had no directive establishing
PROJECT.md/STATE.md/ROADMAP.md as the authoritative sources for the
progress report. When init.progress returned project_exists: false (e.g.
invoked from a subdirectory without .planning/), the model fell back to
whatever was in its session context — including stale CLAUDE.md
## Project blocks — and produced routing output citing the wrong
milestone/phase.
Add a blockquote directive at the top of the report step that names
PROJECT.md, STATE.md, and ROADMAP.md as authoritative and forbids using
the CLAUDE.md ## Project block as a source for any progress report field.
Fixes#2912
* fix: parse non-REQ IDs in gap-analysis and ignore table headers
* fix: parse requirement IDs from first traceability column only
---------
Co-authored-by: Tom Boucher <thomas.boucher@sas.com>
* fix(#2893): surface non-canonical plan filenames instead of silently returning zero plans
Reporter saw `plan_count: 0` from `/gsd:execute-phase` even though five
plan files existed on disk. Investigation showed the planner had written
files like `01-PLAN-01-foundation.md`, while `phase-plan-index`'s strict
filter (`f.endsWith('-PLAN.md') || f === 'PLAN.md'`) rejected them
silently — collapsing two distinct states into the same `plans: []`
return:
- directory truly has no plans (legit empty)
- directory has plans but the filter rejected them (user/agent error)
The canonical contract is documented in three places:
- `agents/gsd-planner.md` write_phase_prompt step (lines 1063-1080)
- `commands/gsd/plan-phase.md`
- `references/universal-anti-patterns.md` (rule 26)
It mandates `{padded_phase}-{NN}-PLAN.md` and explicitly forbids
`PLAN-NN.md` / `01-PLAN-01.md` / `plan-NN.md` etc. The strict filter is
correct per that contract. The bug is that the executor never tells the
user when the contract was violated — they just see `plan_count: 0`
with no signal.
Fix: add a diagnostic helper `describeNonCanonicalPlans()` that scans
the phase directory for files matching `*PLAN*.md` (the diagnostic net)
that the canonical filter rejected, excluding legit derivatives like
`*-PLAN-OUTLINE.md` and `*-PLAN.pre-bounce.md`. When offenders exist,
return a `warning` field naming each one and citing the canonical
pattern so the user knows what to rename to.
Wired into the three filter sites:
- `phase-plan-index` (the executor's main entry point)
- `phases list --type plans`
- `find-phase`
The strict filter itself is unchanged — existing canonical plans behave
identically. This is purely a diagnostic that converts silent-empty
into loud-with-actionable-error.
Tests:
- `phase-plan-index returns warning for reporter's exact filename
pattern (`01-PLAN-01-foundation.md`)`
- `truly empty dir does not emit a warning`
- `canonical plans + outline + pre-bounce files do not emit a warning`
Closes#2893
* test(#2893): add parity tests for find-phase and phases list --type plans warnings
CodeRabbit's only finding on the prior commit: I wired the warning into
three filter sites (`phase-plan-index`, `find-phase`,
`phases list --type plans`) but only `phase-plan-index` had test
coverage for the warning shape. The other two paths could silently
diverge during future refactors — exactly the silent-drift class of bug
this fix exists to prevent.
Add four parity tests mirroring the existing two:
- find-phase: non-canonical filenames produce a warning naming each
offender + citing the canonical pattern.
- find-phase: canonical plan + derivative files (PLAN-OUTLINE,
pre-bounce) produce no warning.
- phases list --type plans: same non-canonical case, but assert the
warning is prefixed with `${dir}: ` (this path aggregates across
phase directories so each offender is tagged with its dir).
- phases list --type plans: canonical case, no warning.
`node --test tests/phase.test.cjs`: 98/98 pass (was 94, +4 new).
* 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`.
Closes#2876 follow-up — CI on main fails because the punctuation test
in tests/gemini-namespacing.test.cjs hardcoded `/gsd-scan` as a known
command, but #2824 (consolidate 86 → 59 skills) removed scan.md from
commands/gsd/. The roster now correctly returns "scan is unknown, leave
unchanged" — the conversion is right, the test fixture is stale.
Swap `scan` for `health` in the punctuation test. Both are bedrock
commands; the test still exercises the original intent (period vs
exclamation handling on adjacent slash commands).
Note added so the next consolidation reviewer knows the swap pattern.
`npm test`: 5936/5936 pass.
* feat(#2833): parseStateMd reads phase-lifecycle frontmatter fields
Extend parseStateMd() to parse 4 new STATE.md frontmatter fields that drive
the phase-lifecycle status-line proposed in #2833:
- active_phase : phase number when orchestrator is in-flight, null when idle
- next_action : recommended next command when idle
- next_phases : YAML flow array of phase numbers for next_action
- progress : nested block with completed_phases / total_phases / percent
All fields default to undefined when absent — formatGsdState() (next commit)
degrades gracefully so existing STATE.md files keep rendering as before.
YAML scope intentionally narrow:
- Only top-level scalar keys (status, milestone, active_phase, next_action)
- Only single-line flow array for next_phases ([...])
- progress block requires 2-space indent for nested keys
Block sequences (- item over multiple lines) and inline comments inside
nested blocks are NOT parsed — keeping the regex-based parser predictable.
Comments outside frontmatter or after the closing --- still work.
Tests: all 27 existing tests still pass (no behavior change for STATE.md
files that don't carry the new fields).
Refs #2833
* feat(#2833): formatGsdState renders phase-lifecycle scenes + opt-in progress bar
Extend formatGsdState() with three lifecycle scenes that activate when the
new STATE.md frontmatter fields (added in the previous commit) are present.
Also append an opt-in progress bar to the milestone segment when
progress.percent is available.
Scenes (first match wins; falls through to the existing path otherwise):
1. active_phase set → 'v2.0 [██░] X% · Phase 4.5 executing'
(status field carries the lifecycle stage:
discussing / planning / executing / verifying)
2. active_phase null + → 'v2.0 [██░] X% · next execute-phase 4.5'
next_action set (idle state — surfaces what the user should
run next without opening STATE.md)
3. percent=100 (or → 'v2.0 [██████████] 100% · milestone complete'
completed=total)
4. (default fallback) → 'v1.9 Code Quality · executing · ph (1/5)'
(existing rendering, byte-for-byte preserved
when none of the new fields are populated)
Backward compat is the design priority:
- STATE.md files without the new fields render identically to v1.38.x
- progress bar is opt-in (empty string when percent absent)
- Each new scene only activates when its specific fields are populated
A new helper renderProgressBar() generates the 10-segment bar that matches
the existing context meter style (so the two bars on the status-line are
visually consistent).
Tests: 27/27 existing tests still pass.
Refs #2833
* test(#2833): cover parseStateMd lifecycle fields + formatGsdState scenes
26 new tests organized in 5 describe blocks, modeled after the existing
enh-2538-statusline-last-command.test.cjs convention:
parseStateMd #2833 lifecycle fields (7 tests)
- reads active_phase / next_action / next_phases / progress.percent
- 'null' literal handled correctly
- YAML flow array parsing (1 item, multiple items)
- progress nested block (3 fields)
- absent fields return undefined
formatGsdState #2833 lifecycle scenes (6 tests)
- Scene 1: active_phase set → 'Phase X.Y <stage>'
- Scene 2: idle + next_action → 'next <action> <phases>' (1+ phases)
- Scene 3: percent=100 OR completed=total → 'milestone complete'
formatGsdState #2833 backward compatibility (4 tests) — CRITICAL
- Legacy STATE.md (no new fields) renders byte-for-byte unchanged
- Empty state, partial state, progress-bar-opt-in all preserved
progress bar rendering (6 tests)
- 0% / 50% / 100% / clamping / opt-in absence
formatGsdState #2833 scene priority (3 tests)
- active_phase wins over next_action when both populated
- next_action wins over fallback when active_phase null
- percent=100 wins over fallback even with phase set
Combined run: 53/53 tests pass (existing 27 + new 26).
Refs #2833
* docs(#2833): describe phase-lifecycle frontmatter fields and rendering scenes
Add docs/STATE-MD-LIFECYCLE.md as the canonical reference for the four new
STATE.md frontmatter fields and the four status-line rendering scenes
introduced by this proposal:
- Frontmatter field reference (active_phase / next_action / next_phases /
progress.percent) with type and population semantics
- Why progress.percent is intentionally the phase dimension and not the
plans dimension (plans dimension trends optimistic when future phases
are unplanned)
- The four rendering scenes including their priority order
- Stage-label convention for Scene 1 (discussing / planning / executing /
verifying matching the four phase orchestrators)
- Frontmatter parsing constraints — frontmatter must start at file head,
no comments inside nested blocks, next_phases is single-line flow only
- Backward-compatibility guarantee (locked in by the test suite)
- Cross-links to the foundation issue #1989 and the read-side issues
this proposal helps close
The document deliberately scopes itself to the read-side (what the hook
parses, what it renders). Write-side SDK and workflow changes that
auto-maintain the fields are out of scope for this PR so each piece can
be reviewed independently — see the issue thread for the full proposal.
Refs #2833
* test(#2833): simplify '0% renders 10 empty segments' assertion
Address CodeRabbit nitpick — drop the convoluted assert.equal that built
the expected value via .replace() and rely on the existing assert.ok
includes-check. The behavior under test is unchanged; the assertion is
just easier to read.
Refs #2884 review comment
* feat(#2790): consolidate 86 gsd-* skills to 59 — zero functional loss
Closes#2790
- `capture.md` — absorbs add-todo (default), note (--note), add-backlog
(--backlog), plant-seed (--seed), check-todos (--list)
- `phase.md` — absorbs add-phase (default), insert-phase (--insert),
remove-phase (--remove), edit-phase (--edit)
- `config.md` — absorbs settings-advanced (--advanced),
settings-integrations (--integrations), set-profile (--profile);
settings.md retained as-is
- `workspace.md` — absorbs new-workspace (--new), list-workspaces
(--list), remove-workspace (--remove)
- `update.md` — adds --sync (absorbs sync-skills) and --reapply
(absorbs reapply-patches)
- `sketch.md` — adds --wrap-up (absorbs sketch-wrap-up)
- `spike.md` — adds --wrap-up (absorbs spike-wrap-up)
- `map-codebase.md` — adds --fast (absorbs scan) and --query (absorbs
intel)
- `code-review.md` — adds --fix (absorbs code-review-fix)
- `progress.md` — adds --next (absorbs next) and --do (absorbs do)
join-discord, research-phase, session-report, from-gsd2,
analyze-dependencies, list-phase-assumptions, plan-milestone-gaps
autonomous.md: updated Skill(skill="gsd:code-review-fix") →
Skill(skill="gsd:code-review", args="--fix --auto") to match
the consolidated skill name
- New: tests/enh-2790-skill-consolidation.test.cjs (48 tests)
- Updated: 14 existing test files redirected from deleted command paths
to their consolidated equivalents
- docs/INVENTORY.md: Commands count 86→59, ghost rows removed, new
consolidated rows added
- docs/INVENTORY-MANIFEST.json: regenerated to match filesystem
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(#2790): add CHANGELOG entry for skill consolidation
* docs(#2790): update COMMANDS.md for 86→59 skill consolidation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#2790): address CodeRabbit review findings
- CHANGELOG.md: add --next alongside --do in progress flag list
- config.md: remove trailing space from --profile code span (MD038)
- COMMANDS.md: add required descriptions to /gsd-phase examples;
/gsd-phase without args errors, not interactive
- COMMANDS.md: add --next and --do to /gsd-progress flags table + examples
- test: convert content.includes('--reapply') to structural frontmatter
parse; add allow-test-rule comment for workflow content assertions
- test: replace redundant existsSync duplicate with assertion that verifies
the full consolidated flag surface (--sync | --reapply) in argument-hint
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#2790): restore reapply-patches workflow and strengthen test assertions
- Create get-shit-done/workflows/reapply-patches.md: the #2790 consolidation
deleted the 14K combined command+workflow file (reapply-patches.md) but
update.md already referenced the workflow via execution_context_extended.
Restoring it fixes a silent behavioral gap where --reapply had no workflow
to load. Includes full three-way merge logic, hunk verification table
(Step 4), and the Hunk Verification Gate (Step 5) that blocks cleanup
until all user-added hunks are confirmed present in the merged output.
- Fix update.md: /gsd-reapply-patches → /gsd-update --reapply (stale ref)
- Fix reapply-verify-hunks.test.cjs: was checking existsSync(update.md) 8×;
now points to the workflow file and asserts real behavioral content
(Post-merge verification, Hunk presence check, Line-count check, backup
reference, per-file tracking, structural ordering)
- Fix reapply-patches.test.cjs: replace content.includes() stubs with
frontmatter-parsed argument-hint assertions; replace 4 existsSync(update.md)
no-ops with real assertions against the workflow content
- Fix edit-phase.test.cjs: /gsd-edit-phase → /gsd-phase (COMMANDS.md now
documents the consolidated command with --edit flag)
- Fix next-safety-gates.test.cjs: split OR predicates into independent
assertions — --next in progress.md and --force in next.md workflow
- Fix workspace.test.cjs: add allow-test-rule comment for routing content
checks (command routing text IS the deployed behavioral contract)
- Fix bug-2439 test: strengthen pre-flight assertion to verify gsd-sdk is
referenced (not just --profile)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address CodeRabbit review findings (CR round 2)
- INVENTORY.md: update sync-skills.md row to reference /gsd-update --sync
instead of stale /gsd-sync-skills (absorbed in #2790)
- enh-2380-sync-skills.test.cjs: align INVENTORY.md assertion with the
corrected reference; was asserting the old /gsd-sync-skills name while the
manifest test correctly asserted /gsd-update, creating conflicting expectations
in the same suite
- reapply-verify-hunks.test.cjs: add explicit notEqual(-1) assertions for all
three anchors before the ordering check so a missing anchor produces a clear
failure instead of a false positive (writeIdx=-1 < verifyIdx=5 is true)
- bug-2439-set-profile-gsd-sdk-preflight.test.cjs: defer fs.readFileSync until
after the existence assertion; eager describe-level read caused the suite to
crash before the existence test could run, making it effectively dead code
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#2790): address CR — INVENTORY routing + reapply test contract wording
Two unresolved CodeRabbit findings (Major):
- docs/INVENTORY.md: workflow-file table still pointed at obsolete
/gsd-do, /gsd-next, /gsd-note, /gsd-add-todo, /gsd-add-backlog,
/gsd-check-todos, /gsd-plant-seed slash commands. Re-route to the
consolidated /gsd-progress (--next, --do) and /gsd-capture (--note,
--backlog, --seed, --list) so the inventory is internally consistent.
- tests/reapply-verify-hunks.test.cjs: 'verification tracks per-file
status' asserted on phrasing that doesn't appear in reapply-patches.md
(the 'per-file' substring only matched accidentally via 'sequential
integer per file'). Switch to the actual contract text — Hunk
Verification Table, one row per hunk per file, verified column.
* test(#2790): update CR-INTEGRATION tests for consolidated --fix invocation
After the merge of main (which carries #2843's hyphen-form fix), the
consolidation in this branch absorbs gsd-code-review-fix into gsd-code-review
as the --fix flag. Update the two CR-INTEGRATION tests that previously
asserted on the standalone gsd-code-review-fix skill name to instead assert
on a gsd-code-review invocation carrying --fix in its arg tokens.
Tests still parse Skill() invocations structurally; only the asserted
skill-name + arg-token shape changed.
* test(#2790): scope success_criteria check to the <success_criteria> block
CodeRabbit nitpick: 'success criteria includes verification' did a
whole-file substring check, which can false-pass if the phrase appears
elsewhere in the document. Extract the <success_criteria>...</success_criteria>
block first via extractTagBlock() and assert against that scope only.
* fix(#2790): post-rebase reconciliation with main
- INVENTORY.md/JSON: add reapply-patches workflow row + bump count to 85
- autonomous.md: switch consolidated --fix invocation to hyphen Skill name
- analyze-dependencies test: assert COMMANDS.md does NOT document the
consolidated-away /gsd-analyze-dependencies entry (was: bare .includes())
* fix(#2790): address remaining CR findings — strengthen contract tests
Doc-fixes:
- INVENTORY.md: route transition.md & edit-phase.md rows to consolidated
/gsd-progress --next and /gsd-phase --edit (was: deleted /gsd-next, /gsd-edit-phase)
- config.md --profile branch: document #2439 pre-flight `command -v gsd-sdk`
guard + install hint BEFORE the gsd-sdk invocation (closes opaque
"command not found: gsd-sdk" regression path)
Test discipline (no-source-grep contract):
- bug-2439: replace bare `content.includes('gsd-sdk')` with structured
parse of <context> block + --profile branch; assert pre-flight token,
install hint, #2439 citation, and ordering vs gsd-sdk invocation
- edit-phase: parse INVENTORY.md edit-phase.md row's "Invoked by" column
and assert `/gsd-phase --edit` (not the deleted /gsd-edit-phase)
- next-safety-gates: tighten `--next` documentation contract — require
--next AND --force AND completeness routing (was OR-based, passed when
only --next present)
- reapply-patches: parse argument-hint flag list structurally; scan ALL
<execution_context*> blocks for the @-include of reapply-patches.md;
parse Hunk Verification Table header columns directly; locate Step 5
via heading parsing then assert (i) table reference, (ii) verified=no
gate, (iii) STOP/halt directive, (iv) explicit absent-table halt path
- workspace: parse frontmatter, tokenize argument-hint across multiple
bracketed segments, parse @-include targets from <execution_context>
rather than substring-matching the file body
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#2876): yamlQuote description in Copilot/Antigravity/Trae/CodeBuddy SKILL.md
A description starting with `[BETA]` (or any YAML flow indicator —
`{`, `*`, `&`, `!`, `|`, `>`, `%`, `@`, backtick) is parsed as a flow
sequence/mapping by YAML 1.2-strict loaders. gh-copilot's frontmatter
loader fails closed:
✖ ~/.copilot/skills/gsd-ultraplan-phase/SKILL.md: failed to parse YAML
frontmatter: Unexpected scalar at node end at line 2, column 21:
description: [BETA] Offload plan phase to Claude Code's ultraplan…
Six emission sites in `bin/install.js` re-wrote the description without
quoting, while the Claude variant (`convertClaudeCommandToClaudeSkill`)
already routed it through `yamlQuote`. Brought all six in line:
- convertClaudeCommandToCopilotSkill
- convertClaudeAgentToCopilotAgent
- convertClaudeCommandToAntigravitySkill
- convertClaudeAgentToAntigravityAgent
- convertClaudeCommandToTraeSkill
- convertClaudeCommandToCodebuddySkill
Each now wraps the value in `yamlQuote(...)` so any leading character is
parser-safe.
Regression test (tests/bug-2876-skill-frontmatter-quote.test.cjs) drives
the four command converters and two agent converters through the
reporter's exact "[BETA] …" description plus a grab-bag of YAML flow
indicators, asserting the emitted `description:` value is a quoted YAML
scalar. Also round-trips the value through `JSON.parse` for converters
that don't apply runtime-name substitution to confirm fidelity.
Updated 7 pre-existing substring assertions in copilot-install.test.cjs
and antigravity-install.test.cjs that hard-coded the unquoted form.
Round trip: 5893/5893 pass on `npm test`.
Closes#2876
* test(#2876): structurally parse frontmatter instead of substring-grep
Addresses CodeRabbit's two nitpicks on PR #2881: the pre-existing
substring assertions in copilot-install.test.cjs (4 sites) and
antigravity-install.test.cjs (3 sites) only got bumped from the unquoted
form (`description: Diagnose...`) to the quoted-prefix form
(`description: "Diagnose...`). Both are still raw-string checks against
rendered YAML and drift on any quoting/order change — exactly what the
project's CONTRIBUTING.md "no-source-grep" testing standard exists to
prevent.
Add `parseFrontmatter()` to tests/helpers.cjs — a small parser that
handles the YAML scalar forms the install converters emit (double-quoted
JSON, single-quoted with `''` escape, bare). Throws if the content has
no closed `---` block so a regression in the emitter shape fails loudly
rather than silently returning {}.
Refactor the 7 description-substring sites to compare on parsed values:
the assertion now reads as `fm.description === 'Diagnose planning
directory health'` rather than `result.includes('description: "Diagnose
planning directory health')`. Same coverage of the #2876 quoting
behavior, no coupling to byte-level quote style.
`npm test`: 5893/5893 pass.
Closes#2876
* test(#2876): make parseFrontmatter delimiter check CRLF/whitespace tolerant
CR nitpick on PR #2881 (review at 03:08:08Z): parseFrontmatter()
splits on '\n' and compares each line strictly to '---'. A
Windows-authored skill file (CRLF endings) leaves a trailing '\r'
on every line, so '---\r' fails the equality check, and the helper
throws "no closed --- block" on perfectly valid input. Same problem
with whitespace-padded delimiter lines.
Switch to splitting on /\r?\n/ and comparing the trimmed line.
Helper is used by tests/copilot-install.test.cjs and
tests/antigravity-install.test.cjs, so this also de-flakes those
suites on Windows runners.
5893/5893 on `npm test`.
* fix(commands): normalize gsd slash namespace drift
* fix(#2855): address CodeRabbit findings on namespace drift PR
Three CR findings, all valid:
1. autonomous.md line 783 still had `gsd:discuss-phase` (the PR's own
normalization missed this line). Switched to `gsd-discuss-phase` and
updated the matching test in autonomous-interactive.test.cjs that was
asserting the now-retired colon form.
2. tests/bug-2543-gsd-slash-namespace.test.cjs source-grepped the
fix-slash-commands.cjs script with .includes() rather than driving
its transform behaviour. Refactored fix-slash-commands.cjs to export
a pure transformContent(src, cmdNames) function, kept the CLI behaviour
unchanged via require.main, and replaced the source-grep block with
five behavioural cases: rewrite, multi-occurrence, idempotence on
canonical input, no-op on gsd-sdk/gsd-tools, and word-boundary safety.
3. tests/bug-2808-skill-hyphen-name.test.cjs matched `name:` anywhere
in SKILL.md; a stray name: in the body could satisfy the assertion.
Scoped the lookup to the YAML frontmatter block via the suggested
diff (parse the leading --- ... --- region first, then find name:
inside it).
Full suite: 5854/5854 passing.
* fix(#2855): address remaining CodeRabbit findings on PR #2858
Three structural concerns flagged on the namespace-drift fix PR:
1. scripts/fix-slash-commands.cjs:24 — `buildPattern([])` compiled
`/gsd:()(?=[^a-zA-Z0-9_-]|$)/g`. The empty capture group still
matches any `/gsd:` token followed by a non-word boundary
(whitespace, EOL, punctuation), rewriting it to a stray `/gsd-`.
Verified live: `transformContent("/gsd:", [])` → `"/gsd-"`. Added
a guard returning null from `buildPattern` on empty input and
updated `transformContent` and `processDir` to no-op when the
pattern is null.
2. tests/autonomous-interactive.test.cjs:44-47 — assertion was
`content.includes('gsd-discuss-phase') && content.includes('INTERACTIVE')`,
which would false-pass on any unrelated co-occurrence (e.g.
`INTERACTIVE=""` initialization plus a stray `gsd-discuss-phase`
prose mention). Replaced with a structural extraction: locate the
`**If \`INTERACTIVE\` is set:**` branch, bound it by the next
`**If` / `<step>` boundary, and assert the
`Skill(skill="gsd-discuss-phase", ...)` invocation lives inside
that region. Tolerates whitespace around `(`, `skill`, and `=`.
3. tests/bug-2808-skill-hyphen-name.test.cjs:104 — colon-call regex
was `Skill\(skill=...` and missed valid formatting like
`Skill(skill = "gsd:cmd")` or `Skill( skill = ...)`. Loosened to
`Skill\(\s*skill\s*=\s*...` so reformatting drift can't slip past
the namespace guard.
Verification: 5854/5854 pass on `npm test` from the rebased branch.
* fix(#2855): drop pre-validation filter that hid namespace drift
CR finding on tests/bug-2808-skill-hyphen-name.test.cjs:128: the test
collected generated skill directories with
`.filter(entry => entry.isDirectory() && entry.name.startsWith('gsd-'))`,
then validated namespace invariants over that filtered list. Anything
that violated the prefix invariant — `gsd:extract-learnings` (colon
form), `extract_learnings` without prefix, `Gsd-foo` mis-cased — would
silently disappear from the iteration and the test would falsely pass.
Drop the `startsWith('gsd-')` filter so every generated directory shows
up. Add explicit assertions before the existing per-skill loop:
- directory list is non-empty (catches a broken converter that
produces nothing)
- every directory begins with `gsd-`
- every directory contains no `:`
- every directory contains no `_`
Re-audited the full PR diff for the same anti-pattern: only this one
site filtered before validating the namespace; bug-2643 and
commands-doc-parity also use `readdirSync().filter()` but only by file
extension, which is correct.
5854/5854 on `npm test`.
* fix(#2855): address remaining CR findings (1 active + 2 nitpicks)
Three findings on PR #2858, all the same root cause: input narrowing
before validation lets drift slip past the guards.
1. tests/bug-2808-...:104 (active) — `colonCallRe` captured local names
with `[a-z0-9-]+`, which excluded the underscore. A drift like
`Skill(skill="gsd:extract_learnings")` (deprecated colon syntax with
the old underscore filename) silently slid through. Broadened the
capture to `[^'"\s)]+` so any malformed local name is surfaced; surrounding pattern (whitespace tolerance, escape support, flags)
unchanged.
2. tests/bug-2643-...:43 (nitpick) — `extractSkillNamesHyphen` and
`extractSkillNamesColon` had the same over-strict capture plus
relied on a single regex over raw bytes, which the project test-
rigor memory bans (`feedback_no_source_grep_tests.md`). Replaced
with `extractSkillCalls(content)` — a small structural extractor
that walks `Skill(` openers, locates each call's matching `)`,
parses the body's `skill = "..."` keyword argument with permissive
whitespace + quoting + escape handling, and returns
`{ name, raw }` records. The two namespace-form helpers become
thin filters over the structured output. Tightened the body class
to `[^'"\\]+` so a trailing escape `\` before the closing quote
(as in `Skill(skill=\"gsd-foo\", …)` written inside another string
context) doesn't get included in the captured name.
3. tests/bug-2543-...:44 (nitpick) — `DOC_SEARCH_FILES` was a hand-
curated 7-entry array. Every doc added in the future would silently
weaken drift detection until someone remembered to extend the list.
Replaced with `discoverDocSearchFiles(ROOT)`: globs every `.md`
under `docs/` and adds `README.md` if present. New docs are picked
up automatically.
Re-audited the diff surface for similar narrowings; no other sites
filter or constrain before validating namespace invariants.
5854/5854 on `npm test`.
* fix(#2855): recurse docs/ tree so localized translations are scanned too
CR finding: discoverDocSearchFiles() stopped at docs/*.md, leaving
localized translation trees (docs/ja-JP/, docs/zh-CN/, docs/ko-KR/,
docs/pt-BR/) and other nested doc collections (docs/skills/,
docs/superpowers/) invisible to the namespace-drift invariant.
Verified the gap: docs/ has 6 nested directories with ~30 .md files
that the previous top-level-only scan was skipping. None contain
/gsd: references today, but a future translation update or new
doc subdir could leak drift.
Switch to an iterative stack walk so every .md under docs/ is scanned
regardless of depth. Stack form (rather than recursion) avoids the
risk of running into the call-stack limit on deep doc trees.
5854/5854 on `npm test`.
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
* fix(install): use colon namespace for Gemini slash commands and help reference
This fixes unexecutable command recommendations in Gemini CLI by correctly
namespacing slash commands (/gsd: instead of /gsd-) in all installed
artifacts (agents, commands, workflows).
- Implements a lazy command roster discovery to ensure 100% accurate
conversion and protect file paths, URLs, and agent names.
- Adds isolated behavioral and unit tests covering all boundary cases.
- Fixes hardcoded command strings in banners and help output.
Closes#2783
* fix(install): close roster gaps in Gemini /gsd- → /gsd: conversion (#2783)
Addresses adversarial review findings on PR #2768:
- Restore regex boundaries (lookbehind + extension lookahead). Roster-only
matching was insufficient: a URL like `https://example.com/gsd-plan-phase`
ends in a known command and would be incorrectly converted. Boundaries +
roster now agree before any conversion fires.
- Smarter trailing lookahead `(?!\.[a-z])` distinguishes file extensions
(`.cjs`, `.md`) from sentence-ending punctuation (`.` at end of input or
before whitespace), so `/gsd-help.` correctly converts.
- Fail loud on missing roster. `commands/gsd/` not found previously fell
through to an empty Set, silently no-op'ing every conversion — exactly the
bug this code exists to prevent. Now emits a one-shot console.warn (gated
on GSD_TEST_MODE) before returning the empty set.
- Drop unnecessary `i` flag — GSD commands are always lowercase; matching
uppercase tokens against a lowercase roster always misses anyway.
- Export `_resetGsdCommandRoster` for test isolation against the module-level
cache.
Test additions pin the actual safety property of the roster check by using
KNOWN command names embedded in URLs and sub-paths — the cases the prior
tests didn't reach because they used `gsd-tools` (not in roster). Added a
roster-load assertion that fails loudly if the empty-Set fallback path
silently neutralises conversions.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(install): centralize <sub> stripping and add structural test assertions
CodeRabbit findings on the prior commit:
- (actionable) Centralizing the Gemini conversion through
convertClaudeToGeminiMarkdown dropped the stripSubTags() call that the
inline command path used to make before TOML conversion. Move stripSubTags
inside convertClaudeToGeminiMarkdown so command/agent/non-command Gemini
outputs all have <sub> consistently stripped. Remove the now-redundant
stripSubTags call in convertClaudeToGeminiAgent (single source of truth).
- (nitpick) Replace `.includes()` checks in the TOML test with structured
parsing — JSON-decode each TOML value and assert on parsed fields, per
the project's "tests parse, never grep" convention.
- (nitpick) Strengthen the install behavioral test to read a real installed
artifact (.gemini/commands/gsd/plan-phase.toml), parse it, and assert the
prompt body actually contains a /gsd: reference and no unconverted
/gsd-plan-phase. A directory-only check would have passed even if every
conversion silently no-op'd.
- Add a regression test that <sub> tags are stripped through the
convertClaudeToGeminiMarkdown pipeline.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#2851): replace bare gsd-tools invocations with absolute path
`gsd-tools` is not a published bin entry — package.json declares only
get-shit-done-cc and gsd-sdk. The shipped invocation pattern is
`node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" <subcommand>`,
used by every other workflow file.
Two leaked bare invocations:
- get-shit-done/workflows/plan-phase.md §13e (gap-analysis)
— reported in #2851; gap-analysis silently skipped on every plan-phase run
- get-shit-done/workflows/ingest-docs.md §finalize (commit)
— caught by the new structural test; ingest-docs commit step was broken
Both updated to canonical absolute-path form.
Adds tests/bug-2851-workflow-bare-gsd-tools.test.cjs which parses every
markdown file under get-shit-done/workflows/, extracts shell-fenced code
blocks, tokenizes each line, and asserts no token in command position is
the bare string `gsd-tools` (the trailing `.cjs` is a different token).
The test also asserts plan-phase.md's gap-analysis call uses the canonical
`node …/gsd-tools.cjs` form.
Closes#2851
* fix(#2851): catch third bare gsd-tools call in ingest-docs.md init
After the first commit, the structural test was strengthened to detect
bare `gsd-tools` inside `$(...)` and backtick command-substitution forms.
The improved test surfaced a third leak:
ingest-docs.md:55: INIT=$(gsd-tools init ingest-docs)
Fixed to canonical form
INIT=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" init ingest-docs)
plus the standard `@file:` handoff line that every other workflow uses
when capturing INIT (required by tests/windows-robustness.test.cjs).
Updated tests/bug-2801-ingest-docs-handler.test.cjs to match either
the bare `gsd-tools init ingest-docs` or canonical
`gsd-tools.cjs" init ingest-docs` form — the test's intent is to verify
the dispatch handler is wired, not to lock the bare-bin form that #2851
removes.
Closes#2851
* test(#2851): tighten ingest-docs and gap-analysis assertions to canonical form
CodeRabbit caught two soft assertions in the regression tests:
1. tests/bug-2801: the init-ingest-docs assertion accepted both the
legacy bare `gsd-tools` form and the canonical node-path form.
Since #2851 is the fix that removes the bare form, the test should
only accept the canonical absolute-path invocation. Switched to
parsed-bash-block extraction with an anchored regex on the full
`node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs"` path.
2. tests/bug-2851: the gap-analysis assertion used two loose
.includes()/word-boundary checks. Replaced with a single
assert.match() against the full canonical path so non-canonical
forms fail.
* test(#2851): env-assignment skip accepts lowercase identifiers too
CodeRabbit caught: the cmdIdx-skip regex /^[A-Z_][A-Z0-9_]*=/ only
matched uppercase variable names, so a line like `tmp=1 gsd-tools init`
would tokenize to ['tmp=1','gsd-tools','init'], the regex would fail on
'tmp=1', cmdIdx would stay at 0, and the command-position check would
compare 'tmp=1' against 'gsd-tools' — false negative.
POSIX shell variable names are [A-Za-z_][A-Za-z0-9_]*. Widen the regex
to match the actual lexical rule. Existing uppercase forms still work
(FOO=bar gsd-tools); now lowercase forms (tmp=1 gsd-tools) and mixed
case forms are also detected.
* fix(#2866): Codex installer strips legacy hooks at end-of-file without trailing newline
The four shape-strip regexes in `bin/install.js` (Codex install path)
required `\r?\n` at end. A stale GSD hook block sitting at end-of-file
without a trailing newline (common — many editors strip them, and the
legacy installer never wrote one) failed every shape, the installer
saw `gsd-check-update` already present, skipped writing the new
Nested-AoT block, and Codex 0.125+ refused to load with
invalid type: map, expected a sequence in `hooks`
Root cause + fix
================
Each shape's terminator changed from `\r?\n` to `(?:\r?\n|$)`, so
end-of-file is also a valid terminator.
Strip logic was lifted into a new pure helper
`stripStaleGsdHookBlocks(configContent)` that the install pipeline now
calls in place of the inline replace chain. The helper is exported via
the GSD_TEST_MODE module.exports for direct unit-test coverage.
Regression test
===============
`tests/bug-2866-codex-strip-no-trailing-newline.test.cjs` exercises
all four historical shapes (Shape 1 — pre-#1755 gsd-update-check;
Shape 2 — flat [[hooks]]+gsd-check-update; Shape 3 — single
[[hooks.SessionStart]] without nested .hooks; Shape 4 — correct
two-block nested) twice each: once with a trailing newline (regression
guard against the existing behavior) and once at end-of-file without a
trailing newline (the reporter's exact repro).
It also asserts:
- the helper is a no-op when no GSD reference is present, and
- Shape 4 strip does not leave an orphaned [[hooks.SessionStart]]
header behind (the same ordering invariant the inline code relied on).
The helper is loaded via `package.json` `bin` field, not a hardcoded
path — `tests/bug-2866-codex-strip-no-trailing-newline.test.cjs`
parses package.json and resolves `pkg.bin['get-shit-done-cc']` to
require the installer.
Closes#2866
* test(#2866): assert TOML structure, not raw-text substrings
CodeRabbit caught the strip assertions using `.includes()` against
raw TOML output. Added a small line-structural parseTomlShape() helper
(table headers + dotted-path key/value map, comments stripped) and
rewrote the assertions to:
- Verify no [[hooks.* table header survives the strip
- Verify no key carries a stale gsd-(update|check)-(check|update) value
- Verify history.persistence is preserved as the parsed string "save-all"
Behaviour is unchanged (the strip function under test is not modified).
The assertions now check structural shape rather than substring presence,
which catches re-shaping regressions that text matching would miss.
No new dependencies — the parser is local to the test and handles only
the small well-formed TOML these tests construct.
* refactor(#2866): replace regex hook strip with TOML AST removal
Per CR feedback on PR #2870: the regex-driven `stripStaleGsdHookBlocks`
implementation was fragile to whitespace, indentation, and key-ordering
variations the regression test never exercised. Variations the regex
silently leaked (verified before the rewrite):
- Shape 4 with an extra blank line between parent/child tables
- Shape 2/3 with `command` ordered before `event`
- Shape 3 with an extra `timeout = 5000` key — worse than a leak: the
regex matched only the command line, leaving `timeout = 5000`
orphaned outside any TOML table (invalid TOML)
- Tight whitespace `event="SessionStart"` (no spaces around `=`)
The structural rewrite uses the TOML parser already present in this file
(`getTomlTableSections` + `getTomlLineRecords` + `parseTomlValue` +
`removeContentRanges` + `collapseTomlBlankLines`):
1. Find every section whose path is `hooks` or starts with `hooks.`.
2. For each, walk the section's line records and parse `command` values
structurally — match by basename equality (`gsd-update-check.js` or
`gsd-check-update.js`), never by regex on raw bytes.
3. Detect orphaned `[[hooks.SessionStart]]` parents: empty body and a
stale child immediately follows → mark for removal.
4. Extend each removal range backward through any preceding
`# GSD Hooks` marker line (detected via line records, not text scan).
5. Remove ranges atomically and collapse resulting blank-line runs.
Legacy hook basenames are hoisted to template-literal constants so the
existing `install-hooks-copy.test.cjs` quoted-literal guard continues to
catch accidental *registration* of the inverted filename, while strip
detection (which legitimately needs both names) bypasses it.
Test coverage added: 8 new sub-tests exercising the four whitespace/
ordering variations (with and without trailing newline) plus a
`[[hooks.UserPromptSubmit]]` user-authored hook to guarantee the strip
only touches GSD-managed sections. 20/20 in the file, 5867/5867 in the
full suite.
* chore(#2868): switch canary publish from main to dev branch
Swaps the four `if:` guards in `.github/workflows/canary.yml` from
`refs/heads/main` to `refs/heads/dev` so the canary stream is owned
by the new long-lived integration branch. Adds a policy comment at
the top of the workflow documenting the branch->dist-tag mapping
(dev=@canary, main=@next/@latest, no overlap).
Closes#2868
* fix(#2868): summary block matches publish-step gate
CodeRabbit caught: the Summary step keyed off DRY_RUN only, so a
non-dry-run on main would falsely report "Published"/"Tagged" even
though all four publish steps were skipped by the new dev-only gate.
Add PUBLISH_ELIGIBLE env mirroring the publish-step `if:` expression
and a VALIDATION ONLY branch in the summary so non-dev runs report
honestly.
The Require Issue Link workflow was posting a comment and failing the
status check, but never transitioning the PR to closed. PR templates
promise auto-close behavior; PR #2863 demonstrated the gap (opened
without a Closes #N, sat open until manually closed).
Adds a `pulls.update({state: 'closed'})` call after the existing
comment, updates the comment heading to 'PR auto-closed', and tells
the author how to reopen after fixing the body.
Closes#2872
rc.7 will be the first RC in the 1.39.0 train that actually rolls in
the post-rc.5 fixes from main (rc.6 was content-identical to rc.5 — see
#2856). Notes enumerate each fix with PR/issue link, recap rc.6 / rc.5 /
rc.4, and follow the established docs/RELEASE-v1.39.0-rc.X.md format.
No SDK-version pinning advice (consistent with the rc.6 doc cleanup).
Markdownlint-clean fenced code blocks.
Closes#2859
* docs(#2856): add release notes for 1.39.0-rc.6
Documents what's actually in rc.6 (= rc.5 content + version-bump only —
release/1.39.0 was not synced with main before the bump) plus the known
SDK publish failure (@gsd-build/sdk@1.39.0-rc.6 is missing from npm with
404 PUT error). Format mirrors RELEASE-v1.39.0-rc.5.md.
Closes#2856
* docs(#2856): drop SDK refs from rc.6 notes; tag git log fence
Per maintainer + CodeRabbit review:
- Strip the 'Known issue: split publish' section, the SDK pin Note, and
the @gsd-build/sdk follow-up bullet. SDK publish failure is a known
separate issue and shouldn't block the rc.6 docs.
- Add bash language tag to the git log fence (markdownlint MD040).
* fix(#2838): SUMMARY rescue handles gitignored .planning explicitly
The pre-fix rescue used `git ls-files --modified --others --exclude-standard`
to detect uncommitted SUMMARY.md before worktree removal. When projects
gitignore .planning/, --exclude-standard filters out the very files the
rescue is meant to save, the rescue branch is skipped, and `git worktree
remove --force` permanently deletes the SUMMARY.
Replace both rescue blocks (quick.md, execute-phase.md) with a
filesystem-level find + cp rescue that bypasses gitignore entirely and
avoids the worktree↔main commit/merge cascade. cmp -s makes it idempotent.
Adds tests/bug-2838-summary-rescue-gitignored-planning.test.cjs which
extracts each rescue block, runs it against a real temp repo with a
gitignored .planning/, and asserts the SUMMARY survives worktree removal.
* test(#2838): assert rescue block exits 0 in idempotency test
CodeRabbit (Minor): the idempotency test pre-creates the destination
SUMMARY.md, so even a syntax/runtime error in the rescue block would
silently false-pass. Add an explicit r.status === 0 assertion.
* 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.
* chore(#2828): add canary release workflow (dev builds on push to main)
Publishes get-shit-done-cc@canary and @gsd-build/sdk@canary on every
push to main. Version format: {base}-canary.{N} where base strips any
pre-release suffix from package.json (1.39.0-rc.4 → 1.39.0-canary.1).
Sequential canary number is auto-detected from existing git tags so
reruns never collide. Concurrency group cancels stale in-flight canary
runs when commits land quickly.
Mirrors the structure and steps of release.yml: same checkout pins,
Node 24, npm-publish environment, build:sdk, tarball verification,
dry-run publish gate, and publish verification with sleep 10.
Closes#2828
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#2828): address CodeRabbit review findings on canary.yml
- cancel-in-progress: false — was true, allowing a newer push to cancel a
run mid-publish (after tag push but before SDK publish), leaving a partial
release state that's unrecoverable since npm versions are immutable
- Guard tag/publish/verify steps with github.ref == 'refs/heads/main' so
a manual workflow_dispatch from a feature branch (dry_run defaults false)
cannot accidentally publish unmerged code under the shared canary dist-tag
- Replace fixed sleep 10 with exponential backoff retry loop (delays: 5 10
20 30 45s); fixed sleep is flaky against normal npm CDN replication lag
and a false failure forces a new canary number since the tag already exists
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(plan-phase): expose --mvp flag in command frontmatter
Adds --mvp to argument-hint and Flags doc. Workflow handler in next commit.
* chore(#2828): remove push:main trigger from canary workflow
Submission rate to main is too high to auto-publish a canary on every
merge. Restrict the workflow to manual workflow_dispatch only.
Closes#2828
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#2836): audit-open quick SUMMARY filename + UAT terminal-status drift
Fixes two convention drifts in bin/lib/audit.cjs that produced false-positive
"open" items at every milestone close:
1. scanQuickTasks: looked for bare `SUMMARY.md`, but workflows/quick.md
mandates `${quick_id}-SUMMARY.md`. Now matches either filename so quick
tasks created via the documented workflow are recognized.
2. scanUatGaps: only treated `status: complete` as terminal, but
workflows/execute-phase.md uses `status: resolved` post-gap-closure.
Now treats both `complete` and `resolved` as terminal, with `result:
all_pass` as a fallback when status is absent.
Also reconciles workflows/help.md one-liner that referenced bare
`SUMMARY.md` so docs match the authoritative quick.md workflow.
Adds tests/bug-2836-audit-open-summary-uat-drift.test.cjs with 6
structural regression tests covering both fixes plus no-regression cases.
* refactor(#2836): hoist TERMINAL_UAT_STATUSES outside scanUatGaps loop
Address CodeRabbit nitpick: the Set was being recreated on each UAT file
iteration. Hoist to module scope so it is constructed once.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#2829): gsd-sdk resolvable in local-mode installs
Local-mode installs previously short-circuited installSdkIfNeeded() the
moment opts.isLocal was true, leaving every `gsd-sdk query …` call site
unable to resolve the binary on PATH. The published tarball ships
sdk/dist/cli.js and bin/gsd-sdk.js regardless of mode, and the shim
resolves the CLI relative to its own __dirname — so the same self-link
strategy that powers npx-cache global installs (#2775) also works for
local installs. We now run the shared self-link path whenever the dist
is present, and only fall back to a non-fatal warning + early return
when the dist is genuinely missing (preserving the #2678 contract).
* test(#2829): correct precondition comment about ~/.local/bin
Address CodeRabbit feedback — the test does not create ~/.local/bin,
so reword the inline precondition to "any HOME bin candidate remains
off-PATH" to match what the test actually sets up.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#2835): align CR-INTEGRATION tests with hyphen namespace
PR #2819 changed autonomous.md skill invocations from `gsd:code-review`
(colon) to `gsd-code-review` (hyphen). Tests still asserted the legacy
colon form against the user-installed plugin dir (which lags the repo).
Switch tests to:
- Read autonomous.md from the canonical repo WORKFLOWS_DIR (not the
plugin install location, which can be stale)
- Parse `Skill(skill="...")` invocations structurally instead of
substring matching, and assert the canonical hyphen form is present
while explicitly rejecting the legacy colon form.
Closes#2835
* test(#2835): parse Skill() invocations structurally in CR-INTEGRATION tests
Replace raw-text regex/.includes() assertions with a proper parser that
walks autonomous.md, skips escaped string contexts, and yields
[{ skill, args }] objects. The three CR-INTEGRATION tests now assert
against parsed fields and tokenized args (not substring matches),
addressing CodeRabbit feedback on PR #2843.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#2831): expand HOME in OpenCode skill/template paths
OpenCode does not shell-expand $HOME in @file references on any platform —
the literal `@$HOME/...` path is resolved relative to the config command/
dir, producing `command/$HOME/...` (file not found). The previous fix for
#2376 only guarded Windows; extend to all platforms.
Closes#2831
* test(#2831): assert behavior via exported computePathPrefix, not source grep
Addresses CodeRabbit review on PR #2842:
- Extracts pathPrefix logic into a named, test-exported computePathPrefix
helper in bin/install.js (no behavior change at the call site).
- Rewrites bug-2376 and bug-2831 regression tests to call the exported
function directly instead of regex-matching install.js source text,
per the repo's no-source-grep testing standard.
- Wraps temp-dir test setup in try/finally so cleanup runs on assertion
failures (no leaked tmp dirs).
