golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
2,655 Commits 2 Branches 0 Tags
7c5adeaad134ccebcee2b4e1f0147157aa9510ea
Commit Graph

277 Commits

Author SHA1 Message Date
Tom Boucher
3f054b4a96 Merge pull request #3505 from gsd-build/fix/3346-codex-aot-toml-event-key 
fix(install): emit event-name leaf key for Codex AoT hooks migration (#3346)
 2026-05-14 14:03:54 -04:00
Tom Boucher
925cce684a fix(codex): remove managed hooks.json SessionStart on uninstall 2026-05-14 13:30:59 -04:00
Tom Boucher
1d6284a718 fix(codex): remove duplicate skill copies and consolidate hook payloads 2026-05-14 13:17:39 -04:00
Tom Boucher
619915de96 fix(install): emit event-name leaf key for Codex AoT hooks migration (#3346) 
migrateCodexHooksMapFormat re-emitted the raw `[hooks.<X>]` path segment as
the leaf TOML key of the new `[[hooks.<EVENT>]]` block. When the legacy
table key was a `<file>:<event>:<line>:<col>` location identifier and the
real event lived in an `event = "..."` body field, the migration emitted a
header like `[[hooks."C:\\Users\\helen\\.codex\\config.toml:session_start:0:0"]]`
that Codex 0.124.0+ refuses to load — causing `npx get-shit-done-cc@latest`
to abort the Codex runtime install on Windows configs that pre-date AoT.

Mirror the flat-AoT branch in the map-format and stale-namespaced-AoT
branches: when the section body declares `event = "..."`, that name wins
as the leaf key and `event` is excluded from the re-emitted handler body.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-14 10:43:09 -04:00
Tom Boucher
ba625c0978 feat(shell-projection): add exec* dispatch and platform* file I/O seam (#3465) (#3470) 
* feat(shell-projection): add exec* dispatch and platform* file I/O seam (Phase 1, #3465)

Extends shell-command-projection.cjs with two new sections:

Subprocess dispatch:
- execGit(args, opts) → { exitCode, stdout, stderr }
- execNpm(args, opts) → same; owns shell:true on Windows (npm.cmd)
- execTool(program, args, opts) → same; ENOENT → exitCode 127, no throw
- probeTty(opts) → string | null; returns null on Windows and non-tty

Platform file I/O:
- normalizeContent(filePath, content) → { content, encoding }; pure function;
  .md → full normalizeMd pass; other → CRLF→LF + trailing newline
- platformWriteSync(filePath, content, opts) → ensureDir + normalizeContent + atomic write
- platformReadSync(filePath, opts) → null on ENOENT; throws when required:true
- platformEnsureDir(dirPath) → mkdirSync recursive, idempotent

Absorbs normalizeMd fence-tracking logic from core.cjs (no circular dep).
Existing rendering exports untouched. core.cjs compat exports unchanged until Phase 4.

Updates CONTEXT.md with Shell Command Projection Module canonical definition.
31 new behavioral tests; full suite green.

Closes #3465

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

* docs(context): record shell-projection expansion session learnings

* chore(changeset): add entry for shell-projection I/O seam (#3465)

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

* docs(adr): add ADR-0010 skill-surface budget module and ADR-0011 review default reviewers (Phase 1, #3465)

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

* docs(context): record phase 1 rebase and PR session learnings (#3465)

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

* test(shell-projection): drop substring match on stderr — assert typed shape only

The lint-no-source-grep rule prohibits substring matching on stderr (or any
test-output text). The exitCode === 127 assertion already proves the ENOENT
path; the stderr content was implementation-detail of the OS.

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

* fix(pr3470): address CodeRabbit findings and minimal-core drift

* docs(adr0010): align profile interface with phase-1 scope

* docs(adr): index newly added 0010/0011 ADR drafts in README

enh-3271 invariant requires every file in docs/adr/ to be linked from
the README. Adds entries for the three ADR drafts committed earlier
in this PR.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-13 17:12:53 -04:00
Tom Boucher
d0f916728b feat(skill-surface): install-time profiles + runtime /gsd:surface (#3408) (#3456) 
* feat(skill-deps): add requires: frontmatter to all 51 skills with cross-skill references

Mechanical migration from docs/research/data/2026-05-12-skill-audit.json.
Every skill whose body references another GSD skill now declares those
dependencies in `requires:` YAML frontmatter (flow-style array).

Notable: discuss-phase, plan-phase, and execute-phase all reference `phase`,
which confirms the latent gap in MINIMAL_SKILL_ALLOWLIST — `phase` is pulled
by the core loop but was never in the allowlist. The profile closure model
(ADR-0010 Phase 1) resolves this automatically.

Closes part of #3408.

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

* feat(skill-surface-budget): add PROFILES map, resolveProfile, loadSkillsManifest, staging, marker IO

Implements the Skill Surface Budget Module core (ADR-0010, Phase 1):

- PROFILES Object.freeze map: core (6 skills), standard (~13), full ('*')
- loadSkillsManifest: parses requires: frontmatter from commands/gsd/*.md
  into a Map<stem, string[]> without external YAML dep
- resolveProfile({modes, manifest}): computes transitive closure over the
  requires: graph; composable (modes=['core','audit'] unions closures)
- stageSkillsForProfile / stageAgentsForProfile: filesystem staging with
  same exit-cleanup machinery as the legacy stageSkillsForMode
- readActiveProfile / writeActiveProfile: .gsd-profile marker round-trip
- Back-compat shims preserved: MINIMAL_SKILL_ALLOWLIST, isMinimalMode,
  shouldInstallSkill (overloaded), stageSkillsForMode — all legacy tests pass

The phase latent bug is now resolved by closure: discuss-phase, plan-phase,
and execute-phase all require phase, so any profile including any of them
automatically includes phase via transitive closure.

Tests: 22 manifest+resolve, 9 stage, 10 marker (41 new tests, all green).
Back-compat anchor: 80/80 passing.

Closes part of #3408.

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

* feat(skill-surface-budget): add lint-skill-deps.cjs CI gate and fix 19 missed requires: entries

Two lint checks (scripts/lint-skill-deps.cjs):
  a) Frontmatter-body consistency: skill body references must appear in requires:
  b) Profile closure: every requires: dep of any profile skill must be in closure

Running the lint revealed 19 body references missed by the audit JSON (the
audit used static analysis; some bodies have conditional references). Fixed:
  complete-milestone: +audit-milestone, discuss-phase, plan-phase, execute-phase, new-milestone
  fast: +quick
  health: +thread
  map-codebase: +new-project, plan-phase
  new-milestone, new-project, review, ultraplan-phase: +plan-phase
  ship: +verify-work
  sketch, spike: +new-project
  verify-work: +execute-phase
  workstreams: +new-milestone, resume-work

Wired into package.json as lint:skill-deps and added to pretest.
8 fixture-based tests: all green.

Closes part of #3408.

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

* feat(skill-surface-budget): wire --profile= arg, profile marker write/read in bin/install.js

- Add --profile=<name> / --profile=<n1>,<n2> arg parsing (composable).
  Mutually exclusive with --minimal / --core-only (aliases for --profile=core).
  Default (no flag): full.
- Import readActiveProfile / writeActiveProfile from install-profiles.cjs.
- After writeManifest: persist active profile to .gsd-profile marker.
- gsd update path: if no --profile flag given, read existing .gsd-profile
  marker so non-full profiles are not silently re-expanded to full (ADR-0010).
- Update --help block to document --profile= with per-tier token costs.

New test: install-minimal-backcompat.test.cjs (6 tests):
  - PROFILES.core === MINIMAL_SKILL_ALLOWLIST (contract)
  - --minimal writes .gsd-profile marker "core"
  - --profile=core, --profile=standard write correct markers
  - default install writes marker "full"

Closes part of #3408.

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

* chore(changeset): add feat-3408-skill-profiles changelog fragment

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

* feat(install-profiles): derive agents from skill body refs and wire into resolveProfile

Deviation 1 of ADR-0010 phase 1b: tiered profiles (core, standard) now produce
a non-empty agents Set instead of always returning empty. resolveProfile() scans
each skill body for gsd-* agent name references (via new parseCallsAgents()),
stores them in _calls_agents_<stem> manifest entries, and unions them across the
resolved skill closure. stageAgentsForProfile() already checked resolvedProfile.agents
— it now gets real data so tiered profiles install the correct subset of agents
instead of zero.

Closes #3408 (partial — Deviation 1 only)

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

* feat(install): honor .gsd-profile marker on update, add resolveEffectiveProfile/mostRestrictiveProfile

Deviation 2 of ADR-0010 phase 1b: the marker written during installation is now
actually honored when re-running without explicit flags (e.g. gsd update). The
dead-end logging block is replaced by resolveEffectiveProfile(), which picks the
marker profile over 'full' when no explicit --profile= flag was given. The resolved
profile is piped through to all 13 stageSkillsForMode dispatch sites (now _stageSkills)
so updates install only the previously-chosen skill subset.

--minimal retains its back-compat behavior (strict 6-skill allowlist, no closure)
while writing 'core' to the marker. mostRestrictiveProfile() is exported for callers
that need to reconcile disagreeing markers across runtimes (smallest skill set wins).

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

* feat(surface): add CLUSTERS data + state IO module

Add clusters.cjs with 10 named skill groups covering all 66 skills
(verified by surface-clusters.test.cjs). Add surface.cjs with readSurface/
writeSurface atomic IO, resolveSurface, applySurface, and listSurface.
Tests: 17 passing (11 state IO + 6 cluster integrity).

Closes #3408

* docs(adr): add ADR-0011 Skill Surface Budget Module (Phase 1 accepted, Phase 2 amendment)

Records the install-time profile staging decision (Phase 1, landed) and the
runtime /gsd:surface cluster-toggle decision (Phase 2, in flight) as an
amendment. Updates the ADR README index.

Closes #3408

* docs(install-profiles): update module docblock for Phase 2 and ADR-0011

Corrects the ADR reference from 0010 to 0011, documents the three-profile
model and back-compat aliases, adds resolveEffectiveProfile precedence rule,
and notes the companion surface.cjs Phase 2 engine.

* docs(context): add Skill Surface Budget Module canonical entry

Adds the Domain terms entry for the Skill Surface Budget Module covering
both Phase 1 (install-time profiles, .gsd-profile marker) and Phase 2
(runtime /gsd:surface cluster toggles, clusters.cjs, .gsd-surface.json),
per ADR-0011 Consequences requirement.

* feat(surface): add resolveSurface and applySurface engine + tests

Tests cover: profile → surface equivalence, cluster disable/enable,
explicitAdds transitive closure, applySurface file sync (add missing,
remove superseded, preserve non-gsd files), listSurface token cost.
16 new tests passing.

* docs(readme): document --profile= flag and /gsd:surface command

Brief user-facing mention of install profiles (core/standard/full) and the
/gsd:surface slash command in the Commands table. Points to ADR-0011 for details.

* feat(surface): add /gsd:surface slash command runbook

New skill: gsd:surface — runtime profile/cluster toggle without reinstall.
Sub-commands: list, status, profile <name>, disable/enable <cluster>, reset.
Persists state to .gsd-surface.json (independent of .gsd-profile).
Description 96 chars (≤100 limit). lint:descriptions + lint:skill-deps: 0 violations.

* feat(surface): add changeset fragment for /gsd:surface runtime toggle

* feat(surface): add surface skill stem to utility cluster

surface.md is a new skill; add it to the utility cluster so the
surface-clusters.test.cjs coverage invariant stays satisfied.

* docs(adr): fix ADR references to 0011 and record Phase 2 as shipped

ADR-0010 number was already claimed by the file-operation-engine ADR; this
ADR landed as 0011-skill-surface-budget-module.md. Update inline ADR
references in clusters.cjs, surface.cjs, install-profiles.cjs, and the
Phase 2 changeset to ADR-0011. Update the ADR Status section to record
Phase 2 artifacts as shipped on this branch rather than "in progress".

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

* docs(research): port skill-surface-budget memo and audit data

ADR-0011 references docs/research/2026-05-12-skill-surface-budget.md and
docs/research/data/2026-05-12-skill-audit.json, which only existed in the
research worktree. Port both onto this branch so the ADR's References
section resolves and reviewers can read the cluster taxonomy (§3.2),
dependency topology (§3.1), and option grading (§4) that justify Phase 1
and Phase 2 decisions.

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

* fix(registration): register surface/clusters in INVENTORY, COMMANDS, and help.md

- surface.md: convert allowed-tools from inline YAML array to block style
  (was parsed as a single tool name "[Read, Write, Bash]" by test harness)
- docs/INVENTORY.md: add CLI module rows for clusters.cjs and surface.cjs;
  add Commands row for /gsd-surface; bump CLI Modules count 55→57, Commands 66→67
- docs/INVENTORY-MANIFEST.json: add entries for clusters.cjs, surface.cjs,
  and /gsd-surface (filename-based command key)
- docs/COMMANDS.md: add ### `/gsd-surface` heading in Configuration Commands
- get-shit-done/workflows/help.md: add /gsd:surface entry in Configuration section

Fixes registration failures introduced by Phase 2 of #3408.

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

* fix(surface,docs): scrub .claude leakage and escape hypothetical slash tokens

Two PR regressions introduced earlier on this branch:

1. surface.cjs JSDoc comments contained the canonical paths
   (~/.claude/commands/gsd, ~/.claude/agents) as example values, which the
   cline-install leak regex (~\/\.claude\/(?:get-shit-done|commands|agents
   |hooks)) flagged as install-time path leaks. Reworded the docblocks to
   describe runtime-resolved paths without literal ~/.claude tokens.

2. The ported research memo proposed hypothetical Option C dispatchers
   using slash syntax (/gsd:milestone, /gsd:research). The
   docs-parity-live-registry test enforces that every slash-command token
   in docs/ resolves to a real command. Rewrote the Option C sketch
   without the slash prefix and added a clarifying note that the
   dispatchers are illustrative, not shipped.

Targeted tests now pass: tests/cline-install.test.cjs and
tests/docs-parity-live-registry.test.cjs both green.

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

* test: remove raw output/source grep in lint tests

* fix: close coderabbit profile and requires issues

* test: align surface token-cost assertion wording

* fix(install): align core profile alias and defer profile marker write

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-13 12:45:16 -04:00
Tom Boucher
bf3c736029 feat(shell-projection): centralize managed hook command policy (#3450) 
* feat(shell-projection): route persistent PATH hints through projection seam

* refactor(shell-projection): unify managed hook command policy

* fix(install): guard malformed settings hooks during uninstall

* chore(changeset): add fragment for pr 3450

* fix(install): guard malformed settings hook entries
 2026-05-12 20:15:43 -04:00
Tom Boucher
16f5009270 feat(shell-projection): route PATH guidance through typed action projection (#3447) 
* feat(shell-projection): route PATH guidance through typed action projection

* chore(changeset): add PR 3447 changelog fragment
 2026-05-12 19:19:04 -04:00
Tom Boucher
7e91a861ee feat: deepen shell command projection seam (#3445) 
* feat: deepen shell command projection seam

* chore: add changeset for shell projection seam

* feat: centralize local and portable hook path projection

* docs: avoid prompt-scan false positive in installer migrations

* docs(adr): fix path-style token punctuation
 2026-05-12 19:02:18 -04:00
Tom Boucher
589318ce0a feat: route Codex hook commands through projection seam (#3444) 
* feat: route Codex hook commands through projection seam

* docs: add changeset for Codex hook projection slice

* docs: set changeset pr for #3444

* fix(codex): decode toml-escaped legacy hook path

* test(codex): use structured assertion for hook projection
 2026-05-12 18:16:36 -04:00
Tom Boucher
98cc1a84c0 fix: scope Windows hook syntax to Gemini runtime (#3438) 
* test: cover Windows hook shell drift for Claude (#3413)

* fix: scope Windows hook syntax to Gemini runtime (#3413)

* docs: add changeset for #3413

* docs: set changeset pr for #3413

* fix: route Windows hook formatting through runtime-aware projection seam

* test: cover runtime projection edge cases for Windows hooks

* fix(docs): add shell-command-projection to inventory parity

* docs: align CLI module shipped count after rebase
 2026-05-12 17:32:50 -04:00
Tom Boucher
656fb5868c Fix(hooks): resolve Windows Bash hook runner (#3397) 
* fix: resolve windows bash hook runner

* chore: add changeset for codex bash hooks

* fix: avoid hardcoded windows bash paths

* fix: skip unavailable bash hook registrations
 2026-05-11 15:32:21 -04:00
Tom Boucher
1f99085559 Fix(hooks): normalize Windows managed hook script paths (#3396) 
* fix: normalize windows managed hook script paths

* chore: add changeset for codex hook paths

* docs: hyphenate windows managed hooks changeset
 2026-05-11 15:27:44 -04:00
Tom Boucher
7a0a7f1300 Merge main into phase 5 installer migrations 2026-05-11 15:07:50 -04:00
Tom Boucher
d40c576dd0 Merge main into phase 4 installer migrations 2026-05-11 15:02:10 -04:00
Tom Boucher
6a93b14186 Merge remote-tracking branch 'origin/main' into codex/installer-migrations-phase-two 
# Conflicts:
#	bin/install.js
#	get-shit-done/bin/lib/installer-migrations.cjs
 2026-05-11 14:57:34 -04:00
Tom Boucher
041e94c43b fix: roll back installer migration state 2026-05-11 14:40:57 -04:00
Tom Boucher
c4fe891391 fix: tighten installer migration authoring guards 2026-05-11 14:36:14 -04:00
Tom Boucher
908a19cd04 fix: harden installer migration integration 2026-05-11 14:36:14 -04:00
Tom Boucher
4e40ee8b2f fix: harden installer migration paths 2026-05-11 14:29:33 -04:00
Tom Boucher
7fe75c2c3e Feat(installer): harden phase 4 migration integration 2026-05-11 10:02:02 -04:00
Tom Boucher
0b62129847 Wire installer migrations into install flow 2026-05-11 09:11:11 -04:00
Tom Boucher
3943146484 feat: migrate legacy codex hooks cleanup 2026-05-10 23:08:35 -04:00
Tom Boucher
6d33055756 feat: add installer migration framework 2026-05-10 22:23:28 -04:00
Tom Boucher
cb27c18026 fix(installer): warn on stale gsd-sdk path 2026-05-10 17:40:39 -04:00
Tom Boucher
19295f5ab6 fix(gemini): make Windows hooks and agent tools valid 2026-05-10 17:39:06 -04:00
Tom Boucher
b55848bd7d fix(codex): block unsupported execute worktrees 2026-05-10 17:38:36 -04:00
Tom Boucher
14e47acd9c fix(codex): remove legacy hooks json update hook 2026-05-10 17:38:03 -04:00
Tom Boucher
4cb5649e8a fix(gemini): drop Agent dispatcher tool (#3349) 2026-05-10 11:20:06 -04:00
Tom Boucher
e14ef535aa fix(install): allow codex hooks.state.<key> as regular table (#3285) (#3289) 
* test: codex hooks.state.<key> tables must validate as regular tables (#3285 RED)

Drive validateCodexConfigSchema with a fixture containing both [hooks.state]
and [[hooks.SessionStart]] entries. Expect the state tables to pass as regular
tables. Currently fails — validator over-classifies every hooks.* path as AoT.

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

* fix(install): treat codex hooks.state as regular table not AoT (#3285)

validateCodexConfigSchema was over-classifying every hooks.* section header
as an event-handler array-of-tables, rejecting the hooks.state.* namespace
that Codex CLI 0.130.0+ uses for per-hook trust persistence.

Fix:
1. Section-header check: carve out `hooks.state` and `hooks.state.*` from
   the AoT-required rule — only paths that are neither of those still
   require double-bracket form.
2. Parsed-object check: skip the `state` key when iterating Object.entries
   (parsed.hooks) so the "must be array" guard does not fire for the trust
   namespace object.

All other hooks.<EVENT> validation (SessionStart AoT, handler-field placement)
is unchanged.

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

* changeset: pr=3289 for #3285

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

* fix(install): codex hooks.state must be regular-table, reject AoT/scalar (CR finding 1)

- migrateCodexHooksMapFormat: exclude hooks.state and hooks.state.* from
  legacy-map detection so [hooks.state] is never promoted to [[hooks.state]] AoT
- validateCodexConfigSchema: reject [[hooks.state]] / [[hooks.state.*]] AoT at
  section level; reject Array/scalar values at parsed-object level
- Accept only plain-object shape for hooks.state and hooks.state.* (Codex
  CLI 0.130.0+ trust-persistence namespace)

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

* test: assert codex hooks.state trust entry preserved with original values (CR finding 2)

Strengthen post-install preservation assertion to verify the actual trust
entry key and its enabled/trusted_hash values survive — not just that
hooks.state is an object. Add two validator-reject tests for [[hooks.state]]
and [[hooks.state.foo]] AoT forms.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-09 07:42:22 -04:00
Tom Boucher
b08e57c92d fix(install): copy sdk/shared/model-catalog.json + resolve chain in CJS (#3288) (#3293) 
* test: reproduce model-catalog MODULE_NOT_FOUND in install layout (#3288)

Tests A/B/C exercise the install-layout regression introduced by #3230:
- A: confirms the old 3-level __dirname path fails when sdk/shared/ is absent
- B: confirms the new co-located bin/shared/ path resolves correctly (RED)
- C: confirms install() copies model-catalog.json to co-located path (RED)

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

* fix(install): copy sdk/shared/model-catalog.json + resolve chain in CJS (#3288)

Two-part fix for the CRITICAL regression introduced by #3230:

Install-side: bin/install.js now copies sdk/shared/model-catalog.json into
get-shit-done/bin/shared/model-catalog.json immediately after the main
get-shit-done/ copy step. Every runtime install (Claude Code, Codex, OpenCode,
Gemini, etc.) now includes this file in the payload.

CJS-side: model-catalog.cjs replaces the brittle single-path require with a
resolve-chain that checks candidates in order:
  1. get-shit-done/bin/shared/model-catalog.json (co-located, preferred post-install)
  2. sdk/shared/model-catalog.json (source-repo dev path, legacy fallback)
  3. GSD_MODEL_CATALOG env override (custom deployments / test harnesses)

When no candidate resolves, throws with a diagnostic listing all tried paths
(PRED.k301 — throw must include candidate paths for debuggability).

REFACTOR audit: three other __dirname traversals in bin/lib/ were inspected:
  - core.cjs:1242 (3 levels up → agents/) — safe; agents/ IS copied to targetDir
  - profile-output.cjs:547,740 (2 levels up → templates/) — safe; templates/ is
    inside get-shit-done/ and IS copied

Only model-catalog.cjs traversed outside the installed payload.

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

* changeset: pr=3293 for #3288

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

* fix(model-catalog): narrow catch to missing-file errors; clear env in test (#3288)

Two CR findings from PR #3293 review:

1. model-catalog.cjs catch block swallowed ALL errors — malformed JSON,
   permission errors, and other real failures were silently absorbed into
   the fallback chain. Now only MODULE_NOT_FOUND (with matching path in
   message) and ENOENT are treated as recoverable; any other error is
   rethrown immediately.

2. test beforeEach saved GSD_EXPLICIT_CONFIG_DIR but didn't clear it —
   a CI-set value could leak into install() and redirect the install to
   an unexpected directory, making test C non-deterministic. Added
   `delete process.env.GSD_EXPLICIT_CONFIG_DIR` to beforeEach.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-09 07:40:17 -04:00
Tom Boucher
d241227129 fix(install): Windows persistent SDK shim; replace legacy gsd-tools.cjs shim (#3211) (#3282) 
* test: reproduce Windows SDK not found after fresh npx install (#3211)

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

* fix(install): Windows persistent Path probe + npx-PATH filter on Windows (#3211)

Add getUserShellWindowsPersistentPath() — the Windows counterpart to
getUserShellPath(). Probes the user-level 'Path' registry key via
powershell.exe so the installer can verify gsd-sdk is reachable from
PowerShell/cmd.exe/Git Bash post-install, not just in the transient
npx subprocess PATH.

Wire it into installSdkIfNeeded: on Windows, use the registry-derived
persistent Path (with npx dirs stripped) as the cross-shell reachability
gate, instead of skipping the check entirely. This is the Windows sibling
of the Linux fix in #3249/#3231.

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

* changeset: pr=3282 for #3211

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

* fix(install): include Machine+User Path in Windows persistent probe (#3211)

getUserShellWindowsPersistentPath now merges Machine-level and User-level
registry Path entries (matching the effective PATH that PowerShell, cmd.exe,
and Git Bash inherit), instead of reading only User-level. Reading User-only
would produce a false warning when gsd-sdk is installed in a machine-level
bin dir (e.g. C:\Program Files\nodejs).

Addresses CodeRabbit finding on PR #3282.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-09 02:03:57 -04:00
Tom Boucher
deeb6deb67 fix(install): accept Codex TOML floats; idempotent rollback (#3245) (#3254) 
* test: reproduce extractFrontmatter LAST-block bug (#3240)

* test: reproduce state.update progress trampling and percent formula (#3242)

Two failing regression tests:
- Bug A: state.update "Last Activity" tramples curated progress.* frontmatter via readModifyWriteStateMd → syncStateFrontmatter
- Bug B: 12 declared ROADMAP phases / 6 realized / 6/6 plans done → percent: 100 instead of 50 (phase-fraction ignored)

* test: reproduce TOML float rejection and partial rollback (#3245)

Two failing regression tests:
1. parseTomlToObject rejects valid Codex TOML floats (tool_timeout_sec = 20.0)
2. Post-install validation failure leaves skills/, agents/, VERSION on disk
   despite restoring config.toml — hybrid state after abort

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

* fix(install): accept TOML floats; idempotent codex rollback (#3245)

Two fixes for the Codex install failure introduced by #2760 CR4 finding 3:

1. parseTomlValue now accepts TOML 1.0 float literals (decimals,
   exponents, underscore separators, signed). Codex CLI's serde schema
   requires f64 for tool_timeout_sec / startup_timeout_sec — the prior
   strict-integer-only check was the inverse of what Codex requires,
   causing every config with a float to trigger a fatal schema validation
   failure. Date/time separators (-/:T/Z) are still rejected.

2. restoreCodexSnapshot is extended into a unified idempotent rollback
   that reverts ALL Codex-specific mutations on failure:
   - config.toml (existing behavior)
   - skills/gsd-* directories (new)
   - agents/gsd-*.{md,toml} files (new)
   - get-shit-done/VERSION (new)
   - orphaned atomic-write temp files (new)
   Pre-install state is captured before the first Codex write so the
   rollback reflects the true pre-GSD state. Non-gsd-* user content is
   untouched. The rollback is safe to call multiple times and before any
   snapshots are captured.

Fixes #3245

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

* changeset: pr=3254 for #3245

* test: fix source-grep lint violation in bug-3242 test (#3242)

Replace content.includes() check with line-by-line parse of STATE.md body.
The lint enforces structural assertions over raw text matching.

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

* test: mark #3242 RED tests as todo pending fix (#3242)

The three failing tests are intentional regression tests for bugs in
state.cjs that will be fixed in a separate PR. Mark them { todo: true }
so they don't block CI on this branch.

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

* fix(install): tighten TOML underscore placement validation (CR finding 1)

The float regex used [\d_]* which accepts invalid forms like 1__0, 1_.0,
and 1._0. TOML 1.0 §2 requires underscores only between digits. Switch
both the integer pre-check and the full float pattern to (?:_?\d)* so
consecutive underscores, leading underscores on a segment, and trailing
underscores on a segment are all rejected before replace(/_/g,'') can
silently normalize them into valid JS numbers.

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

* fix(install): restore pre-existing gsd-* content on rollback (CR finding 2)

The snapshot only recorded names of pre-existing skills/gsd-* dirs and
agents/gsd-* files. On a failed reinstall the rollback could delete
newly-created dirs but could not restore the bytes of dirs/files that
were overwritten, leaving the user in a hybrid state (old config.toml,
new skill files).

Now snapshot the full file tree of every pre-existing gsd-* skill dir
into codexPreInstallSkillContents (Map<name, Map<relPath, Buffer>>) and
every pre-existing agent file into codexPreInstallAgentContents
(Map<filename, Buffer>). restoreCodexSnapshot() uses these maps to
wipe-and-restore overwritten entries and only removes entries that had
no pre-install state, giving a true atomic rollback guarantee.
Reads are best-effort so a partial snapshot is still better than none.

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

* fix(install): scope temp-file cleanup to installer-owned writes (CR finding 3)

_cleanTmpFiles() was deleting any *.tmp-<pid>-<n> file found under
targetDir. This is too broad: other tools in the user's Codex/home
directory may create temp files matching the same suffix pattern, and a
GSD install rollback would silently delete them.

Add __atomicWrittenTmps (a module-level Set<string>) populated by
atomicWriteFileSync for every temp path it creates. _cleanTmpFiles()
now checks __atomicWrittenTmps.has(full) before unlinking, so only temp
files this installer process actually wrote are eligible for cleanup.

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

* fix(test): remove no-op doesNotThrow wrapping try/catch (CR finding 4)

assert.doesNotThrow(() => { try { f(); } catch(_){} }) always passes
because the catch block swallows every exception before the outer
assertion can see it. This meant the rollback-idempotency guarantee was
never actually verified.

Replace with an explicit threw flag around runCodexInstall, assert that
the install did throw (validation failure is expected), and add a
post-rollback state assertion that skills/ was not created. This gives
a loud failure surface if runCodexInstall starts crashing from inside
the rollback path, matching the intent described in the test comment.

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

* fix(test): correct describe title for float-acceptance tests (CR nitpick 1)

The describe block title said 'rejects malformed input that previously
slipped through', but the test inside now asserts that TOML floats are
accepted (the #3245 inversion). This misled readers expecting every
sub-test to assert rejection. Update the title to reflect the mixed
behaviour: floats are accepted; dates and trailing-garbage are rejected.

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

* fix(test): rename test to match what the assertion actually checks (CR nitpick 2)

The test name 'post-install config retains float literal form (20.0 not
truncated to 20)' promised a string-form invariant, but the assertion
uses numeric equality (assert.strictEqual(parsed.tool_timeout_sec, 20))
which cannot distinguish 20 from 20.0 in JS. Rename to 'post-install
config round-trips tool_timeout_sec as numeric 20' so the description
matches what the test actually verifies.

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

* fix(test): replace raw text scan with state json assertion (CR nitpick 3)

The 'Last Activity updates the body field' test was reading STATE.md as
raw text, splitting on newlines, and using lines.find/startsWith to
locate the 'Last Activity:' line — the exact pattern-match-on-source
approach prohibited by the no-source-grep testing standard.

Replace with runGsdTools('state json', tmpDir) which surfaces the body-
extracted Last Activity value as fm.last_activity in its JSON output,
and assert against that structured field instead.

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

* fix(test): correct post-rollback state assertion for early-failure case

The previous assertion checked that skills/ didn't exist, but the
installer writes skills/ before the schema validator fires. Rollback
removes gsd-* dirs inside skills/, not skills/ itself. Update the
assertion to verify that no gsd-* skill dirs survive rollback, which
is the actual invariant the test name describes.

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

* changeset: document full rollback scope (CR finding 1)

Adds config.toml restoration and orphaned atomic-write temp-file
cleanup to the changeset description — the previous text only listed
skills/, agents/, and VERSION.

* fix(install): wrap post-snapshot scope in rollback handler (CR finding 2)

Any throw between the pre-install snapshot capture and the Codex config
block (skills copy, agents copy, VERSION write, manifest write, leaked-
path scan, etc.) now triggers _codexPreConfigRollback() so the caller
is never left in a partially-installed state.  Previously only the later
config.toml mutation paths had rollback wired in.

Introduces _codexPreConfigRollback (defined right after snapshot capture)
and wraps the intervening operations in a try/catch that invokes it on
error for Codex installs; non-Codex paths are unaffected.

* test: assert threw=true to prevent vacuous pass (CR finding 4)

Two tests used bare try/catch without asserting threw === true, so they
would silently pass even if runCodexInstall never threw (k060 pattern).
Each bare catch block is replaced with a threw flag and a
strictEqual(threw, true, ...) assertion.

CR findings 2+3 are both addressed in the preceding install commit:
finding 3 (restore from snapshot manifest, not current FS state) lands
alongside the rollback-wrapper change as part of the restoreCodexSnapshot
refactor.

* fix(install): reject leading zeros in TOML float integer part per TOML 1.0 (CR finding round 4)

TOML 1.0 §2 disallows leading zeros in the integer part of numeric
literals — `01`, `00`, `01.5`, `00e2`, `+01.0`, `-01.0` are all invalid.
The pre-check and float regexes in parseTomlValue used `\d(?:_?\d)*` which
accepted any digit as the leading digit.

Both regexes are tightened to `(0|[1-9](?:_?\d)*)` for the integer part:
- `0` alone is valid
- a non-zero leading digit followed by optional underscored digits is valid
- `01`, `00`, and any variant with a leading zero and further digits is rejected

The "still rejects bare time (07:32:00)" test assertion is broadened from
`/unsupported TOML value/` to `/unsupported TOML value|trailing bytes/`
because the parser now stops at `0` and the remainder `7:32:00` is rejected
as trailing bytes — the invariant (time literals are not accepted) is unchanged.

25 new regression tests cover all rejection cases and valid TOML forms.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-08 10:25:59 -04:00
Tom Boucher
c4d3fe62a5 fix(install): require persistent SDK reachability before reporting ready (#3231) (#3249) 
* test: reproduce false GSD SDK ready signals on Linux (#3231)

* fix(install): require persistent SDK reachability before reporting ready (#3231)

* changeset: pr=3249 for #3231

* fix(install): filter _npx from login-shell PATH probe (CR finding 1)

Apply filterNpxFromPath() to the getUserShellPath() result before passing
it to isGsdSdkOnPath(), mirroring the same filtering already applied to
process.env.PATH. Without this, a transient _npx entry in the login-shell
PATH can falsely satisfy the cross-shell reachability check and reintroduce
the false-ready condition this PR fixes.

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

* fix(test): unconditional legacy-shim replacement assertion (CR finding 2)

Replace readFileSync+includes source-grep check with isLegacyGsdSdkShim()
and add an else branch asserting that when sdkReady is false, a warning/error
was emitted. Previously the sdkReady===false path had no assertion at all,
allowing the test to pass without verifying any postcondition.

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

* test: replace text-grep assertions with structured ones (CR finding 2 + nitpick)

Finding 2: restructure the legacy-shim replacement assertion to branch on
isLegacyGsdSdkShim() state (a behavioral fact) rather than console output,
and add an unconditional postcondition for both branches.

Nitpick 3 (4 locations):
- lines 149-153: replace /GSD SDK ready/.test(combined) with
  isGsdSdkOnPath(filterNpxFromPath(PATH)) === false
- lines 167-169, 185-189: split filterNpxFromPath result into segments array
  and use array.includes() instead of string.includes() on the raw PATH string
- lines 375-377: replace /GSD SDK ready/.test(combined) with
  fs.existsSync(shimPath) + isGsdSdkOnPath(filterNpxFromPath(localBin))

All 8 tests pass. lint-no-source-grep: 0 violations.

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

* fix(build-hooks): per-PID staging dir eliminates concurrent-cleanup TOCTOU race

When multiple test before() hooks spawned build-hooks.js concurrently
(--test-concurrency=4), a race existed: Process A would finish all copies,
call rmdirSync('.dist-staging/') in cleanup, then Process B — still in its
copy loop — would call copyFileSync(src, '.dist-staging/hook.pid.ts') and
get ENOENT because the staging directory was gone.

On macOS/Linux, copyFileSync reports the SOURCE path in ENOENT errors when
the destination directory is missing, making the failure appear to be a
missing source file (hooks/gsd-statusline.js) rather than a missing
destination directory. This misled the diagnosis.

Fix: make STAGE_DIR per-PID ('.dist-staging-<pid>/') so each builder owns
its own staging directory. No other process touches it, eliminating all
contention on staging-dir creation and cleanup. Update .gitignore to match
the new 'hooks/.dist-staging-*/' glob.

Reproduces as: CI test matrix (macos-24, ubuntu-22, ubuntu-24) all failing
with ENOENT on hooks/gsd-statusline.js in bug-2136 before() hook. The new
test file added in this PR (bug-3231) shifts the concurrency schedule just
enough to expose the race on every CI run.

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

* test: assert on captured console output, not tautological PATH state (CR finding)

The two discarded `captureConsole()` return values in the bug-3231 test
were flagged by CodeRabbit as tautological assertions. Fix:

- Test 1 (transient _npx PATH): capture stdout/stderr and assert the
  installer does NOT emit "GSD SDK ready" (the false-positive the PR
  fixes), and that it does emit some diagnostic output instead.

- Test 3 (clean install): capture stdout/stderr and assert the installer
  DOES emit "GSD SDK ready" after successfully self-linking into a
  persistent PATH dir — confirming the positive path works correctly.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-08 09:39:33 -04:00
Tom Boucher
739b95ef80 fix(install): normalize Homebrew node@NN Cellar paths 2026-05-06 22:06:56 -04:00
Tom Boucher
69aa7ec04e fix(install): prefer stable Homebrew symlinks over versioned Cellar paths in node runner 
process.execPath on Homebrew resolves symlinks and returns the versioned
Cellar path (e.g. /usr/local/Cellar/node/25.8.1/bin/node). After
brew upgrade node, the old Cellar binary fails with dyld: Library not
loaded because shared libraries have changed SOVERSION.

- Add normalizeNodePath() helper that maps Cellar paths to stable
  Homebrew symlinks (/usr/local/bin/node or /opt/homebrew/bin/node)
- resolveNodeRunner() now calls normalizeNodePath() before quoting
- rewriteLegacyManagedNodeHookCommands() also normalizes baked Cellar
  runner paths in existing hook commands so reinstall doesn't re-bake them
- Export normalizeNodePath for testability
- Add 22 tests covering all cases (Cellar paths, stable symlinks,
  NVM, system node, Windows, null/empty, both function surfaces)

Closes #3181

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-06 22:06:56 -04:00
Tom Boucher
2bc49b0aec fix(install): wire --sdk flag into installSdkIfNeeded (#3033) 
hasSdk was parsed in bin/install.js but never passed to
installSdkIfNeeded, so `npx get-shit-done-cc@latest --sdk` silently
skipped SDK deployment via the isLocal early-return and emitted a
misleading "✓ GSD SDK ready" message.

installSdkIfNeeded now accepts opts.forceSdk. When true (set from
hasSdk at the call site in installAllRuntimes), the local-install
soft-skip is bypassed so the full shim-link path runs regardless of
install mode. When dist is also missing with forceSdk=true, the
fail-fast diagnostic fires instead of silently returning.

The #2678 soft-skip (isLocal + missing dist + no --sdk) is preserved.

Closes #3033

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-06 21:55:45 -04:00
Otavio Salvador
8ca86b5e24 fix: use #!/usr/bin/env bash in community .sh hooks for distro portability 
The three opt-in bash hooks (gsd-phase-boundary.sh, gsd-session-state.sh,
gsd-validate-commit.sh) shipped with #!/bin/bash, which fails on distros
that don't ship bash at /bin/bash (NixOS, minimal Alpine images, some
container runtimes). POSIX guarantees /bin/sh but not /bin/bash.

This is latent in the default install path because Claude Code wires the
hooks as `bash <path>` from settings.json (PATH-resolved — the script's
own shebang is read as a comment by bash). The fix matters when scripts
are run directly: tests, future installer changes, or manual debugging.

Changes:
- hooks/gsd-{phase-boundary,session-state,validate-commit}.sh: shebang
  switched to #!/usr/bin/env bash, matching the convention already used
  in scripts/*.sh.
- tests/bug-2136-sh-hook-version.test.cjs: assertion updated to expect
  the new shebang; comment updated to spell out the rationale.
- tests/bug-2979-hook-absolute-node.test.cjs: doc-comment updated — the
  prior wording cited "POSIX std PATH always has /bin" as the reason
  bare `bash` is OK. The actual reason is that bare `bash` is
  PATH-resolved, which is portable across distros that don't ship
  /bin/bash. POSIX std PATH guarantees /bin/sh, not /bin/bash.
- bin/install.js::buildHookCommand: comment block clarifying the same.
  No behavior change in this file — bare `bash` was already correct.
- .changeset/portable-bash-shebang-hooks.md: changeset entry.

Verified locally on NixOS:
- npm run build:hooks: hooks/dist/*.sh shebangs propagate correctly.
- node --test tests/bug-2136-*.cjs tests/bug-2979-*.cjs
  tests/bug-1817-*.cjs tests/bug-1834-*.cjs tests/bug-1906-*.cjs
  tests/bug-2557-*.cjs tests/bug-3017-*.cjs tests/security-scan.test.cjs
  tests/hooks-doc-parity.test.cjs: 126/126 pass.
- node scripts/run-tests.cjs (full suite): 6944 pass / 0 fail / 5 skip.
 2026-05-06 15:41:27 -04:00
Tom Boucher
dca12242b5 fix(install): skip Gemini local commands/gsd when global GSD present (#3037) (#3041) 
* fix(install): skip Gemini local commands/gsd when global GSD present (#3037)

Reporter showed that running `npx get-shit-done-cc --gemini --global`
followed by `--gemini --local` in a project creates the same 65 GSD
command files in both Gemini scopes:
  - ~/.gemini/commands/gsd/ (user scope)
  - <project>/.gemini/commands/gsd/ (workspace scope)

Gemini conflict-detects by command name across scopes and renames every
overlapping /gsd:* command to /workspace.gsd:* and /user.gsd:*, breaking
the documented /gsd:* namespace.

Fix: in bin/install.js, when handling --gemini --local, detect whether
~/.gemini/commands/gsd/ already exists with managed-shape content. If
so, skip the local copy and print a clear three-line warning explaining
the conflict avoidance. The user-scope install already provides the
same /gsd:* commands in this project; the local copy adds zero value.

Sibling fixes (test isolation):
- tests/install-minimal-all-runtimes.test.cjs: pass HOME/USERPROFILE
  through the spawned installer's env so the developer's real
  ~/.gemini/commands/gsd/ doesn't trigger the new skip path during
  test runs that want to assert the local-install populates
  commands/gsd/.
- tests/gemini-namespacing.test.cjs: the "Gemini Install (Behavioral)"
  describe block now creates an isolated tmpHome and points
  process.env.HOME at it before calling install(false, 'gemini'),
  with proper restore in afterEach.

Test:
- tests/bug-3037-gemini-duplicate-commands.test.cjs — 4 structural
  tests:
    1. global install populates HOME/.gemini/commands/gsd
    2. local install AFTER global skips the local copy
    3. local install with NO existing global still populates locally
       (no-regression)
    4. local install when HOME has .gemini/ but no GSD-managed
       commands/gsd/ still populates locally (non-GSD-Gemini-user
       no-regression)

6909/6909 full suite pass. Lints clean.

Closes #3037

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

* fix: address CR feedback on PR #3041 — narrower detection + USERPROFILE restore

CR findings:

1. **bin/install.js (Major)** — userScopeHasGsd used
   `fs.readdirSync(homeGeminiGsd).length > 0` which would skip the
   local install for any non-empty directory, including a user who
   hand-dropped a single override at ~/.gemini/commands/gsd/<thing>
   .toml without ever running --gemini --global. Narrowed the
   detection to require at least 3 canonical GSD command files
   (help.toml, progress.toml, new-project.toml) — a marker that
   ships in every GSD Gemini install (minimal mode included) and is
   structurally impossible to produce by accident.

2. **tests/bug-3037-...:59 (Minor)** — beforeEach overwrites
   process.env.USERPROFILE but afterEach only restores HOME, leaking
   the temp home into later tests on Windows or any code path that
   reads USERPROFILE. Added save/restore symmetric with HOME.

Plus added a 5th regression test covering the narrowed detection:
"local install when HOME has hand-dropped overrides UNDER commands/gsd/
(but no full GSD) still populates locally" — directly exercises the
edge case CR identified.

5/5 targeted tests pass. 6910/6910 full suite pass.

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

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-02 17:44:52 -04:00
Tom Boucher
95d2bc20f8 feat(hooks): opt-in SessionStart update banner for non-statusline users (#2795) (#3035) 
* feat(hooks): opt-in SessionStart update banner for non-statusline users (#2795)

When a user declines (or keeps a non-GSD) statusline at install time, the
installer now offers an opt-in SessionStart banner that surfaces GSD update
availability. The banner reads the existing
~/.cache/gsd/gsd-update-check.json cache (written by
gsd-check-update-worker.js) and emits a single systemMessage line only when
update_available is true:

    GSD update available: <installed> → <latest>. Run /gsd-update.

It is silent when up-to-date and rate-limits "check failed" diagnostics to
once per 24h via a sentinel file so a corrupt cache doesn't nag every
session. Removed cleanly by `npx get-shit-done-cc --uninstall` which strips
both the script and the SessionStart entry. The banner is never offered when
GSD's statusline is being installed (statusline already surfaces update
info, so re-prompting would be noise).

Implementation:
- hooks/gsd-update-banner.js — pure functions buildBannerOutput,
  shouldSuppressFailureWarning, readCache; thin main() wires them.
- bin/install.js — handleUpdateBanner() prompt, parseUpdateBannerInput(),
  buildUpdateBannerHookEntry(), buildUpdateBannerPromptText(); chained into
  installAllRuntimes() so finalize() receives both flags. updateBannerCommand
  computed alongside the other JS-hook commands; finishInstall() registers
  the SessionStart entry only when shouldInstallBanner === true and the
  hook file is present at the target.
- Hook ships in scripts/build-hooks.js HOOKS_TO_COPY, listed in
  MANAGED_HOOKS for stale-detection in gsd-check-update-worker.js, in the
  uninstall hook-removal lists in install.js, and in the
  rewriteLegacyManagedNodeHookCommands allowlist.

Tests:
- tests/feat-2795-update-banner.test.cjs — 22 tests, structural-IR
  assertions on parsed JSON envelopes (no raw-text matching). Covers
  pure-function branches (cache present/absent, parseError, rate-limit
  suppression, missing version fields), end-to-end hook invocation against
  fixture cache states, and install.js wiring (prompt text, input parsing,
  hook entry shape).
- tests/trae-install.test.cjs — updated install() return-shape assertion to
  include updateBannerCommand: null for the no-settings runtime.
- 6881/6881 tests pass.

Docs (bundled in same commit per the bundle-docs-with-code skill):
- docs/USER-GUIDE.md — new "Surface GSD Update Notifications Without GSD's
  Statusline" task section with opt-in/opt-out instructions.
- docs/FEATURES.md — REQ-HOOK-08 added; "Update Banner" subsection under
  the Hook System feature with cache flow + removal path.
- docs/INVENTORY.md — hook count 11 → 12, new row for gsd-update-banner.js.
- docs/INVENTORY-MANIFEST.json — regenerated.

Closes #2795

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

* fix(install): gate banner prompt on actual installability (CR #3035)

CodeRabbit findings on PR #3035:

- bin/install.js (Major): continueAfterStatusline gated banner prompt on
  the raw `shouldInstallStatusline` flag from handleStatusline. But
  finishInstall later silently skips the statusline write on local
  installs unless --force-statusline is set (#2248). Two consequences:
    1. Interactive local Claude/Gemini installs got neither a statusline
       nor a banner offer.
    2. Codex/Cursor/Copilot/Windsurf/Trae/Cline-only installs (where
       every result.updateBannerCommand is null) still got prompted even
       though the choice was silently ignored.
  Fix: derive willInstallStatusline = shouldInstallStatusline &&
  (isGlobal || forceStatusline), and gate the banner prompt on a
  canInstallBanner precondition computed from results[].updateBannerCommand.
  Pass the raw shouldInstallStatusline through to finalize unchanged so
  per-runtime statusline gating in finishInstall is unaffected.

- tests/feat-2795-update-banner.test.cjs (Minor): rate-limit suppression
  test parsed r1.stdout without first asserting r1.status === 0. Other
  e2e tests in this file (lines 210, 241) do this. A non-zero exit would
  surface as a cryptic SyntaxError instead of a status assertion failure.
  Fix applied verbatim.

6881/6881 tests pass.

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

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
 2026-05-02 16:33:16 -04:00
Tom Boucher
c9f5b7daac fix(#3020): probe user shell PATH at install-time, not just process.env.PATH (#3028) 
* fix(#3020): probe user shell PATH at install-time, not just process.env.PATH

The installer's "✓ GSD SDK ready" message was a false positive whenever
the install subprocess's process.env.PATH contained the gsd-sdk shim
but the user's later interactive shells did not. Three known sources
of mismatch on POSIX:

- ~/.local/bin: install subprocess inherits npm/npx-injected PATH;
  user's login shell may not add ~/.local/bin if .profile/.bashrc/
  .zshrc don't.
- nvm/fnm/volta: node version managers shim PATH per-shell, so
  `npm prefix -g` from inside the install subprocess can resolve to
  a different bin dir than the user's interactive shell sees.
- npm-prefix tooling: some installers inject extra PATH entries that
  vanish in fresh sessions.

Result reported on #3011 by @x0rk and @stefanoginella: install prints
✓, but every workflow invocation later fails with
"bash: gsd-sdk: command not found".

Fix:

- isGsdSdkOnPath(pathString?) — now accepts an explicit PATH string.
  Zero-arg form preserves existing behavior (reads process.env.PATH).
  Pure walk, no spawn. Lets callers verify against any PATH source.

- getUserShellPath() — new helper. Probes the user's login shell via
  `$SHELL -lc 'printf %s "$PATH"'` (POSIX). 2-second timeout so a
  misconfigured rc file can't hang the install. Returns null on
  Windows (cross-shell PATH probing requires a different strategy
  per Git Bash / PowerShell / cmd.exe — tracked separately) or when
  the probe fails; callers fall back to process.env.PATH in that case.

- installSdkIfNeeded() — after the existing isGsdSdkOnPath() check
  passes, also verify the shim is reachable from getUserShellPath()
  on POSIX. If install-PATH and user-shell-PATH disagree, downgrade
  to the actionable ⚠ diagnostic from PR #3014 (which has the shim
  location, shell-specific PATH-update commands, and an npx fallback
  note). Routing affected users into PR #3014's diagnostic is the
  point — not silently green-then-red.

Tests:
- bug-3020-install-shell-path-probe.test.cjs (10 tests, structural):
  - isGsdSdkOnPath accepts an explicit PATH (true/false on fixture
    PATH dirs with/without an executable shim)
  - zero-arg form returns a boolean
  - empty string PATH → false
  - getUserShellPath returns string-or-null
  - returns null on Windows
  - returns null when $SHELL unset on POSIX
  - cross-shell mismatch detection: install-PATH and user-PATH that
    differ produce different isGsdSdkOnPath results — the invariant
    the install-time check now exploits
- All assertions on structural records, not console output. Adheres
  to typed-IR / CONTRIBUTING.md "Prohibited: Raw Text Matching".

Verification:
- 10/10 pass on new regression test
- 6768/6768 pass on full suite (5 net-new tests)
- lint-no-source-grep clean

Windows cross-shell coverage (gsd-sdk.cmd resolves under PowerShell
but not Git Bash without a no-extension sibling) is tracked separately
— this PR is the POSIX-side fix and the Windows scaffolding (the
optional pathString arg on isGsdSdkOnPath) that a Windows fix can
build on.

Closes #3020

* fix(#3020): type-guard pathString, last-line PATH parse (CR)

CodeRabbit on PR #3028 (4 findings — 3 actionable + 1 nitpick):

1. .changeset/install-shell-path-probe.md (2 findings):
   - `pr: TBD` → `pr: 3028`
   - Doc said `echo $PATH` but impl uses `printf %s "$PATH"` (chosen
     to avoid shell-dependent echo behavior, e.g. interpreting `-n`).
     Aligned changeset prose with implementation.

2. bin/install.js:9176 — isGsdSdkOnPath(pathString) used
   `pathString !== undefined` to gate the explicit-PATH branch, but
   getUserShellPath() can return null and `null.split()` throws.
   Tightened to `typeof pathString === 'string'` so null / number /
   object inputs fall back to process.env.PATH. Added 2 regression
   tests covering the null and non-string cases.

3. bin/install.js:9232 — getUserShellPath trimmed entire stdout. A
   misconfigured rc file that prints a banner / motd / log line
   BEFORE the printf would pollute the result and incorrectly flip
   the cross-shell check to false. Take the LAST non-empty line
   (PATH itself is single-line) so noise can't hijack the probe.

4. Nitpick: the changeset PR placeholder — covered by (1).

Verification: 12/12 pass on regression test (10 original + 2 new
type-guard tests), 6770/6770 full suite, lint clean.

* docs(#3020): JSDoc references printf %s "$PATH", not echo $PATH (CR)

CodeRabbit caught two stale JSDoc references that still said
`$SHELL -lc 'echo $PATH'` while the implementation uses
`$SHELL -lc 'printf %s "$PATH"'`. echo is undesirable here because:

- POSIX echo's behavior with `-n` / backslash escapes varies across
  shells (bash builtin vs /bin/echo vs zsh) and can introduce
  trailing-newline pollution that the per-line trim now papers over.
- printf is portable and emits exactly the bytes given.

Synced both stale doc strings:
  - bin/install.js:9211 (getUserShellPath JSDoc)
  - tests/bug-3020-install-shell-path-probe.test.cjs:27 (header)

No behavior change — implementation already uses printf.
 2026-05-02 11:45:39 -04:00
Tom Boucher
6df9b44297 fix(#3018): codex adapter must stop and ask, not silently default decisions (#3027) 
* fix(#3018): codex adapter must stop and ask, not silently default decisions

@jon-hendry: running `\$gsd-discuss-phase 81` in Codex Default mode
proceeded toward writing CONTEXT.md / DISCUSSION-LOG.md / checkpoint
artifacts without surfacing the discussion questions to the user. The
generated Codex skill adapter explicitly told it to do that:

  Execute mode fallback:
  - When `request_user_input` is rejected (Execute mode), present a
    plain-text numbered list and pick a reasonable default.

That instruction is wrong for any workflow whose contract is to
discuss with the user (most prominently `$gsd-discuss-phase`). The
fallback now requires the agent to:

1. STOP. Present the questions as a plain-text numbered list, then
   wait for the user's reply.
2. Only proceed without a user answer when one of these is true:
   (a) invocation included --auto / --all,
   (b) user explicitly approved a default for this question, or
   (c) workflow's documented contract permits autonomous defaults.
3. Do NOT write CONTEXT.md, DISCUSSION-LOG.md, PLAN.md, or checkpoint
   files until the user has answered or one of (a)-(c) above applies.

Tests:
- bug-3018-codex-discuss-fallback.test.cjs (5 tests, structural-IR):
  parses the generated header into sections via regex,  asserts on
  the Execute-mode-fallback section's content (must contain stop/
  wait + plain-text directives, must NOT contain "pick a reasonable
  default", must name a permission path, must forbid artifact
  writing). No raw text snapshot — the assertions describe the
  behavioral invariant, so prose can be reworded without test churn.
- codex-config.test.cjs:128 still passes — section still mentions
  "Execute mode" as required.

Verification:
- 5/5 pass on new regression test
- 116/116 pass on bug-3018 + codex-config combined
- 6763/6763 pass on full suite
- lint-no-source-grep clean

Closes #3018

* test(#3018): parse fallback into typed semantic-flag record (CR)

CodeRabbit nitpick on PR #3027: the regression tests grepped the
generated header prose with regex, which is brittle and tests wording
rather than semantics. Per CONTRIBUTING.md "no-source-grep" standard.

Refactored to a structural-IR shape:

- New `parseExecuteModeFallback(section)` walks the section text once
  and returns a typed record:
    {
      ok, sectionLength,
      instructsStop,                          // STOP/HALT/WAIT directive
      presentsPlainTextQuestions,             // plain-text / numbered list
      namesPermissionPath,                    // --auto / --all / explicit approval
      forbidsWritingArtifactsBeforeAnswer,    // write-ban + named artifact class
      silentlyPicksDefaults,                  // anti-pattern guard (must be false)
    }
- Each positive invariant gets its own test asserting on the parsed
  boolean, so a failure points at the exact invariant that broke.
- A final test does a single assert.deepStrictEqual against the full
  expected contract — gives a structured diff when any flag flips.
- The artifact-write ban now requires BOTH a "do not write" intent
  AND a named artifact class (was a single broad regex), so generic
  "do not write" prose elsewhere in the section can't satisfy it.

Verification: 8/8 pass; lint-no-source-grep clean.
 2026-05-02 11:45:36 -04:00
Tom Boucher
8e25eb6546 fix(#3017): codex SessionStart hook uses absolute node, not bare 'node' (#3022) 
* fix(#3017): codex SessionStart hook uses absolute node, not bare 'node'

PR #3002 fixed #2979 for settings.json-based managed JS hooks (Claude
Code, Gemini, Antigravity) by routing through buildHookCommand() →
resolveNodeRunner(), emitting the absolute Node binary path so hooks
resolve under GUI/minimal-PATH runtimes (/usr/bin:/bin:/usr/sbin:/sbin)
where nvm/Homebrew/Volta-installed node is not on PATH.

The Codex install path bypassed both helpers — line 7935 of bin/install.js
wrote `command = "node ${path}"` directly into config.toml. So Codex
SessionStart hook still failed with exit 127 ("node: command not found")
under the same minimal-PATH conditions PR #3002 was meant to close.

Fix:
- Add buildCodexHookBlock(targetDir, { absoluteRunner, eol }) — a pure
  helper that emits the toml hook block with the absolute runner. Returns
  null when absoluteRunner is null so the caller skips registration with
  a warning instead of writing a broken bare-node hook.
- Add rewriteLegacyCodexHookBlock(content, absoluteRunner) — mirror of
  rewriteLegacyManagedNodeHookCommands for the toml surface, so
  reinstall migrates a 1.39.x bare-node config.toml to the absolute form.
  Uses basename equality (CODEX_MANAGED_HOOK_BASENAMES set) so user-
  authored bare-node hooks are left alone.
- Replace the inline string-concat at line 7935 with a call to the new
  helper, threaded with the detected line ending so CRLF files stay CRLF.
- On the codex reinstall path, call rewriteLegacyCodexHookBlock first so
  existing bare-node entries get migrated before the new entry is added.

Tests:
- bug-3017-codex-hook-absolute-node.test.cjs (9 tests, all typed-IR):
  - buildCodexHookBlock emits absolute runner, parses to expected fields
  - returns null on missing runner (caller skips)
  - integrates with resolveNodeRunner() in the live process
  - rewriteLegacyCodexHookBlock migrates managed bare-node entries
  - leaves user-authored bare-node hooks alone (basename allowlist)
  - leaves entries with absolute runner unchanged (idempotent)
  - returns content unchanged when absoluteRunner is null
- codex-config.test.cjs e2e expectation updated to match new shape:
  parsed.hooks.SessionStart[0].hooks[0].command now equals
  '"<process.execPath>" "<hookPath>"' instead of 'node <hookPath>'.

Verification:
- 9/9 pass on the new regression test
- 179/179 pass across all codex-touching test files
- 6767/6767 pass on full suite, lint-no-source-grep clean
- Adheres to typed-IR / CONTRIBUTING.md "Prohibited: Raw Text Matching":
  parseCodexHookBlock returns a typed record; assertions are on
  structured fields (runner, hookPath, type, hasMarker), not stdout regex.

Closes #3017

* test(#3017): tighten runner assertions to exact process.execPath (CR)

CodeRabbit on PR #3022 (3 findings, 2 actionable + 1 nitpick):

1. .changeset/codex-bare-node-fix.md:3 — replace `pr: TBD` with
   `pr: 3022` so changeset metadata is traceable.

2. tests/bug-3017-codex-hook-absolute-node.test.cjs:81-146 — the test
   asserted `parsed.runner !== 'node'` and `parsed.runner.includes('/node')`,
   which would false-positive on any absolute path containing '/node'
   (e.g. /Users/x/notnode/foo). Tightened to compare against the EXACT
   absolute path supplied by the caller (after stripping toml + JSON
   escape layers via a new unescapeRunner() helper). The live-process
   integration test now compares against process.execPath exactly. The
   rewriteLegacyCodexHookBlock test also uses exact-equality.

3. Nitpick (skipped): use repository's TOML parser for parsing instead
   of bespoke regex. The hand-rolled parser is small, scoped, and
   fully tested by these structural assertions; pulling in a TOML lib
   for tests would create a circular dependency on the SUT (the
   installer's own parser). Leaving as-is.

Verification: 9/9 pass on regression test, 6767/6767 full suite, lint clean.
 2026-05-02 11:45:30 -04:00
Tom Boucher
f2decefede fix(#3010): post-install message and docs use /gsd-update --reapply (#3012) 
* fix(#3010): post-install message and docs use /gsd-update --reapply

PR #2824 consolidated 86 skills into ~58, removing the standalone
/gsd-reapply-patches command and folding it into a flag on /gsd-update
(/gsd-update --reapply). The 1.39.1 hotfix (#2954) updated help.md
but missed three other surfaces that still recommended the dead form:

1. bin/install.js reportLocalPatches() — runtime emitter shown after
   every install with backed-up patches. All branches updated:
   - claude/opencode/kilo/copilot: /gsd-update --reapply
   - gemini: /gsd:update --reapply
   - codex: $gsd-update --reapply
   - cursor: gsd-update --reapply (mention the skill name)

2. get-shit-done/workflows/update.md — Step 4 prose and the
   check_local_patches block both referenced /gsd-reapply-patches.
   Replaced with /gsd-update --reapply (with backticks around the
   command per CR feedback for copy/paste UX).

3. Localized docs (en/ja-JP/ko-KR/zh-CN) — 14 files across
   ARCHITECTURE.md / COMMANDS.md / FEATURES.md / INVENTORY.md /
   USER-GUIDE.md / manual-update.md still listed the removed command.

Tests:
- bug-3010-reapply-patches-references.test.cjs (4 tests): scans
  bin/install.js's reportLocalPatches body, every workflow file, and
  every doc (excluding CHANGELOG history and help.md's deprecation
  notice) for the removed command form, and verifies each runtime
  branch emits the consolidated form via captured console output.
- tests/copilot-install.test.cjs:1081-1115 — stale assertions that
  hard-coded the removed string updated to assert /gsd-update --reapply.

Verification: 115/115 pass across both files.

Co-authored-by: Patrick Clery <patrick@patrickclery.com>
Closes #3010

* test(#3010): broaden dead-command scan + tighten runtime exact-match

CodeRabbit follow-up findings on #3012:

1. Workflow + docs scans only matched "/gsd-reapply-patches", missing
   the gemini ("/gsd:reapply-patches") and codex ("$gsd-reapply-patches")
   spellings. A regression that re-introduced either form in localized
   docs would have passed silently. Extracted a DEAD_COMMAND_PATTERNS
   array + findDeadCommands() helper used by both scans, so all three
   removed forms are checked uniformly. Match output also reports which
   spellings hit, for faster diagnosis.

2. reportLocalPatches runtime test asserted output.includes('update --reapply'),
   which is too loose — a malformed prefix like '/gsd:update --reapply' on
   the claude branch would have passed. Replaced with an exact
   {runtime → expected token} map covering all 7 branches:
     claude/opencode/kilo/copilot → /gsd-update --reapply
     gemini → /gsd:update --reapply
     codex → $gsd-update --reapply
     cursor → gsd-update --reapply
   Negative assertion also runs DEAD_COMMAND_PATTERNS against output for
   every runtime, so dead forms can't slip in regardless of branch.

Verification: 4/4 pass on bug-3010-reapply-patches-references.test.cjs.

* test(#3010): add prefix-absence guard for cursor runtime (CR follow-up)

CodeRabbit (Minor): the cursor expected token "gsd-update --reapply" is
a substring of every prefixed form ("/gsd-update --reapply" for claude/
opencode/kilo/copilot, "\$gsd-update --reapply" for codex). The positive
output.includes(expectedToken) check therefore can't distinguish correct
cursor output from a regression where the installer emits a prefixed
form for cursor — both pass the substring check.

Add an explicit prefix-absence assertion for cursor that fails if any
of /, \$, or : appears immediately before "gsd-update --reapply" in
output. The gemini form ("/gsd:update --reapply") doesn't share the
substring (gsd:update vs gsd-update) so it's already caught by the
positive includes failing on cursor's expected bare token.

Verification: 4/4 pass.

---------

Co-authored-by: Patrick Clery <patrick@patrickclery.com>
 2026-05-02 09:38:34 -04:00
Tom Boucher
a4e5cc7c24 fix(#3011): actionable SDK-not-on-PATH diagnostic with shim location and shell-specific commands (#3014) 
* fix(#3011): actionable SDK-not-on-PATH diagnostic with shim location and shell-specific commands

The previous diagnostic was a generic 'GSD SDK files are present but
gsd-sdk is not on your PATH' message with no concrete path or
shell-specific PATH-export command. Windows users reported that they
couldn't find where the shim was written and didn't know how to add
it to PATH for each shell (PowerShell vs cmd.exe vs Git Bash vs WSL
all read PATH from different sources).

New formatSdkPathDiagnostic({ shimDir, platform, runDir }) helper
returns a typed IR:
- shimLocationLine: explicit 'Shim written to: <path>'
- actionLines: platform-specific PATH-export commands
  - Windows: 3 lines (PowerShell, cmd.exe, Git Bash with backslash->/
    translation for bash compatibility)
  - POSIX: 1 line (export PATH=...)
- npxNoteLines: 'you're running via npx ... npm install -g instead'
  when runDir is under an _npx cache segment (where the shim may be
  written to a temp dir that won't persist for the user's interactive
  shell)
- isNpx, isWin32: structured booleans for assertions

Renderer in install.js just emits each line. Tests assert on the
typed IR fields directly (no source-grep, no console-output parsing).

Tests: 12 cases across 5 suites covering Windows shell flavors
(PowerShell preserves backslashes, Git Bash translates to forward),
POSIX exports, null-shimDir fallback to npm install -g advice, npx
detection on both path-separator conventions, and IR shape contract.

Closes #3011

* fix(#3011): cmd.exe guidance uses powershell -Command, not setx

CodeRabbit flagged the cmd.exe action line as a Major Windows
correctness bug:

  setx PATH "${shimDir}; %PATH%"

Two failure modes:
1. setx silently truncates the registry value above 1024 chars,
   permanently storing the truncated PATH and breaking applications
   until restored from the registry backup or fixed manually.
2. %PATH% expands to its current literal value at the moment setx
   runs, and the result is written as REG_SZ instead of REG_EXPAND_SZ.
   Lazy references like %SystemRoot% are baked in as literals, so
   future changes to those variables stop propagating.

Replace with the same SetEnvironmentVariable call already used for
the PowerShell line, invoked through `powershell -Command` so cmd.exe
users get a safe command without us recommending two different APIs.

* fix(#3011): escape shimDir for PowerShell, bash, and POSIX export

CodeRabbit (Minor): a Windows username with a single quote (e.g.
"C:\Users\O'Neil\AppData\Roaming\npm") would interpolate raw into the
suggested commands, producing unparseable shell input the user can't
fix without understanding the bug.

Each shell context needs a different escape:

- PowerShell single-quoted strings: '' is the literal-quote escape.
  Apply to both the PowerShell line and the cmd.exe line (which
  delegates to PowerShell).

- Git Bash, where the path lives inside an outer single-quoted echo:
  '\'' (close-quote, escaped-quote, reopen-quote) embeds a literal
  single quote. The slash-conversion (\\ → /) still applies first.

- POSIX export (Linux/macOS) inside double quotes: escape \, $, ",
  and backtick so the path is copied verbatim. $PATH lives outside
  the escape and still expands at paste time.

Regression test: bug-3011-sdk-path-diagnostic.test.cjs locks in the
expected escape sequence for all three shell flavors.
 2026-05-02 09:30:58 -04:00
Tom Boucher
f55069ecbf test(#2974): migrate 8 test files to typed-IR assertions (#3016) 
* test(#2974): migrate 8 test files to typed-IR assertions

Replaces raw stdout/stderr substring matching with structured-field
assertions per CONTRIBUTING.md "Prohibited: Raw Text Matching on Test
Outputs". Adds shared infrastructure for typed error emission so this
pattern is the easy path going forward.

Shared infrastructure:
- core.cjs: ERROR_REASON frozen enum + setJsonErrorMode/getJsonErrorMode
- gsd-tools.cjs: --json-errors CLI flag, parsed before subcommand dispatch
- config.cjs: typed reasons at all 7 error sites
- graphify.cjs: GRAPHIFY_REASON enum + reason/timeout_ms in execGraphify result
- bin/install.js: pure buildSdkFailFastReport() IR builder + renderer
- hooks/gsd-session-state.sh, gsd-phase-boundary.sh: emit Claude Code
  hookSpecificOutput JSON envelope with typed state_present/config_mode/
  planning_modified/file_path fields (no-op when hooks.community is off)

Test migrations (all pass, 171 tests across the 8 files):
- bug-2649-sdk-fail-fast: assert on ir.reason / ir.context / ir.fix_command
- bug-2687-config-read-warning-parity: assert.equal stderr === ''
- bug-2796-arg-parsing-regression: assert on result.json.updated/.phase
- bug-2838-summary-rescue: parse rescue footer, assert mtime invariant
- bug-2943-config-get-context-window: parse JSON, assert ERROR_REASON.CONFIG_KEY_NOT_FOUND
- graphify: assert reason === GRAPHIFY_REASON.ENOENT/TIMEOUT
- hooks-opt-in: parse hookSpecificOutput, assert typed fields
- security-scan: reclassified as source-text-is-the-product (scan label
  output and CI workflow YAML ARE the deployed contract)

Verification: lint-no-source-grep clean (0 violations), full suite
6741/6741 pass.

Closes #2974

* test(#2974): address CR feedback — typed code field, robust idempotency

Two CodeRabbit findings on #3016 addressed:

1. tests/hooks-opt-in.test.cjs:355 (Minor, inline) —
   parsed.reason.includes('Conventional Commits') was still substring
   matching after the typed-IR migration. Fixed at the source: the
   gsd-validate-commit hook now emits a typed `code` field
   ('CONVENTIONAL_COMMITS_VIOLATION', 'COMMIT_SUBJECT_TOO_LONG')
   alongside the human-readable `reason`. Test asserts strictEqual
   on the code; the prose copy is no longer part of the test contract.

2. tests/bug-2838-summary-rescue-gitignored-planning.test.cjs:224-250
   (Outside-diff) — mtimeMs alone can stay unchanged on coarse-grained
   filesystems (HFS+, FAT) when two rewrites land within the same
   timestamp tick, falsely passing the idempotency assertion.
   Replaced with a full snapshot (mtimeMs, ctimeMs, size, ino, sha256
   of contents) compared via assert.deepStrictEqual — the hash
   catches any rewrite the timestamp would miss.

Verification: 30/30 pass on the two affected files; lint-no-source-grep
clean (0 violations across 368 test files).
 2026-05-02 09:27:23 -04:00
Tom Boucher
de25400b70 fix(#2979): emit absolute node path in managed hooks for GUI/minimal-PATH runtimes (#3002) 
* fix(#2979): emit absolute node path in managed hooks for GUI/minimal-PATH runtimes

Installer-emitted hook commands started with bare 'node' which works
under interactive shells (nvm/Homebrew/Volta on PATH) but fails in
GUI-launched runtimes that start with /usr/bin:/bin:/usr/sbin:/sbin.
Every managed JS hook (gsd-check-update, gsd-statusline, gsd-context-monitor,
gsd-prompt-guard, gsd-read-guard, gsd-read-injection-scanner,
gsd-workflow-guard) failed with /bin/sh: node: command not found —
silently disabling update checks, statusline, and security guards.

Fix: new resolveNodeRunner() helper returns process.execPath (the
absolute path of the Node binary running the installer) forward-slash-
normalized and double-quoted. Used in:
  - buildHookCommand() for global installs (.js runner)
  - local-install code paths for all 7 managed JS hooks

.sh hooks keep bare 'bash' — /bin/bash is in the POSIX standard PATH
and always resolves under minimal-PATH GUI launches.

Tests: bug-2979-hook-absolute-node.test.cjs parses emitted commands
into { runner, hookPath } records and asserts:
  - resolveNodeRunner returns quoted absolute forward-slash node path
  - .js hooks emit absolute runner (default and portableHooks modes)
  - .sh hooks still emit bare 'bash'

Closes #2979

* chore(#2979): add changeset fragment for PR #3002

* chore(#2979): add changeset fragment for PR #3002

* fix(#2979): resolveNodeRunner returns null on missing execPath; rewrite legacy bare-node managed hooks (CR feedback)

CodeRabbit on PR #3002 caught two issues:

1. resolveNodeRunner fell back to bare 'node' when process.execPath was
   empty -- recreating the exact #2979 bug. Now returns null. Callers
   (buildHookCommand and the local-install code paths) check for null
   and skip registration rather than emit a broken command.

2. The original #2979 fix only updated NEWLY registered hooks. Existing
   bare-node managed hook entries from pre-#2979 installs stayed
   broken across reinstalls. New rewriteLegacyManagedNodeHookCommands
   walks settings.hooks and rewrites any managed-hook entry that starts
   with bare 'node ' to use the absolute runner. Filename allowlist
   (gsd-check-update.js, gsd-statusline.js, gsd-context-monitor.js,
   gsd-prompt-guard.js, gsd-read-guard.js, gsd-read-injection-scanner.js,
   gsd-workflow-guard.js) ensures user-authored bare-node hooks are
   left untouched.

Tests: bug-2979-hook-absolute-node.test.cjs grows by 8 cases:
- 5 for the migration walker (rewrites managed entries, leaves quoted-
  runner entries alone, leaves user-authored entries alone, leaves .sh
  entries alone, no-ops on null runner).
- 2 for resolveNodeRunner returning null on empty execPath.
- 1 for buildHookCommand returning null when execPath unavailable.

* chore(#3002): drop direct CHANGELOG.md edit; release entry now lives in .changeset/

The changeset-fragment workflow (#2975) renders fragments into
CHANGELOG.md at release time. Direct edits to [Unreleased] on
each PR caused merge conflicts on every concurrent PR. This commit
restores CHANGELOG.md to match origin/main; the release entry for
this fix is preserved in the .changeset/*.md fragment(s) on this
branch, which the release workflow consolidates.

* fix(#2979): guard hook + statusline pushes against null commands (CR follow-up)

CodeRabbit on PR #3002 found an outside-diff issue: when
resolveNodeRunner() returns null, every dependent *Command becomes
null, but the registration sites still pushed { type: 'command',
command: null } entries onto settings.hooks. The runtime's hook
schema rejects null commands and the failure surfaces as a confusing
parse error.

Fix:
- One unified warning at the top of configureSettings when ANY JS-hook
  command resolves null (operator sees the cause once instead of per-hook).
- Each of the 6 managed JS hook registration if-clauses now guards on
  the *Command variable being truthy: && updateCheckCommand,
  && contextMonitorCommand, && promptGuardCommand, && readGuardCommand,
  && readInjectionScannerCommand, && workflowGuardCommand.
- Statusline registration adds an else-if (!statuslineCommand) clause
  with its own warn before the settings.statusLine write site.

Tests: bug-2979-hook-absolute-node.test.cjs grows by 7 cases
(6 per-hook structural assertions parsing install.js for the
`fs.existsSync(<file>) && <command>` shape, plus 1 statusline
guard-precedes-write test).

* fix(#2979): defense-in-depth validateHookFields before writeSettings (CR)

CodeRabbit on PR #3002 (post-fix-up review): replace source-grep
structural tests with behavioral assertions on the settings object.

The push-site `&& <command>` guards (commit ce696c64) prevent null
commands from being pushed in the first place. As a defense-in-depth
backstop, install.js now runs validateHookFields(settings) right
before writeSettings(); validateHookFields already filters
{type:'command', command: null} entries (line 5884), so even if my
push-site guards ever regress, no null-command entries reach disk.

Tests: replaced the 7 install.js source-grep tests with 8 truly
behavioral tests:
- validateHookFields strips null-command entries for each of the 6
  managed JS hook shapes (parameterized by event + matcher)
- validateHookFields drops the entry entirely when all its hooks are
  null-command
- validateHookFields preserves agent-type hooks while stripping
  null-command sibling hooks in the same entry

These tests exercise the actual function the production code uses,
not its source representation. They survive future refactors of the
registration call sites.

* fix(#2979): tighten managed-hook migration to basename equality (CR)

CodeRabbit on PR #3002 (post-fix-up review): the previous
`trimmed.includes(name)` matcher had a false-positive vector. A
user-authored hook whose path contained a managed filename as a
substring (e.g. /home/me/scripts/wraps-gsd-check-update.js-helper.js)
would be unconditionally rewritten with the GSD runner, replacing
the user's bare `node` with our absolute path -- silently mutating
their hook configuration.

Fix: parse the command into <runner> <script-token> with the
script-token allowed to be quoted (single or double) or bareword.
Extract the path inside quotes, take the basename (handles both
forward and backslash separators on Windows), and match against
MANAGED_HOOK_FILES via Set.has() — exact equality, not substring.

Tests: bug-2979 grows by 4 cases:
- user hook with managed-filename-as-substring is NOT rewritten
- single-quoted path: rewritten correctly
- bareword path: rewritten correctly
- Windows backslash path: basename extraction works
 2026-05-02 00:40:09 -04:00
Tom Boucher
ca78b65de7 fix(#2973): /gsd-profile-user writes dev-preferences.md to skills/, not legacy commands/gsd/ (#3003) 
* fix(#2973): /gsd-profile-user writes dev-preferences.md to skills/ not legacy commands/gsd/

v1.39.0's install summary claimed the legacy ~/.claude/commands/gsd/
directory had been removed in favor of skills-only architecture, but
the cmdGenerateDevPreferences writer at profile-output.cjs:781 still
defaulted to the legacy path. Every /gsd-profile-user --refresh
deterministically re-created the legacy directory.

Missed in PR #1540's migration because dev-preferences is a
runtime-generated user artifact, not a GSD-shipped command file.

Fix:
- Writer default: ~/.claude/skills/gsd-dev-preferences/SKILL.md
- profile-user.md Display message + artifact list reference new path
- New migrateLegacyDevPreferencesToSkill(targetDir, saved) installer
  helper. Called at all 5 skills-aware install branches. Copies
  preserved legacy dev-preferences.md into skills/gsd-dev-preferences/
  SKILL.md, but ONLY if no SKILL.md already exists -- never clobbers
  user-customized skill content.

Tests: bug-2973-profile-user-skills-path.test.cjs runs the writer in
a subprocess (core.cjs:output uses fs.writeSync(1, ...) which bypasses
in-process stubbing), asserts the writer's command_path field is the
skills location, the file is on disk at that path, the legacy path is
NOT created. Tests for migration helper assert it writes when no skill
exists and skips when one does.

Closes #2973

* chore(#2973): add changeset fragment for PR #3003

* fix(#2973): rephrase comment to avoid cline-install leaked-path lint

The new comment at line 780 of profile-output.cjs literally contained
the string '~/.claude/commands/gsd/' which the cline-install
leaked-path regression test (tests/cline-install.test.cjs:175)
correctly flagged.

Cline transforms .claude/skills/ -> .cline/skills/ in installed .cjs
files but does not transform .claude/commands/. The new comment talks
about the legacy 'commands/gsd' subdirectory without the ~/.claude/
prefix, so the lint passes. The path semantics are unchanged -- the
runtime construction at line 787 still uses path.join(os.homedir(),
'.claude', 'skills', ...) which the lint regex does not match.

* test(#2973): add timeout to spawnSync to prevent CI hangs (CR feedback)

CodeRabbit on PR #3003: without a timeout, a regression that hangs the
writer or dispatcher would block CI indefinitely. Added a 30s timeout
(generous for what should complete in <1s) and an explicit signal
assertion so a timeout trip surfaces as a clear test failure with
context rather than a hung worker.

* test(#2973): add allow-test-rule annotation for legitimate product-text parsing

The new var-binding lint from #2982/#2985 caught readFileSync(...).match()
and readFileSync(...).includes() calls in this test. Both are legitimate
structural assertions against the product workflow markdown, not source-grep:

- match() extracts the path from a structured Display: "..." line and
  asserts on the typed path value (same pattern as bug-2470's installer
  scanForLeakedPaths regex test).
- includes() asserts the absence of a legacy path literal.

profile-user.md IS the shipped workflow artifact, and its Display: line
IS what the user sees. Per the existing test-rigor convention, this is
the source-text-is-the-product justification category.

Annotated with allow-test-rule citing that category.

* chore(#3003): drop direct CHANGELOG.md edit; release entry now lives in .changeset/

The changeset-fragment workflow (#2975) renders fragments into
CHANGELOG.md at release time. Direct edits to [Unreleased] on
each PR caused merge conflicts on every concurrent PR. This commit
restores CHANGELOG.md to match origin/main; the release entry for
this fix is preserved in the .changeset/*.md fragment(s) on this
branch, which the release workflow consolidates.

* fix(#2973): preserve user-owned gsd-dev-preferences skill across wipe (CR)

CodeRabbit on PR #3003 caught a real bug: copyCommandsAsClaudeSkills()
wipes ALL gsd-* skill directories at the top of every install, then
reinstalls from the package source. Since gsd-dev-preferences is
user-generated (written by /gsd-profile-user --refresh) and NOT
shipped by the npm package, the wipe deletes the user's customized
SKILL.md with nothing to restore from.

Fix: USER_OWNED_SKILLS allow-list in copyCommandsAsClaudeSkills.
Snapshot files under skills/gsd-dev-preferences/ before the wipe,
restore after. Same preserve/restore pattern as PR #1924.

Tests: bug-2973 grows by 2 cases:
- user-customized SKILL.md survives the wipe
- non-user-owned gsd-* skills are still wiped (preservation is opt-in)
 2026-05-02 00:29:45 -04:00
Tom Boucher
9f09246f3b fix(#2998): populate gsd-pristine/ from install transform pipeline so verifier has a real baseline (#3004) 
* fix(#2998): populate gsd-pristine/ from install transform pipeline so verifier has a real baseline

saveLocalPatches declared a pristineDir variable and JSDoc'd 'saves
pristine copies to gsd-pristine/' but no code ever wrote there. Effect:
/gsd-reapply-patches Step 5 verifier (#2972) silently fell back to its
over-broad heuristic ('every significant backup line') -- exactly the
silent-success-on-lost-content failure mode #2969 was designed to
prevent.

Fix: new populatePristineDir({...}) helper runs copyWithPathReplacement
(the install transform pipeline) into a tmp staging dir, then copies out
only the modified-file paths into gsd-pristine/. saveLocalPatches now
accepts a pristineCtx and calls the helper when local patches are
detected. Soft-fails on transform errors (logs warning, continues with
empty pristine -- no worse than pre-fix).

Pristine reflects the about-to-install version's content, which is the
right baseline for 'what would survive without the user's modifications'.

Tests: bug-2998-pristine-dir-populated.test.cjs asserts the helper is
exported, no-ops on empty input, writes one pristine file per source-
existing path, skips ghost paths, and produces deterministic output
(byte-identical across runs -- the property pristine_hashes depends on).

Closes #2998

* chore(#2998): add changeset fragment for PR #3004

* fix(#2998): expand pristine to all manifest install roots; clear stale pristine on populate (CR)

CodeRabbit on PR #3004 caught two issues:

1. populatePristineDir only staged packageSrc/get-shit-done/ but
   manifest.files records edits under several install roots (commands/,
   agents/, hooks/, skills/, root files like .clinerules). Modified
   paths outside get-shit-done/ were silently skipped, leaving the
   verifier with no baseline for those edits. Fixed by computing the
   set of top-level dirs from the modified set and staging each one
   that exists in source. Root-level files (no slash) bypass the
   transform pipeline and are copied directly.

2. populatePristineDir did not wipe pre-existing gsd-pristine/ before
   populating. A previous run's stale pristine could survive into the
   current run's diff baseline. Now wipe before populate AND in the
   catch path so soft-failures don't leave half-populated data on disk.

Tests: bug-2998-pristine-dir-populated.test.cjs grows by 2 cases:
- agents/ paths are staged and copied (was silently skipped pre-fix)
- mixed get-shit-done/ + agents/ in same modified list both stage
 2026-05-01 21:14:14 -04:00
Tom Boucher
e9a66da1e7 fix(#2962): write npm-style gsd-sdk shim on Windows under --sdk install (#2971) 
* fix(#2962): write npm-style gsd-sdk shim on Windows under --sdk install

trySelfLinkGsdSdk previously contained `if (process.platform === 'win32')
return null;` — a missed gap from #2775's POSIX self-link rather than an
intentional design choice. As a result, `npx get-shit-done-cc@latest
--claude --global --sdk` on Windows left `gsd-sdk` off PATH despite the
installer reporting success, and the obvious recovery (`npm i -g
@gsd-build/sdk`) lands the stale 0.1.0 publication that lacks the `query`
subcommand the agents call ~40 times.

This PR addresses the shim half. The npm-publish half (publishing
@gsd-build/sdk at parity with the get-shit-done-cc version) requires
maintainer credentials and is left for separate action.

Changes:

- bin/install.js: replace the unconditional Windows return-null with
  dispatch to a new trySelfLinkGsdSdkWindows() that:
  * resolves npm's global bin via `execFileSync('npm', ['prefix', '-g'])`
    (no shell interpolation; npm is the only PATH-resolved binary)
  * verifies write access with a probe before producing partial state
  * writes the standard npm shim triple to npm's global bin:
    - gsd-sdk.cmd (cmd.exe; CRLF line endings)
    - gsd-sdk.ps1 (PowerShell)
    - gsd-sdk    (Bash wrapper for Cygwin/MSYS/Git-Bash)
  * each shim invokes `node "<absolute path to bin/gsd-sdk.js>"` with the
    passed args, decoupling shim location from SDK location — same logical
    structure as the POSIX wrapper-via-require() fallback above
  * unlinks any stale shims before writing so prior installs don't pin
    callers to a now-absent path
  * returns the .cmd path on success (handle the existing onPath check
    looks for) or null on any failure, falling through to the existing
    "gsd-sdk is not on your PATH" warning at line 8704

- tests/bug-2962-windows-sdk-shim.test.cjs (new): 5 tests exercising
  trySelfLinkGsdSdkWindows directly with cp.execFileSync mocked to redirect
  npm prefix to a temp dir. Asserts shim contents reference the absolute
  path, .cmd uses CRLF, stale shims are replaced not appended, and null is
  returned when `npm prefix -g` fails.

- tests/no-unconditional-win32-skip.test.cjs (new): regression guard
  that fails CI if any future commit re-introduces
  `if (process.platform === 'win32') return null;` (or similar
  skip-only branches) in bin/install.js. Negative test verified by
  transiently re-introducing the bad pattern → guard fired → restored
  → passes.

Out of scope: publishing @gsd-build/sdk@<current> to npm so the natural
`npm i -g @gsd-build/sdk` recovery also lands a usable SDK. That requires
maintainer credentials and is the second half of the issue.

Closes #2962

* fix(#2962): address CodeRabbit findings — execSync for npm.cmd, behavior-based regression guard

CR finding 1 (🟠 Major): Node's child_process docs explicitly call out that
.cmd/.bat files cannot be spawned via execFile/execFileSync without a shell
("Spawning .bat and .cmd files on Windows" section). Since `npm` on Windows
is `npm.cmd`, my use of execFileSync('npm', ['prefix', '-g'], { shell: false })
would have failed on the very platform this PR is meant to fix.

Switched to cp.execSync('npm prefix -g', ...) — matching the existing
convention at line ~8718 which makes the same lookup. Args are static literals
so shell interpolation is not an injection vector.

CR finding 2 (🟠 Major): the source-grep regression test in
tests/no-unconditional-win32-skip.test.cjs violated the repo's no-source-grep
testing standard (CONTRIBUTING.md). Replaced with a behavior-based test that:

  - overrides process.platform to 'win32' via Object.defineProperty
  - mocks cp.execSync to return a temp-dir as npm prefix
  - calls trySelfLinkGsdSdk(shimSrc) and asserts it returns non-null AND
    materializes gsd-sdk.cmd on disk

The behavior guard is strictly stronger than the regex version: it would
catch any equivalent skip pattern (e.g. os.platform() === 'win32', a
typeof-based guard, etc.), not just literal `if (process.platform === 'win32')`
text. Negative-tested by re-introducing the `return null` skip → test fails
with maintainer-quoted diagnostic "trySelfLinkGsdSdk must not silently
return null on Windows; a no-op skip is a missed-parity regression"; restored
→ passes.

Test for Windows shim materialization (bug-2962-windows-sdk-shim.test.cjs)
also updated to mock cp.execSync (matching the new production code path)
instead of cp.execFileSync.

Full suite: 6480/6480 pass.

* test(#2962): make Windows shim tests self-contained per CR

Each test now invokes trySelfLinkGsdSdkWindows() itself before reading
the shim files, so they don't implicitly depend on the earlier test's
side effects. Addresses CR's order-dependence finding.

* test(#2962): structured shim parsing — eliminate substring source-grep

CR found that even after the prior refactor, three tests in the suite
still used .includes()/.startsWith() against shim file content
(cmdContent.includes(\`@node ${jsonQuoted} %*\`) etc.). Substring matching
on file text is the same anti-pattern the no-source-grep standard
forbids — even when the file is one this test wrote — because it asserts
a literal exists rather than that the structured shape is correct.

Replace with three small parsers (parseCmdShim, parsePs1Invocation,
parseBashInvocation) that split each shim into header + invocation
tokens and assert via deepEqual on structured records. The assertions
now check that the .cmd has @ECHO OFF / @SETLOCAL / @node <abs> %* in
that order with exactly 3 meaningful lines, and that the .ps1 and bash
wrappers split into the expected (call, nodeCmd, target, argToken)
tuples.

The stale-shim replacement test was hardened the same way: instead of
proving the absence of a sentinel substring, it now proves the parsed
target equals the new shimSrc and != the old path.

Verified: scripts/lint-no-source-grep.cjs reports 0 violations across
348 test files. The 6-test windows-sdk-shim + win32-skip-guard suite
all pass.

* fix(#2962): expose pure shim IR + tests assert on typed fields, not rendered text

Earlier "structured parser" approach (parseCmdShim / parsePs1Invocation /
parseBashInvocation) was still raw-text manipulation behind a function
wrapper — split('\\r\\n'), trim().split(/\\s+/), content.includes('\\r\\n').
Maintainer was right: hiding grep behind a parser is still grep.

Real fix: refactor production code to expose the structured intermediate
representation, and have tests assert on the IR fields directly.

Production:
- New buildWindowsShimTriple(shimSrc) — pure function, no fs/spawn.
  Returns { invocation: { interpreter, target }, eol: { cmd, ps1, sh },
  fileNames: { cmd, ps1, sh }, render: { cmd: () => string, ... } }.
  The IR is the contract; rendered text is an implementation detail of
  the renderers.
- trySelfLinkGsdSdkWindows now calls buildWindowsShimTriple, looks up
  filenames from triple.fileNames, and writes triple.render[kind]() to
  each target. Same observable behavior, structurally separated.
- buildWindowsShimTriple added to test-mode exports.

Tests (full rewrite — no shim file content is read at any point):
- Layer 1: pure-IR tests assert on triple.invocation.target,
  triple.eol === { cmd: '\\r\\n', ps1: '\\n', sh: '\\n' },
  triple.fileNames === { cmd: 'gsd-sdk.cmd', ... }, and the
  documented IR shape via Object.keys().sort() deepEqual.
- Layer 2: fs/spawn driver tests assert filesystem FACTS:
  - return value equals expected path
  - all three target files exist as regular non-empty files
  - rendered file byte length === Buffer.byteLength of triple.render(kind)
    output (proves the writer writes what the renderer produces, no
    mutation, no truncation, no double-write — without comparing content)
  - mtime advances on rewrite (proves stale-replace behavior)
  - returns null when npm prefix -g throws

No more split, .includes, .startsWith, .endsWith, or substring matching
anywhere in the test suite. Lint clean. 10/10 tests pass.
 2026-05-01 16:10:30 -04:00