Between `open(lockPath, O_CREAT | O_EXCL)` and `fd.writeFile(pid)` there
is an async await gap. If a second process reads the lock file during that
window it sees empty content, which parseInt returns as NaN. The previous
code returned `true` (dead) for non-finite PID values, causing the second
process to unlink the live lock and steal it — both processes then entered
readModifyWriteStateMd simultaneously, producing a lost-update TOCTOU.
Fix: return `null` (unknown) instead of `true` when the lock file
contains no parseable PID. The caller already treats `null` as "not
confirmed dead" and falls through to the normal retry + timeout path,
giving the first process time to finish writing its PID.
The CJS acquireStateLock in state.cjs never had this race because it uses
synchronous fs.openSync / fs.writeSync / fs.closeSync with no async gap.
Fixes intermittent failure of:
#1925 TOCTOU: state commands use readModifyWriteStateMd
→ state add-blocker: both concurrent calls append different blockers
(observed: macos-latest / Node 22 and windows-latest / Node 22 on main)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Export isPhaseUatPassed and supporting types/enums from SDK public surface
- Remove /m flag from frontmatter regex to prevent mid-body strip
- Implement reasonsHuman population with per-ReasonCode humanizer
- Add test for multiple UAT files in same phase
- Add test for INVALID_PHASE_NUM error path
- Add test for workstream routing
- Eliminate redundant stripMarkdownInjection call
- Update stale cycle-number comments
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 16 hand-edited command-aliases.generated.ts to register the
new query — but command-aliases is auto-generated and has a CJS
counterpart that must stay in sync. Running `npx tsx scripts/gen-command-aliases.ts`
from sdk/ syncs both surfaces; feat-3598 parity tests now pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 16 TDD cycles passed vitest but tsc --noEmit flagged TS2322:
UatItem lacked a string index signature, so the consumer at
phase-uat-passed.ts:249 (returning Record<string, unknown>[])
failed to typecheck. Index signature added; behaviour unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 16 of 16: wires isPhaseUatPassed into the SDK query
registry as 'phase.uat-passed' (alias 'phase uat-passed').
Read-only, JSON output. Handler validates the phase argument
and adapts isPhaseUatPassed to the QueryHandler signature.
Adds INVALID_PHASE_NUM to ERROR_CODE for argument validation.
Updates command-aliases.generated.ts to keep seam coverage parity.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 15 of ~15: introduces PhaseUatPassedError (extends
GSDError, classification Validation) with typed ERROR_CODE
enum. Missing-projectDir now fails fast with a typed error
rather than letting downstream code crash on a vague fs error.
Cycle 14 of ~15: a UAT file present but empty of headings,
orphans, and placeholders now emits NO_ITEMS_EXTRACTED rather
than reporting passed=false with reasons=[] (which was
indistinguishable from missing-files).
Cycle 13 of ~15: 'result: [pending]' (template placeholder
copy-pasted without being filled) was previously dropped by
the \w+ regex. Now flagged as BRACKETED_PLACEHOLDER.
De-conflicted against ORPHAN_ITEM_MISSING_RESULT so an item
with a bracketed result doesn't double-report.
Cycle 12 of ~15: heading-without-result-line was previously
silently dropped by the regex — meaning a phase with an
unfilled UAT item could falsely pass the predicate. We now
scan for orphan headings and emit ORPHAN_ITEM_MISSING_RESULT
so the operator's typo / unfilled-template is surfaced.
Cycle 11 of ~15: reuses parseVerificationFrontmatterItems from
uat.ts. Frontmatter-declared manual-verification items roll
into the same items[] roster and emit HUMAN_VERIFICATION_NEEDED
reasons so passed=false is justified, not silent.
Cycle 10 of ~15: a captured 'result:' value that lowercases to
'pass' but isn't literally 'pass' now produces a CASE_MISMATCH
reason rather than a generic NON_PASS_RESULT — so operators can
distinguish a real non-pass from a likely typo.
Cycle 9 of ~15: regex now matches both bare 'result:' and
bold-prefixed '**result:**' forms. Requester used the bold
form in the issue text; canonical template uses bare. Both
are equally valid; predicate is form-agnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 8 of ~15: adds blockquote-line stripping pass. Quoted
documentation snippets no longer pollute the item roster.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 6 of ~15: adds fenced-block stripping pass to
stripMarkdownInjection. Docs-and-examples inside code fences
no longer pollute the item roster.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 5 of ~15: introduces stripMarkdownInjection helper.
First pass strips the YAML frontmatter region so injected
'### N. item' patterns inside frontmatter literal blocks
cannot be mistaken for real UAT items.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 4 of ~15: phase dir present but no *-HUMAN-UAT.md files
now returns a typed NO_UAT_FILES reason instead of an empty
reasons array (which prior cycles used as a 'shouldn't happen'
fallback).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 3 of ~15: when resolvePhaseDir returns null, predicate
returns a typed NO_PHASE_DIR reason. Previously the empty
reasons array was indistinguishable from other failure modes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 2 of ~15: introduces REASON_CODE frozen enum and UatReason
typed shape. Non-pass items (result not literally 'pass') now
contribute a typed reason instead of being invisible to callers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle 1 of ~15: minimal end-to-end predicate. One phase dir, one
UAT file, one pass result. No injection stripping, no orphan
detection, no CLI registration yet — those follow in subsequent
commits per TDD discipline.
Also exports resolvePhaseDir from phase-list-queries.ts (was
private) so the new predicate can reuse it without duplication.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(3081): auto-trim review prompts for small-context model reviewers
Adds review.max_prompt_tokens and review.max_prompt_tokens_per_reviewer
config keys. When configured, the /gsd-review workflow deterministically
trims the assembled prompt before sending to each reviewer (drop CONTEXT
→ RESEARCH → REQUIREMENTS; head-shrink PROJECT.md; tail-truncate PLANs
proportionally; reserve disclosure-note tokens upfront). Trim metadata
is recorded in REVIEWS.md frontmatter. Reviewer is skipped with a
warning if even the minimum review set exceeds the budget.
Closes#3081
* fix(3081): register prompt-budget in SDK query registry and update inventory manifest
review.md references `gsd-sdk query prompt-budget` at three call sites, but the
command had no handler in the SDK registry — failing the registry-integration
drift-guard test on all 6 CI matrix legs. Added a native TypeScript SDK handler
(sdk/src/query/prompt-budget.ts) that ports the applyBudget logic from the CJS
module, registered it in DOMAIN_STATIC_CATALOG, and regenerated
docs/INVENTORY-MANIFEST.json to include the new cli_modules/prompt-budget.cjs entry.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(3081): bump ws to 8.20.1 and allowlist prompt-budget sibling pair
Two additional CI failures after the registry fix:
1. ws moderate CVE (GHSA-58qx-3vcg-4xpx, uninitialized memory disclosure):
The advisory covers ws >=8.0.0 <8.20.1. Both root and sdk/package.json
pinned ^8.20.0 which resolved to 8.20.0. Bumped both to 8.20.1 to clear
the npm audit drift-guard test (bug-3588-npm-audit-clean.test.cjs).
2. lint-shared-module-handsync detected the new prompt-budget.ts / prompt-budget.cjs
sibling pair without an allowlist entry. Added a cooperatingSiblings entry
to scripts/shared-module-handsync-allowlist.json with classification and
justification matching the established pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(3081): align prompt-budget skip semantics across CJS and SDK dispatch paths
Replace brittle `[ $EXIT -eq 2 ]` guards with `[ $EXIT -ne 0 ]` in all three
local-reviewer blocks (Ollama, LM Studio, llama.cpp) in workflows/review.md.
Any non-zero exit from prompt-budget now triggers a skip with a descriptive
warning — exit 2/11 prints "budget too small", any other non-zero prints
"unexpected exit code". This ensures the SDK bridge dispatch path (exit 11
via GSDError(Blocked)) triggers the same skip as the CJS path (exit 2).
The SDK handler (sdk/src/query/prompt-budget.ts) already writes both metadata
and prompt files before throwing, so no change needed there.
The Ollama block also gains the missing OLLAMA_SKIP guard so the reviewer
invocation is actually skipped (previously the block only suppressed the
OLLAMA_PROMPT_FILE update but still ran the curl invocation).
SDK integration path (hardFailed via GSDError(Blocked) → exit 11) is covered
by handler unit tests in tests/prompt-budget.test.cjs; no gsd-sdk-*.test.cjs
exercising the full bridge dispatch for this command exists yet — that gap
remains and is documented here.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix prompt-budget trim ordering and review guard follow-ups
* perf: optimize prompt-budget and dedup reviewer trim workflow
* fix(3708): drop source-grep theater tests to satisfy lint-no-source-grep
All four test files added in commit 2df566ed were pure source-grep theater:
they read .cjs / .ts / .md source files and asserted that specific string
literals were present or absent. None exercised runtime behaviour.
Deleted:
- tests/gsd-tools-memory-optimizer.test.cjs — 7 includes() on gsd-tools.cjs
- tests/prompt-budget-hotpath-optimizer.test.cjs — includes() on prompt-budget.cjs + .ts
- tests/prompt-budget-io-optimizer.test.cjs — includes() on prompt-budget.ts + gsd-tools.cjs
- tests/review-workflow-budget-dedup.test.cjs — includes() on review.md
Behavioural coverage for the prompt-budget feature already exists in
tests/prompt-budget.test.cjs and tests/prompt-budget-cli.test.cjs (also
added by this PR). No replacement tests needed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(3708): correct budget-pressure threshold and minSet accounting
Two bugs in applyBudget caused premature trimming and false hard-fails:
1. UNNEEDED_TRIM: budgetUnderPressure compared baseTokens against
effectiveBudget - NOTE_RESERVE_TOKENS, triggering trim pressure 80
tokens before the budget was actually exceeded. Fix: compare against
effectiveBudget directly; NOTE_RESERVE_TOKENS are still reserved in
contentBudget once real pressure is confirmed.
2. FALSE_HARDFAIL: minSet included NOTE_RESERVE_TOKENS unconditionally,
treating the note as mandatory even when no trim would occur and no
note would be injected. Fix: exclude NOTE_RESERVE_TOKENS from minSet;
a prompt that fits untrimmed needs no note and must not hard-fail.
Both fixes applied in CJS and TypeScript implementations. Two regression
tests added (cycles 11 and 12) that reproduce each case behaviorally.
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds four predicates derived from PR #3708 post-mortem:
- RULESET.TESTS.boundary-coverage — must test {limit-1, limit, limit+1} and near-reserve-distance inputs
- RULESET.TESTS.boundary-coverage.fixtures — required fixtures for budget/limit/quota/threshold code
- RULESET.TESTS.boundary-coverage.anti-pattern — names the trivially-large + trivially-small pairing as the failure mode
- LEARNING.prompt-budget.boundary-gap — full post-mortem of the UNNEEDED_TRIM + FALSE_HARDFAIL regressions
* fix(ci): skip install + slow lanes on Windows in main test matrix
Gates the `Run install tests` and `Run slow tests` steps in
`.github/workflows/test.yml` to `matrix.os != 'windows-latest'`.
The install lane performs `npm install -g <tarball>` 7× per invocation
of release-tarball-smoke.install.test.cjs (1× in the shared before()
hook + 1× per of the 6 test cases). On windows-latest each install
costs 60–90 s (NTFS + Defender) so the lane alone consumes ~8–9 min
on top of the ~7 min already spent on npm ci + build:sdk + unit +
integration + security — overflowing the 15-min `timeout-minutes` cap
and cancelling the job mid-install.
The dedicated install-smoke.yml workflow already excludes Windows from
its matrix (ubuntu + macOS only); the weekly windows-compat workflow
provides Windows-specific regression coverage. Linux + macOS install
and slow lanes remain on main push for parity.
Refs #3709
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(deps): bump ws 8.20.0 → 8.20.1 to clear GHSA-58qx-3vcg-4xpx
The bug-3588 `npm audit --omit=dev reports zero advisories` test
(tests/bug-3588-npm-audit-clean.test.cjs) is failing on main after a
new advisory dropped against ws@8.20.0:
GHSA-58qx-3vcg-4xpx — Uninitialized memory disclosure
ws: range >=8.0.0 <8.20.1 (CVSS 4.4, moderate, CWE-908)
Fix: `npm audit fix --omit=dev` at both root and sdk/. Lockfile-only
bump to ws@8.20.1; package.json untouched (ws is transitive).
`npm audit --omit=dev` reports `found 0 vulnerabilities` in both
workspaces after the bump.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Set NODE_OPTIONS=--max-old-space-size=6144 on the Unit coverage step so the
c8 report phase has 6 GB instead of Node's default ~4 GB heap; the Linux
Node 24 runner has 7 GB available so this leaves 1 GB headroom. Raise the
runNpm default timeout from 55 000 ms to 180 000 ms so cold-cache Windows
npm install -g runs (which take 60-90 s on NTFS + Defender) complete before
the child process is killed.
Refs #3703
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* Match `gsd-sdk query commit` in graphify auto-update hook (#3653)
The PostToolUse Bash hook only substring-matched direct shell git ops in
tool_input.command. `gsd-sdk query commit` invokes git via spawnSync,
so the literal "git commit" never appears in the Bash tool's command
string and the hook silently skipped every SDK-issued commit. Result:
.planning/graphs/ drifted stale after every phase that closed via
gsd-sdk query commit, with no error and no log.
Gate 2 now also matches `gsd-sdk query commit`. Other SDK verbs
(phase.complete, roadmap.update-plan-progress, state.begin-phase) do
not invoke git themselves and remain non-matching to avoid spurious
rebuilds per state mutation. Adds positive + negative matcher tests.
* Fix changeset frontmatter for #3658
`type: Bug Fix` rejected by scripts/changeset/parse.cjs ALLOWED_TYPES
(Keep a Changelog values: Added/Changed/Deprecated/Removed/Fixed/Security).
Switch to `type: Fixed` and add `pr: 3658` required by MISSING_PR check.
docs-lint now reports `ok_no_triggering_fragments` locally.
* fix(#3658): bound graphify SDK commit matcher
* fix(#3658): exempt release note docs lint
* chore(3686): add release-tarball lifecycle smoke to install-smoke workflow
Closes#3686.
Adds a non-interactive lifecycle smoke that runs against the installed
tarball (not the working tree). Catches the two recent release-time bug
classes that the working-tree test suite cannot see:
* #3684 — symbol mismatches between init.cjs imports and secrets.cjs
exports that landed in v1.42.3 (closed/fixed-pending-release).
* #3668 — bare `gsd-sdk` invocations in 75 of 78 workflow files with no
`command -v gsd-sdk … elif node "$GSD_TOOLS"` fallback (open).
Shape:
* `scripts/release-tarball-smoke.cjs` — pure CJS module exporting a
frozen `SMOKE` enum and a `runSmoke({ tarballPath, installPrefix,
expectedVersion, fixtureDir, lifecycleCommands })` function. CLI
`--json` mode prints `JSON.stringify(result)` and exits 0 iff
`result.code === SMOKE.OK`. Install is `--prefix <tmpdir>` so it
does not pollute global node_modules.
* `tests/release-tarball-smoke.test.cjs` — 6 tests covering happy
path, version mismatch, lifecycle command file resolution,
missing-command detection, sdk binary callability, and structural
workflow-body checks. Tests assert on the SMOKE enum directly; no
`assert.match` on rendered prose, no try/finally in test bodies,
no source-grep theater. Uses `before`/`after` to pack+install
once across the test file.
* `tests/release-tarball-smoke-workflow.test.cjs` — 7 structural
assertions on the parsed install-smoke.yml IR (workflow_call
trigger preserved, lifecycle step calls release-tarball-smoke.cjs
with --json, jq check enforces result.code === "ok", path filter
includes the new files, artifact-on-failure step present).
* `.github/workflows/install-smoke.yml` — extended (not duplicated).
New "Lifecycle smoke" step after the existing version check, on the
same matrix. Artifact upload on failure for debugging. Path filter
now triggers on changes to the new script + test.
Per CONTRIBUTING.md §"Prohibited: Raw Text Matching on Test Outputs"
this PR avoids the same anti-pattern that caused PR #3666 to be
reverted (PR #3688) — the script returns frozen enum codes, tests
assert on the enum, never on stdout strings.
Workflow-body checks in Cycle 3 are INFORMATIONAL (count returned,
not enforced) on this PR. After #3668's fix lands and the 75 missing
fallbacks are added, the lane can be tightened to enforce zero.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(3686): harden release smoke workflow and query scanner
* fix(3686): move tarball-smoke test to install suite to fix Windows ETIMEDOUT + coverage OOM
Windows (Node 22/24/26, jobs 76565215694/76565215710/76565215812):
release-tarball-smoke.test.cjs had no suite marker so run-tests.cjs
classified it as 'unit', running it on Windows PR CI. The before() hook
calls execFileSync(npm.cmd install -g ...) with a 55 s timeout; on
Windows GHA runners this npm global install consistently hits ETIMEDOUT
(~62 s observed), causing all 3 Windows lanes to fail.
Coverage (job 76565215085):
c8 ran test:coverage:unit (unit suite only) with V8 coverage tracking
active across child processes. The tarball-smoke test's before() hook
spawned npm install subprocesses while c8 held V8 coverage descriptors
open, driving the Node heap to 4 GB+ and triggering an OOM abort during
report generation (exit code 134, all 5682 tests had already passed).
Fix: rename to tests/release-tarball-smoke.install.test.cjs so
run-tests.cjs routes it to the 'install' suite. The install suite is
already skipped on PR CI by design (test.yml lines 179-181: only runs on
main push). The dedicated install-smoke.yml workflow continues to exercise
this test on its own matrix. Also update the install-smoke.yml PR path
filter and the structural wiring test assertion to match the new filename.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(test): route tarball-smoke install test through tests/helpers.cjs
Replaces direct fs.mkdtempSync and execFileSync calls in tests/release-tarball-smoke.install.test.cjs with createTempDir() and a new runNpm() helper in tests/helpers.cjs. Cleanup is now automatic via the helper. Addresses CodeRabbit Major refactor at https://github.com/gsd-build/get-shit-done/pull/3692#discussion_r3260433892.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(#3689): use find instead of chained ls for continue-here scan
The check_incomplete_work step in resume-project.md chained six bare-glob
ls arguments to discover .planning/.continue-here*.md handoff files.
Under zsh's default NOMATCH option (macOS default shell), the first
non-matching glob aborts the entire command during word-expansion,
silently dropping every pattern after it — including
.planning/.continue-here*.md, which holds the canonical pause checkpoint
for default-context handoffs. The 2>/dev/null || true guard suppresses
ls's own stderr / exit code but has no effect on the shell's pre-exec
glob-abort.
Replace the chain with two find invocations:
find .planning -maxdepth 3 -name '.continue-here*.md' -print 2>/dev/null
find . -maxdepth 1 -name '.continue-here*.md' -print 2>/dev/null
find does not use shell glob expansion and tolerates absent directories
on both bash and zsh.
Adds tests/bug-3689-resume-glob-nomatch.test.cjs covering:
- zsh -o nomatch with .planning/.continue-here-AT-1234.md present and
no spike/sketch/deliberation subdirs (the regression scenario)
- bash default, same layout
- zsh -o nomatch with no checkpoints anywhere (clean exit, no output)
- text invariant: resume-project.md no longer carries the chained-ls
pattern and does carry the find-based scan
Closes#3689
* fix(#3689): guard find commands with || true for Windows safety
Restore the trailing '|| true' guard that the original chained ls had,
per the windows-robustness.test.cjs invariant (informational bash
commands in critical workflows must not let an exit-1 from find on
exotic platforms tank the resume-project.md flow).
* fix(#3689): address CodeRabbit review feedback
- Set changeset frontmatter pr: 3693 (was 3690 placeholder); the release-
note link now points at the right PR.
- Use createTempDir / cleanup from tests/helpers.cjs instead of local
tmpdir/cleanup wrappers, per repo test standards.
* fix(#3689): update bug-3446 discovery contract to new find-based scan
bug-3446 enforced three text invariants on the chained-ls implementation
that this PR replaces with find. Update the assertions to verify the
same three discovery paths are still covered:
- .planning/.continue-here*.md at depth 1 -> find .planning -maxdepth 3
- .planning/sketches/SKETCH-NNN/.continue-here*.md at depth 3 -> same
find with -maxdepth >= 3 (assertion now reads the actual depth and
enforces a lower bound so future changes can deepen but not shallow it)
- repo-root .continue-here*.md legacy fallback -> find . -maxdepth 1
The discovery contract is preserved; only the implementation under
inspection changes.
* test(#3689): convert bug-3446 from source-grep to behavioral assertions
CodeRabbit flagged the text-regex assertions on the workflow source as a
violation of the no-source-grep testing standard. Replace them with a
behavioral integration test that:
1. Extracts the actual check_incomplete_work bash block from
resume-project.md (so the test stays in sync with whatever the
workflow does, no string match required).
2. Plants three handoff files in a temp dir covering the three
discovery surfaces bug #3446 originally filed:
- .planning/.continue-here.md (depth 1 under .planning)
- .planning/sketches/SKETCH-001/.continue-here.md (depth 3)
- ./.continue-here.md (legacy repo root)
3. Runs the snippet under bash and asserts each planted file appears
in stdout.
Same contract; now validated through runtime behavior instead of
regex-on-file.
* fix(#3689): use \\r?\\n in bash-fence regex for Windows CRLF parity
The Windows test-parity guard at tests/windows-test-parity-guard.test.cjs:106
flags any new fence-extraction regex that uses a literal \\n after the
language tag — on Windows CRLF the byte after `bash` is \\r, the regex
silently fails to match, and the extracted snippet is empty. Switch to
the canonical /```(?:bash|sh)\\r?\\n([\\s\\S]*?)```/ pattern.
* fix(#3689): harden extractCheckBlock against missing </step> and CRLF closing fence
Per CodeRabbit review: assert that the closing </step> tag is present before
slicing (otherwise slice(stepStart, -1) silently grabs the wrong block and
the test fails misleadingly), and add \r?\n to the closing fence as well so
Windows CRLF doesn't sneak a stray carriage return into the captured snippet.
Root cause: e50ad812 introduced `const { minimatch } = require('minimatch')`
in scripts/pr-template-policy.cjs, but minimatch is not listed in
package.json (neither dependencies nor devDependencies). The
.github/workflows/pr-template-format.yml workflow checks out main via
actions/checkout and runs the script directly with no `npm ci`/`npm install`
step. Because the workflow uses `pull_request_target` + plain checkout
(no `ref:`), every PR — including PRs that don't touch this file — invokes
main's broken script and the `Pull request template format` check fails
with `Cannot find module 'minimatch'`. This blocks every open PR's check.
Fix choice: replace minimatch with Node's built-in `path.matchesGlob`
(stable since Node 22, required engine is `>=22.0.0`). The only minimatch
usage was `minimatch(file, pattern, { matchBase: false, dot: true })` in
allPathsAreTooling, against simple glob patterns (`**`, `*`, `*.md`,
`requirements*.txt`, etc.) with no extglobs, brace-expansion alternation,
or negation. path.matchesGlob handles all required cases including dot
files, so no new dependency is needed and no workflow change is required.
Verified: all 25 existing tests in tests/pr-template-policy.test.cjs pass,
including the tooling carve-out, exempt-marker, and template-detection
suites. Direct script invocation with CHANGED_FILES=.github/workflows/...
produces the expected `skipped: tooling-paths` carve-out.
This unblocks every open PR's `Pull request template format` check.
* fix(3696): pr-template enforcer recognises CI/tooling carve-out
The enforcer in scripts/pr-template-policy.cjs only validated against
three typed templates and three hard-coded DEFAULT_TEMPLATE_MARKERS.
It had zero awareness of the documented CI/tooling/dep/doc-only exception
in .github/pull_request_template.md, meaning external contributors who
followed the documented escape hatch still had their PRs auto-closed.
Fix:
1. Path-scope auto-skip: if every changed file matches a tooling glob
allowlist (.github/**, scripts/**, docs/**, *.md, .changeset/**,
dependency manifests), skip enforcement and exit success with no
comment posted.
2. Explicit exemption marker: if the PR body contains
<!-- pr-template-exempt: <non-empty reason> -->, skip enforcement.
3. Workflow updated to fetch changed file paths via gh and pass them
as CHANGED_FILES env to the policy script.
4. Pull request template updated to document both mechanisms and retire
the old "delete this file" prose carve-out.
5. 14 new tests (TDD red→green); all 25 tests pass.
Closes#3696
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(3696): allow hyphenated pr-template exemption reasons
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Node 26 lanes have been causing recurring instability and CI churn.
Drop the Node 26 entry from the strategy matrix in .github/workflows/test.yml
until the Node 26 release stabilises. Also remove the companion
continue-on-error guard and its explanatory comment block, which
existed solely to prevent Node 26 failures from gating the workflow.
Files changed:
.github/workflows/test.yml — removed node-version: 26 from matrix,
removed continue-on-error: ${{ matrix.node-version == 26 }} and its
two-line comment block
Preserved: node-version: [22, 24] still run on all three OS lanes
(ubuntu-latest, macos-latest, windows-latest).
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Reverts c5657fcbfd (squash of PR #3666).
The production change was correct in intent. The new test file
tests/surface-state.test.cjs violates two CONTRIBUTING.md rules
that the project enforces specifically because they produce
passing-but-useless tests:
1. ~15 try { ... } finally { cleanup(dir); } blocks in test bodies
— CONTRIBUTING.md "Never use try/finally inside test bodies."
Approved forms are beforeEach/afterEach hooks or t.after(...).
2. 17 assert.match calls on rendered console.warn prose
— CONTRIBUTING.md "Prohibited: Raw Text Matching on Test
Outputs." Tests must assert on a typed structured surface
(frozen reason enum) instead of the rendered prose, otherwise
they pass-but-rot the moment a warning string is reworded.
The .changeset/witty-birds-gather.md fragment is also reverted so
the v1.43 changelog does not advertise a fix that is not on main.
A reworked PR addressing items 1-4 from the re-review (linked on
the closed PR thread) is welcome — the intent of the fix is right.
Refs #3666#3662
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* W002 health check: cross-reference milestones archive for STATE.md phase refs
After /gsd-complete-milestone, phase dirs move into milestones/vX.Y-phases/
and their `#### Phase N:` headings in ROADMAP.md are collapsed inside
<details> blocks. The ROADMAP heading scan misses them, so W002 fired for
every archived phase number mentioned in STATE.md's historical narrative body
("Recent", "Decisions", "Deferred Items") — leaving every project that ever
ran /gsd-complete-milestone permanently degraded with proportional W002 noise.
Union archived milestone phase directories into the validPhases set used by
the W002 check, mirroring the same milestone-archive lookup that W006
already uses (sdk/src/query/validate.ts:723-736).
Closes#3652
* Address review: use shared regex constants and mirror W002 archive fix into CJS path
CodeRabbit (PR #3655 review 1): the ad-hoc /^(\d+[A-Z]?(?:\.\d+)*)/i used to
extract phase tokens from archived phase dirs would skip project-code-prefixed
names like `CK-64-...`. Switch the new SDK block to the shared
PHASE_TOKEN_FROM_DIR_RE / MILESTONE_ARCHIVE_DIR_RE constants (defined at
sdk/src/query/validate.ts:32-33) so prefixed archives are recognised. Added a
companion regression test using `CK-`-prefixed dirs.
Codex review: the shipped CJS health command path (get-shit-done/bin/lib/verify.cjs
cmdValidateHealth, routed by validate-command-router.cjs) only unions
collectDiskPhases (active archive only) plus ROADMAP heading scan — same bug as
the SDK had. Port the all-archive scan into the CJS path via listMilestoneArchiveDirs
+ PHASE_TOKEN_FROM_DIR_RE (already declared at verify.cjs:401-402). Added a CJS
regression test covering the multi-sub-milestone (v1.3a + v1.3b) scenario from
the issue report.
Adds .changeset/lucky-lynx-wave.md.
* Address Gemini findings: shared regex + helper reuse + cross-platform path
P1 #2 — refactor the new W002 archive-scan block to reuse the existing
listMilestoneArchiveDirs helper (sdk/src/query/validate.ts:40) instead of
re-implementing readdir + filter inline. Eliminates duplication and prevents
the two call sites from drifting apart.
P2 #4 — listMilestoneArchiveDirs sorted by `a.slice(a.lastIndexOf('/') + 1)`,
which returns the full path on Windows where path.join produces backslashes.
Switch to path.basename(a) so the numeric version sort works cross-platform.
Brings the SDK helper in line with the CJS sibling at
get-shit-done/bin/lib/verify.cjs:411 which already uses path.basename.
P1 #1 / P2 #5 — the pre-existing W006/W007 archive + active phase scans
(Check 8) used an ad-hoc `^(\d+[A-Z]?(?:\.\d+)*)` regex that silently skipped
project-code-prefixed phase dirs like `CK-64-foo`, so W006 fired for a
correctly-archived phase and W007 fired for a correctly-on-disk phase. Switch
both scans to the shared PHASE_TOKEN_FROM_DIR_RE constant declared at
sdk/src/query/validate.ts:32. The W006 archive loop also now reuses
listMilestoneArchiveDirs for consistency.
P2 #6 — strengthen the CK-prefix regression test to also assert no W006
fires for `#### Phase 64: Prior shipped` (placed inside <details> so the
heading scan picks it up while the on-disk scan does not), pinning the
shared-regex behaviour in the W006 path.
* CI: switch retired /gsd-<cmd> comment syntax to canonical /gsd:<cmd>
The bug-2543 slash-namespace invariant lint scans get-shit-done/bin/lib/**
for /gsd-<cmd> patterns and fails CI when one slips into a comment. Use the
canonical /gsd:complete-milestone form in the new verify.cjs comment (and
mirror the change in the SDK + tests + changeset entry so all docstrings
referencing the milestone-completion command share one spelling).
Also: extract a small forEachArchivedPhaseToken helper in validate.ts (Gemini
P3 finding from review pass 2) so Check 4 (W002) and Check 8 (W006) share the
archive-walking loop instead of inlining it twice.
* Address rev3 review: shared regex parity, numeric sort, drop dead try/catch
Gemini P1 — Check 4's flat phases/ scan still used the ad-hoc
/^(\d+[A-Z]?(?:\.\d+)*)/ regex while the archive scan used the shared
PHASE_TOKEN_FROM_DIR_RE. Project-code-prefixed dirs (e.g. CK-65-current) on
the flat layout would have slipped past validity, so the W002 check could
still mis-classify them. Use PHASE_TOKEN_FROM_DIR_RE here too.
Gemini P3 — `[...validPhases].sort()` ordered tokens alphabetically, producing
error messages like "phases 1, 10, 19, 2, 20" instead of "1, 2, 10, 19, 20".
Switch both SDK and CJS to numeric localeCompare so the displayed list is
human-readable. Mirrored in both paths.
Grok P3 — the CJS Check 4 archive block wrapped listMilestoneArchiveDirs in
an outer try/catch even though the helper already swallows ENOENT/EACCES into
[]. The outer catch was unreachable. Removed; only the per-archive readdir
needs a catch.
Grok P2 / Gemini P1 (CJS Check 8 archive scan) — the assertion that CJS
Check 8 needs the same archive union as Check 4 was repeatedly raised across
review passes. It is incorrect: Check 8 filters ROADMAP.md through
extractCurrentMilestone() before scanning headings, which strips shipped
milestones (collapsed in <details> or not) so archived phase numbers never
reach `roadmapPhases`. Added an inline note documenting this and a positive
regression assertion in the CJS test that W006 does NOT fire for the
archived phases in the multi-sub-milestone fixture. (Skipped a parallel
W007 assertion because the active-archive fallback in
getActiveMilestoneArchiveDir is pre-existing behavior unrelated to #3652.)
* Port forEachArchivedPhaseToken helper to verify.cjs for SDK parity
Gemini rev5 P2 — the CJS Check 4 inlined the archive-walking loop while
the SDK already factored it into forEachArchivedPhaseToken(). Add a
mirror helper in verify.cjs so both seams use the same primitive,
matching the cooperating-sibling pattern documented in
scripts/shared-module-handsync-allowlist.json.
* ci: retrigger to clear unrelated TOCTOU flake
The previous CI run failed at the pre-existing #1925 concurrency test
(state add-blocker concurrent calls) on macos-24 and ubuntu-22 but
passed on ubuntu-24 — and the same test passed on the prior CI run of
this branch (commit 8a246916). The state add-blocker code path is
completely independent of the W002 archive-union changes in this PR.
* surface: default missing optional fields in readSurface, normalize writeSurface input
readSurface used to reject any .gsd-surface.json missing one of its four fields
and return null with no diagnostic, so the active surface silently degraded to
the 'full' profile. Optional array fields (disabledClusters, explicitAdds,
explicitRemoves) now default to [] when missing or wrong-typed; hard failures
(malformed JSON, non-object root, missing/non-string baseProfile) still return
null but emit a console.warn naming the file + reason.
writeSurface now normalizes its input to the full SurfaceState shape and
throws on missing baseProfile, so partial writes can no longer land on disk
and trip readSurface later. Tests extended to cover the new lenient and
warn-on-hard-fail behavior plus the writer guard.
Fixes#3662
* surface: reject whitespace-only baseProfile + migrate new tests to helpers.cjs
Applies CodeRabbit findings on PR #3666:
1. readSurface and writeSurface now reject baseProfile values that are blank
after trim() (e.g. " "), not only the empty string. Whitespace-only
strings would split-by-comma to [''] downstream and silently produce an
unresolvable profile mode. Both guards updated symmetrically; warn/error
messages reworded to "missing, non-string, or blank".
2. tests/surface-state.test.cjs now uses createTempDir + cleanup from
tests/helpers.cjs instead of local mkdtempSync + fs.rmSync, aligning with
the repo coding guideline for root-level tests. The local tmpDir() helper
delegates to createTempDir for backward-compat with the existing test
bodies. Per-test cleanup calls swapped to cleanup(dir).
3. Added regression tests:
- readSurface rejects whitespace-only baseProfile and warns
- writeSurface rejects whitespace-only baseProfile, non-string baseProfile,
and null surfaceState
Refs #3662
* surface: warn on unknown baseProfile mode names in read and write
Applies a Codex review finding on PR #3666:
readSurface and writeSurface used to accept any non-blank string as
baseProfile. A typo like {"baseProfile":"standrad"} would pass validation,
then resolveProfile() in install-profiles.cjs would silently fall back to
'full' with no diagnostic — the same silent-degradation symptom that #3662
was filed to fix, just through a different code path.
Both functions now split baseProfile by comma, validate each mode against
the registered PROFILES set ('core', 'standard', 'full'), and emit a single
[gsd] console.warn line that names the unknown modes and lists the valid
ones. The state is still parsed/written — resolveProfile() decides the
actual resolution fallback. Composed profiles where some modes are valid
and some are not warn only about the unknown subset.
Side note: the pre-existing 'round-trips composed base profile' test used
'core,audit' as a stand-in composed string. 'audit' is not a registered
profile (the three known profiles are 'core', 'standard', 'full'), so the
test was relying on the old lack of validation. Switched to 'core,standard'
to preserve the round-trip intent without producing diagnostic noise.
Refs #3662
* changeset: include blank/typo baseProfile in documented read failure cases
CodeRabbit minor finding on PR #3666 — the changeset wording only mentioned
"missing/non-string baseProfile" but the implementation also rejects blank
(including whitespace-only) baseProfile values, and warns on typo'd /
unknown profile mode names. Updated to match actual behavior.
* changeset: pr field should be PR number, not issue number
Codex review finding on PR #3666. The changeset's `pr: 3662` was the linked
issue (#3662), but the convention across other .changeset/*.md files is that
`pr:` carries the PR number. Verified by spot-checking other changesets
(2937 → pr: 3515, 3298 → pr: 3306, 3541 → pr: 3547 — all PR numbers).
Updated to `pr: 3666`. The body text still references the issue.
* surface: reject comma-only baseProfile + warn on wrong-typed optional fields
Three Gemini review findings on PR #3666 — all spirit-of-#3662 edge cases:
1. Comma-only baseProfile bypasses validation. readSurface used to accept
baseProfile: ", ," because trim() returned "," (non-empty). Downstream
resolveProfile() would split-and-filter to [] and silently fall back to
'full'. Added effectiveProfileModes() helper that splits, trims, filters
empty — both readSurface and writeSurface now reject when the result is
empty. Same silent-degradation symptom as the original bug.
2. Wrong-typed optional fields silently coerced. readSurface used to coerce
{disabledClusters: 42} to {disabledClusters: []} with no diagnostic.
Now warns via mistypedOptionalFields() before normalizeSurfaceState()
does the coercion, in both reader and writer.
3. Missing test coverage for the EACCES branch in readSurface. Added a
chmod-000 unreadable-file test, skipped on Windows and root accounts
(mode bits are ignored on those platforms).
48/48 surface tests pass. Final codex pass returned LGTM on the prior
state; these three additions strengthen the same lenient/loud contract.
Refs #3662
* fix(3678): executor must respect commit_docs:false; teach SDK skip envelope
Closes#3678
When `commit_docs: false` in `.planning/config.json`, the SDK's
`cmdCommit` correctly short-circuits and returns
`{committed: false, hash: null, reason: 'skipped_commit_docs_false'}`
without staging or committing anything. The agent prompt at
`agents/gsd-executor.md:710-720` (final_commit block) tells the executor
to call `gsd-sdk query commit "docs(...)" --files .planning/...` but says
NOTHING about how to interpret a skipped return. With no explicit
instruction, the LLM improvises raw `git add` / `git add -f` / `git commit`
to "fulfill" the per-plan commit step it was told to make, which leaks
gitignored `.planning/` artifacts into the user's git history (exactly
what the reporter observed).
Three coordinated fixes:
1. **agents/gsd-executor.md final_commit block** — adds explicit handling
text for all three SDK return envelopes (`committed:true`, `skipped:true
commit_docs`, `skipped:true gitignored`, `committed:false other reasons`).
States plainly: "Do not fall back to raw `git add` / `git commit` /
`git add -f` when the SDK returns `skipped: true`."
2. **get-shit-done/bin/lib/commands.cjs cmdCommit** — adds `skipped: true`
to both skip-path envelopes so agents see "skipped" as a first-class
success signal rather than inferring "no commit happened, I must
improvise" from absent `hash` / `committed:false`. Backward-compatible:
existing callers reading `committed` / `hash` / `reason` are unaffected.
3. **tests/bug-3678-executor-commit-docs-respect.test.cjs** — 7-test
regression covering:
- A1/A2: agent prompt mentions the skip envelope AND explicitly forbids
raw-git fallback (`source-text-is-the-product` exception)
- B1: SDK envelope carries `committed:false`, `skipped:true`, canonical
`reason: 'skipped_commit_docs_false'` (frozen enum)
- B2: git index empty after commit_docs:false skip (no `.planning/` staged)
- B3: HEAD unchanged after commit_docs:false skip
- C1/C2: structural ban on `git add -f` / `git add --force` in any agent
or workflow body (prohibition-sentence exception preserves audit prose)
Verification:
- node --test tests/bug-3678-*: 7/7 pass
- Targeted regression (10 commit/executor-adjacent files): 135/135 pass
- Full docker suite (gsd-test-summary): 11751/11740 pass / 0 fail
(the 11 added are this test plus a few collateral pickups)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(changeset): add fragment for #3678 fix (Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>)
* chore(changeset): set PR number 3679 (Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>)
* fix(3678): preserve skip-aware carve-out in executor completion checklist
The new `final_commit` prose at lines 717-741 teaches the executor to
treat `skipped:true` as success and forbids raw-git fallback, but the
downstream completion checklist still contained an unconditional
"Final metadata commit made" checkbox. An LLM executor reading an
unchecked mandatory box may attempt to satisfy it via raw `git add`,
re-introducing the exact regression this PR is meant to prevent.
Update the checklist line to carve out the intentional-skip case and
add a regression test asserting the carve-out remains present.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(3683): normalize /gsd:<cmd> → /gsd-<cmd> in command, workflow, and reference bodies
Extends #3677's agent-body normalizer to all body text staged through
copyWithPathReplacement (commands, workflows, references). The initial
isCommand guard was structurally redundant — normalizeAgentBodyForRuntime
already self-gates on shouldNormalizeHyphenNamespaceInAgentBody(runtime),
so dropping it covers all hyphen-name runtimes (Claude / Qwen / Hermes)
without affecting colon-canonical runtimes (Gemini).
Addresses the user-visible symptom in #3683: workflows like
get-shit-done/workflows/discuss-phase.md (7 colon refs) leaked /gsd:<cmd>
markers to the model context, which the model echoed at the end of
/gsd-discuss-phase runs.
Source-prose drift caught by the new cross-reference invariant test:
- commands/gsd/plan-phase.md: removed a slash-form mention of the
deleted /gsd-research-phase command (#3042)
- commands/gsd/profile-user.md: replaced a slash-form artifact
reference with a backticked bare name (the referenced item is a
skill config, not a user-callable slash command)
Tests:
- tests/bug-3683-command-colon-namespace-leak.test.cjs — runtime-form
regression for commands/gsd/*.md staging
- tests/bug-3683-command-cross-reference-invariant.test.cjs — locks
cross-reference coherence: every /gsd-X / /gsd:X reference in a
command body must resolve to commands/gsd/X.md (so a future rename
forces every cross-reference to update)
- tests/bug-3683-workflow-colon-namespace-leak.test.cjs — runtime-form
regression for workflows + references; negative test for gemini
asserting the colon form is preserved
Fixes#3683
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(changeset): remove undefined cycle reference
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(3677): normalize /gsd:<cmd> → /gsd-<cmd> in agent bodies for hyphen-name runtimes
Closes#3677
The executor agent bodies installed to `~/.claude/agents/gsd-*.md` (and
the Qwen / Hermes equivalents) still contained retired `/gsd:<cmd>`
colon-form references in their prose. Every GSD skill / agent has
registered under the canonical hyphen `name:` form since #2808, so the
colon form is unroutable — Claude Code rejects it with `Unknown command:
/gsd:execute-phase. Did you mean /gsd-execute-phase?`. Reporter measured
~28 agent files / ~96 leaked refs on a full Claude global install.
This is the agent-body surface of the same class of bug as the two
already-fixed sibling surfaces:
- #3583 (SKILL.md skill bodies) — fixed via #3629
- #3584 (user-facing runtime "Next step: /gsd:…" emissions) — fixed via #3606
The agent-body surface in `bin/install.js`'s agent install loop was
never covered: the Claude-default / Qwen / Hermes branches register
hyphen `name:` but copy bodies verbatim (Qwen/Hermes do branding-only
swaps; Claude-default falls through with no body conversion at all), so
the colon refs leak.
Fix:
1. Add a pure predicate `shouldNormalizeHyphenNamespaceInAgentBody(runtime)`
backed by an explicit allow-list `HYPHEN_NAME_AGENT_RUNTIMES =
{claude, qwen, hermes}`. Unknown / future runtimes default to false
(better to leak than to mangle).
2. Add `normalizeAgentBodyForRuntime(content, runtime, cmdNames)` that
conditionally applies the shared `transformContentToHyphen` from
`scripts/fix-slash-commands.cjs` (same transform #3629 used for
SKILL.md bodies).
3. Call `normalizeAgentBodyForRuntime(content, runtime, readGsdCommandNames())`
in the agent install loop right before `fs.writeFileSync`, so it
composes with all the existing runtime branches. For Gemini and
self-converting runtimes the predicate short-circuits, so their
convertClaudeAgentToXAgent output is not re-rewritten.
4. Export both functions from `bin/install.js` for the regression test.
Regression test (`tests/bug-3677-agent-colon-namespace-leak.test.cjs`):
24 tests across 4 groups — A (exports exist), B (predicate matrix
covering all 15 runtimes in the layout table + an unknown-runtime case),
C (normalize helper applies/skips correctly for claude/qwen/hermes/
gemini/copilot), D (sanity check of the underlying transform).
Verification:
- node --test tests/bug-3677-*: 24/24 pass
- Sibling-regression (6 slash-namespace test files): 76/76 pass
- All install-minimal-all-runtimes suites: 54/54 pass after `npm run
build:sdk` (the prior 27 fails were pre-existing — missing local
sdk/dist build, not introduced by this change)
- Full docker suite (gsd-test-summary): 11769/0 fail
(11751 baseline + 18 new = my 24 tests with some collateral pickups)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(changeset): set PR number 3680 (Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>)
* test(3677): port real-source efficacy + idempotence tests from #3681
Adds describe group E with 5 behavioral tests credited to John Turner
(johnzilla, PR #3681 — closed in favor of this PR by its author):
E0: command roster is populated and includes symptom commands
E1: every agents/gsd-*.md transforms clean — real-source efficacy
E2: idempotent — repeat transform on hyphenated input is a no-op
E3: word boundary — /gsd:plan-phase-extra is not a roster match
E4: rewrites bare gsd:<cmd> shorthand (no leading slash)
E1 is the test that would have caught the original bug — pure-function
tests can pass while the install.js wiring silently bypasses the
transform. E2 guards against double-rewrite mangling during reinstall.
29/29 tests pass (24 original + 5 ported).
Co-Authored-By: John Turner <johnzilla@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: John Turner <johnzilla@users.noreply.github.com>
Phase 2 of #3660 / ADR-3660. Routes both lifecycle verbs through the
Runtime Artifact Layout Module landed in Phase 1 (#3663):
- Add installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile)
and uninstallRuntimeArtifacts(runtime, configDir, scope) as the public
orchestrators. Both pre-prune stale gsd-* entries before staged copy;
installRuntimeArtifacts brackets the prune+copy with preserveUserArtifacts /
restoreUserArtifacts so user-owned content (e.g. gsd-dev-preferences) survives
wipe-and-replace for claude/qwen/hermes runtimes.
- Add applyRuntimeContentRewritesInPlace as the per-runtime path/branding
post-stage step (preserves byte-output equivalence with the legacy
copyCommandsAs* pipeline, including Qwen/Hermes branding rewrites).
- Add _copyStaged, _removeGsdEntries kind-aware filesystem helpers.
- Add _runLegacyInstallMigrations, _runLegacyUninstallCleanup as thin
dispatchers over existing ADR-0008 legacy migrations (Hermes flat->nested
per #2841, dev-preferences-as-skill per #2973). For Hermes, also clean up
the intermediate skills/gsd/gsd-*/ layout that pre-Phase-2 installs left
on disk.
- Delete the 9 copyCommandsAs*Skills functions (Codex / Cursor / Windsurf /
Trae / CodeBuddy / Copilot / Claude / Antigravity / Augment) and the
_copyCommandsAsSkillsViaConverter helper. All test entry points migrated
to call installRuntimeArtifacts directly through the unified seam.
- Collapse the 9-branch uninstall ladder to one uninstallRuntimeArtifacts
call plus preserved non-layout side-effects (Codex TOML, Copilot
instructions, hooks).
- Unify install dispatcher: a single _isSkillsRuntime gate routes all 11
skills runtimes through installRuntimeArtifacts for both full and core/
minimal profiles. Removes 11 per-runtime if-else branches (3 minimal-mode
shim branches + 8 dead after-the-gate branches).
Net delta on bin/install.js: 11,495 -> 11,174 (-321 LOC).
New tests:
- tests/install-uninstall-layout-loop.test.cjs (34 tests) - per-runtime
fixture assertions on install/uninstall/legacy-migration ordering.
- tests/install-hermes-regressions.test.cjs (6 tests) - covers the six
defects surfaced by iterative review: Hermes upgrade leaves stale dirs,
--hermes --profile=core fall-through, --qwen --profile=core fall-through,
minimal-mode dev-preferences migration skipped (Hermes/Qwen/Claude-global),
and ordering bug in _runLegacyInstallMigrations.
Existing tests (10,038 prior + 40 new) all green: 10,078/10,078 pass.
Refs #3664
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After PR #3649 (merge 40a442b2), test (windows-latest, 24) failed at npm ci with zero stdout/stderr (16s opaque exit). Identical code passed on the PR. Root cause: the npm ci and npm run build:sdk steps in .github/workflows/test.yml (jobs test and coverage) lacked an effective shell — neither step-level nor via defaults.run.shell. On windows-latest, Actions defaults to pwsh; npm.cmd → node.exe → npm-cli.js child-process chain under pwsh can swallow stderr.
Pin shell: bash on the 4 outlier steps. Add tests/workflow-shell-pinning.test.cjs that scans every workflow file referencing windows-latest, computes effective shell with proper workflow- and job-level defaults.run.shell inheritance, and fails CI if any run: npm|npx … step lacks an effective shell.
Closes#3672
