fix: resolve 9 CLI and config bugs (#734, #737, #739, #740, #741, #759, #786, #664, #670)#813
Closed
Tibsfox wants to merge 8 commits intogsd-build:mainfrom
Closed
fix: resolve 9 CLI and config bugs (#734, #737, #739, #740, #741, #759, #786, #664, #670)#813Tibsfox wants to merge 8 commits intogsd-build:mainfrom
Tibsfox wants to merge 8 commits intogsd-build:mainfrom
Conversation
…e, task_count (gsd-build#734) cmdPhasePlanIndex had 3 mismatches with the canonical XML plan format defined in templates/phase-prompt.md: - files_modified: looked up fm['files-modified'] (hyphen) but plans use files_modified (underscore). Now checks underscore first, hyphen fallback. - objective: read from YAML frontmatter but plans put it in <objective> XML tag. Now extracts first line from the tag, falls back to frontmatter. - task_count: matched ## Task N markdown headings but plans use <task> XML tags. Now counts XML tags first, markdown fallback. All three fixes preserve backward compat with legacy markdown-style plans. Co-authored-by: Claude Opus 4.6 <[email protected]>
loadConfig() didn't include nyquist_validation in its return object, so cmdInitPlanPhase always set nyquist_validation_enabled to undefined. The plan-phase workflow could never detect whether Nyquist validation was enabled or disabled via config. Co-authored-by: Ethan Hurst <[email protected]>
stateReplaceField properly escaped fieldName before building the regex, but stateExtractField did not. Field names containing regex metacharacters could cause incorrect matches or regex errors. Co-authored-by: Ethan Hurst <[email protected]>
…sd-build#737) DEBUGGER_MODEL and CHECKER_MODEL used uppercase bash convention but the Task() calls referenced {debugger_model} and {integration_checker_model} (lowercase). The mismatch caused Claude to skip substitution and fall back to the parent session model, ignoring the configured GSD profile. Co-authored-by: Ethan Hurst <[email protected]>
…iscovery (gsd-build#759) * fix: use `.claude/skills/` instead of `.agents/skills/` in agent and workflow skill references Claude Code resolves project skills from `.claude/skills/` (project-level) and `~/.claude/skills/` (user-level). The `.agents/skills/` path is the universal/IDE-agnostic convention that Claude Code does not resolve, causing project skills to be silently ignored by all affected agents and workflows. Fixes gsd-build#758 Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: support both `.claude/skills/` and `.agents/skills/` for cross-IDE compatibility Instead of replacing `.agents/skills/` with `.claude/skills/`, reference both paths so GSD works with Claude Code (`.claude/skills/`) and other IDE agents like OpenCode (`.agents/skills/`). Addresses review feedback from begna112 on gsd-build#758. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Stephen Miller <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
…nt MODULE_NOT_FOUND (gsd-build#786) Claude Code subagents sometimes rewrite ~/. paths to relative paths, causing MODULE_NOT_FOUND when CWD is the project directory. $HOME is a shell variable resolved at runtime, immune to model path rewriting. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The SessionStart hook always writes to ~/.claude/cache/ via os.homedir() regardless of install type. The update workflow previously only cleared the install-type-specific path, leaving stale cache at the global path for local installs. Clear both ./.claude/cache/ and ~/.claude/cache/ unconditionally. Co-authored-by: Claude Sonnet 4.5 <[email protected]>
…slines (gsd-build#670) The gsd-build#330 migration that renames `statusline.js` → `gsd-statusline.js` uses `.includes('statusline.js')` which matches ANY file containing that substring. For example, a user's custom `ted-statusline.js` gets silently rewritten to `ted-gsd-statusline.js` (which doesn't exist). This happens inside `cleanupOrphanedHooks()` which runs before the interactive "Keep existing / Replace" prompt, so even choosing "Keep existing" doesn't prevent the damage. Fix: narrow the regex to only match the specific old GSD path pattern `hooks/statusline.js` (or `hooks\statusline.js` on Windows). Co-authored-by: ddungan <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]>
Contributor
Author
|
These were all addressed during the productive Feb 27 merge session — wonderful to see the project moving so quickly! Closing this to keep the PR queue tidy. Excited to keep contributing where it helps most. 🙌 |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves 8 distinct CLI and config bugs across gsd-tools core, agent templates, and workflow definitions. Fixes cover null field extraction (#734), missing config loading (#740), regex injection (#741), variable name mismatches (#737), stale skill discovery paths (#759), subagent path resolution failures (#786), split cache invalidation (#663, #664), and overly broad migration regex (#670).
50 files changed, 426 insertions, 198 deletions. 4 new tests added for canonical plan format parsing; full suite passes (423 tests, 0 failures).
Root Cause
files_modifiedonly checked hyphenatedfiles-modifiedkey (plans use underscorefiles_modified).objectiveonly checked frontmatter (plans use<objective>XML tag).task_countonly matched## Task Nmarkdown (plans use<task>XML tags). All three returned null/0 for canonical plan format.loadConfig()incore.cjshad no entry fornyquist_validation-- the field was defined in config.json schema but never read, soplan-phasealways saw it as undefined and skipped Nyquist validation even when enabled.stateExtractField()interpolated user-supplied field names directly into aRegExpconstructor without escaping. Field names containing regex metacharacters (e.g.,Progress (%),Status [draft]) produced invalid or incorrect regex patterns.debug.mdassigned toDEBUGGER_MODEL(uppercase) butTask()calls referenced{debugger_model}(lowercase). Same forCHECKER_MODELvs{integration_checker_model}inaudit-milestone.md. Claude treated the mismatch as an unresolved placeholder and fell back to the parent session model, ignoring the configured GSD profile..agents/skills/as the only skill discovery path. After Claude Code SDK v1.0.17 moved skills to.claude/skills/, agents in projects using the new layout silently skipped all project skills.~/.claude/get-shit-done/bin/gsd-tools.cjswith a bare tilde. Subagents spawned viaTask()do not inherit the parent shell's tilde expansion, so~resolved literally, causingMODULE_NOT_FOUNDerrors in every subagentnodeinvocation.SessionStarthook (gsd-check-update.js) always writes the update-check cache to~/.claude/cache/viaos.homedir(), regardless of install type. The update workflow only cleared the install-type-specific path (local:./.claude/cache/, global:~/.claude/cache/), leaving stale cache at the other path and a phantom "update available" statusline indicator.cleanupOrphanedHooks()ininstall.jsmatched anystatusLine.commandcontainingstatusline.js(e.g.,includes('statusline.js')) and renamed it togsd-statusline.js. This clobbered third-party statusline scripts (e.g.,~/.config/my-tool/statusline.js) that happened to contain the substring.Fix
files_modified(underscore, canonical) andfiles-modified(hyphen, legacy). AddextractObjective()helper to parse<objective>XML tags from plan body. Count<task>XML tags first, fall back to## Task Nmarkdown.nyquist_validationtoloadConfig()defaults (false) and config reader chain (workflow.nyquist_validation).fieldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')before interpolating intoRegExp. Applied to bothstateExtractField()andstateReplaceField().DEBUGGER_MODELtodebugger_modelandCHECKER_MODELtointegration_checker_modelto match the lowercase placeholders inTask()template strings..agents/skills/to.claude/skills/ or .agents/skills/with "if either exists" semantics, supporting both old and new SDK layouts.~/.claude/get-shit-done/bin/gsd-tools.cjswith"$HOME/.claude/get-shit-done/bin/gsd-tools.cjs"(quoted, environment variable) across 44 files (agents, commands, workflows, references)../.claude/cache/gsd-update-check.jsonand~/.claude/cache/gsd-update-check.jsonunconditionally, removing the install-type conditional logic.includes('statusline.js')to/hooks[\/\\]statusline\.js/(must be inside ahooks/directory). Narrow the replacement regex from/statusline\.js/to/hooks([\/\\])statusline\.js/with backreferencehooks$1gsd-statusline.js, preserving the path separator.Relationship to Other PRs
This is PR #6 of 6 from the
dev-bugfixbranch:fix/milestone-completion-bugsfix/cross-platform-windows-cifeat/agent-discuss-codex-enhancementsfix/agent-frontmatter-heredocfix/cli-config-bugsfix/mcp-migration-helperNo code overlap between PRs; all can be merged independently. The
$HOMEpath fix (#786) in this PR complements PR #1's MCP migration work -- both address subagent path resolution but through different mechanisms (environment variable expansion vs. connectivity pre-checks).Testing
PR-specific tests (new in this PR)
files_modified: underscore key is parsed correctlytests/phase.test.cjsfiles_modifiedfrontmatter keyobjective: extracted from <objective> XML tagtests/phase.test.cjstask_count: counts <task> XML tagstests/phase.test.cjsall three fields work together in canonical plan formattests/phase.test.cjsFull suite results
commands.test.cjsconfig.test.cjscore.test.cjsdispatcher.test.cjsfrontmatter-cli.test.cjsfrontmatter.test.cjsinit.test.cjsmilestone.test.cjsphase.test.cjsroadmap.test.cjsstate.test.cjsverify-health.test.cjsverify.test.cjsManual validation
npm testpasses with all fixes appliedhooks/statusline.jspath$HOMEexpansion works in subagent contexts where tilde does not.claude/skills/and.agents/skills/are discoveredImpact
Severity: High -- Six of these bugs cause silent failures in production workflows:
nodeinvocation (MODULE_NOT_FOUND).claude/skills/and.agents/skills/for skill discovery #759 causes agents to silently skip all project skillsScope: 50 files across agents (7), commands (2), workflows (27), references (4), core libs (3), installer (1), and tests (1). The
$HOMEfix alone touches 44 files with 166 path replacements.Risk: Low -- All fixes are narrowly scoped (regex tightening, config key additions, variable renames, path quoting). No behavioral changes to happy-path logic. Full test suite passes with zero regressions.