Skip to content

fix(workstreams): replace GSD_TOOLS env var with resolved binary path#1629

Open
SLAPaper wants to merge 1 commit intogsd-build:mainfrom
SLAPaper:main
Open

fix(workstreams): replace GSD_TOOLS env var with resolved binary path#1629
SLAPaper wants to merge 1 commit intogsd-build:mainfrom
SLAPaper:main

Conversation

@SLAPaper
Copy link
Copy Markdown

@SLAPaper SLAPaper commented Apr 4, 2026

Summary

  • Replace GSD_TOOLS environment variable references with the resolved absolute path $HOME/.claude/get-shit-done/bin/gsd-tools.cjs in the workstreams command docs
  • Ensures workstream commands work correctly in runtimes where GSD_TOOLS is not set

reference: #1236

BTW: actually it's better to use the env var for cross platform deployment, but it seems that the env var comes from nowhere, so comply with other commands first

Hardcode the absolute path to gsd-tools.cjs instead of relying on
the GSD_TOOLS environment variable, which may not be set in all
runtimes.
Copilot AI review requested due to automatic review settings April 4, 2026 02:20
@SLAPaper SLAPaper requested a review from glittercowboy as a code owner April 4, 2026 02:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the /gsd:workstreams command documentation to call the gsd-tools Node script via the expected install location instead of relying on a GSD_TOOLS environment variable, improving runtime robustness where that env var is unset.

Changes:

  • Replaced node "$GSD_TOOLS" ... invocations with node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" ... in the workstreams command docs.
  • Updated all workstream subcommand “Run:” lines (list/create/status/switch/progress/complete) to use the new invocation path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


### list
Run: `node "$GSD_TOOLS" workstream list --raw --cwd "$CWD"`
Run: `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" workstream list --raw --cwd "$CWD"`
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline Run: commands include escaped quotes (\") inside Markdown inline code. This renders with literal backslashes and makes the docs harder to read/copy; it’s also inconsistent with most other command docs that show normal quoting. Suggest removing the backslashes and using plain " quoting in the inline command (or move the command into a ```bash fenced block).

Copilot uses AI. Check for mistakes.
Comment on lines +38 to +42
Run: `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" workstream list --raw --cwd "$CWD"`
Display the workstreams in a table format showing name, status, current phase, and progress.

### create
Run: `node "$GSD_TOOLS" workstream create <name> --raw --cwd "$CWD"`
Run: `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" workstream create <name> --raw --cwd "$CWD"`
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description mentions replacing env-var references with a fully resolved absolute path (example shows a Windows path), but the doc now uses $HOME/.claude/.../gsd-tools.cjs which still depends on an environment variable and won’t match the stated Windows path. Please align the PR description with the actual approach, or update the docs to describe the intended cross-platform resolved path behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@trek-e trek-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: incomplete fix — GSD_TOOLS env var is undefined throughout the codebase, but this PR only fixes workstreams.md

The root issue is correct: GSD_TOOLS is never exported as an environment variable by the GSD installer or any startup script. It appears only as a TypeScript constant inside the SDK source (sdk/src/gsd-tools.ts), not as an env var. Using $GSD_TOOLS in shell commands inside markdown workflows/commands silently fails — node receives an empty string as the binary path.

However, workstreams.md is not the only place this pattern appears. A broader audit is needed. Other command/workflow files should be checked for the same node "$GSD_TOOLS" pattern.

Additionally, the author's own note in the PR body acknowledges: "actually it's better to use the env var for cross platform deployment, but it seems that the env var comes from nowhere, so comply with other commands first." The correct fix is one of:

  1. Have the installer/startup script export GSD_TOOLS pointing to the resolved binary path, and update all usages to use the env var consistently — this is the cross-platform correct approach.
  2. Replace all occurrences with the hardcoded $HOME/.claude/get-shit-done/bin/gsd-tools.cjs path — this is what the rest of the codebase does.

This PR takes option 2 but only for workstreams.md. If the intent is option 2, please audit all command and workflow files for remaining $GSD_TOOLS references and fix them in the same PR. If the intent is option 1, please add the env var export to the installer instead.

No linked issue — please add Closes # or Fixes # if this addresses an existing issue, or open one.

@trek-e trek-e added review: changes requested PR reviewed — changes required before merge bug Something isn't working area: workflow Phase execution, ROADMAP, STATE.md labels Apr 4, 2026
Copy link
Copy Markdown
Collaborator

@trek-e trek-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Verdict: COMMENT

Security

No issues found.

Logic / Correctness

The hardcoded path $HOME/.claude/get-shit-done/bin/gsd-tools.cjs is consistent with how other command docs already work (e.g. commands/gsd/thread.md uses the same pattern). The GSD_TOOLS variable is only defined inside the SDK's TypeScript source (sdk/src/gsd-tools.ts), not in the shell environment where these .md command docs execute — so the old references were already broken in practice.

One concern: $HOME expands correctly on macOS/Linux but will not expand on Windows. If cross-platform support is a goal, the install path should be surfaced as an env var set by the installer (e.g. GSD_TOOLS written by bin/install.js) rather than reconstructed from $HOME at call-site. The PR author notes this in the PR body. If that is out of scope here, a follow-up issue to make the installer export GSD_TOOLS would close the loop cleanly.

Test Coverage

No tests exist for these command-doc shell invocations (they are prose instructions for the Claude runtime, not executable test targets), so no gap to flag.

Style

All six changed lines follow the pattern already established by thread.md and other command docs. No style issues.

Summary

This fix correctly aligns workstreams.md with the rest of the command docs, replacing a broken $GSD_TOOLS reference with the same $HOME-anchored absolute path used elsewhere. The only open question is whether the installer should export GSD_TOOLS to make future path changes a single-point edit rather than a grep-and-replace across all command docs.

Copy link
Copy Markdown
Collaborator

@trek-e trek-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Update (Pass 2)

Verdict: APPROVE — logic is correct, no CI results

Prior Review Resolution

The prior review approved the replacement of GSD_TOOLS env var with the resolved absolute path $HOME/.claude/get-shit-done/bin/gsd-tools.cjs in workstreams command docs. The logic is correct — GSD_TOOLS is only defined in the TypeScript SDK context, not in the shell environment where .md command docs execute.

No CI Results

No checks reported on the main branch (this PR is from main → the base branch, which is unusual). Confirm the target branch is correct.

Summary

The change is correct and safe. The PR branch being named main targeting another main is unusual and worth verifying this wasn't accidentally submitted against the wrong base.

@trek-e trek-e added the needs merge fixes CI failing or merge conflicts need resolution label Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: workflow Phase execution, ROADMAP, STATE.md bug Something isn't working needs merge fixes CI failing or merge conflicts need resolution review: changes requested PR reviewed — changes required before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants