fix(ci): cross-platform Windows CI and path handling#810
Open
Tibsfox wants to merge 5 commits intogsd-build:mainfrom
Open
fix(ci): cross-platform Windows CI and path handling#810Tibsfox wants to merge 5 commits intogsd-build:mainfrom
Tibsfox wants to merge 5 commits intogsd-build:mainfrom
Conversation
Windows PowerShell doesn't expand `tests/*.test.cjs` globs, causing the test runner to fail with "Could not find" on Windows Node 20. Co-Authored-By: Claude Opus 4.6 <[email protected]>
npm scripts pass `tests/*.test.cjs` to node/c8 as a literal string on Windows (PowerShell/cmd don't expand globs). Adding `shell: bash` to CI steps doesn't help because c8 spawns node as a child process using the system shell. Use a Node script to enumerate test files cross-platform. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The run-tests.cjs child process now inherits NODE_V8_COVERAGE from the parent so c8 collects coverage data. Also restores npm scripts to use the cross-platform runner for both test and test:coverage commands. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…r signs - Add toPosixPath() helper to normalize output paths to forward slashes - Use string concatenation for relative base paths instead of path.join() - Apply toPosixPath() to all user-facing file paths in init.cjs output - Use array-based execFileSync in test helpers to bypass shell quoting issues with JSON args and dollar signs on Windows cmd.exe Fixes 7 test failures on Windows: frontmatter set/merge (3), init path assertions (2), and state dollar-amount corruption (2). Co-Authored-By: Claude Opus 4.6 <[email protected]>
The source now outputs posix paths; update the test to match instead of using path.join (which produces backslashes on Windows). Co-Authored-By: Claude Opus 4.6 <[email protected]>
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
GSD tools fail on Windows in three distinct ways: (1)
npm testandnpm run test:coveragesilently find zero test files because PowerShell/cmd do not expand shell globs liketests/*.test.cjs, (2)path.join()produces backslash-separated paths (.planning\phases\01-setup) in JSON output that downstream consumers (Claude Code agents, CLAUDE.md file-read instructions) cannot resolve on Windows, and (3) test assertions that pass JSON strings containing$or["a","b"]throughexecSyncshell interpolation get corrupted on Windows (and can fail on any platform depending on shell state).Root Cause
1. Shell glob expansion in npm scripts (CI failure)
package.jsonused a bare shell glob:On Linux/macOS, bash expands
tests/*.test.cjsbefore invokingnode. On Windows, PowerShell and cmd pass the literal stringtests/*.test.cjsto Node, which does not recognize it as a glob — resulting in zero test files found and a silent pass.2.
path.join()producing backslash paths in output (runtime bug)Multiple functions in
core.cjsandinit.cjsusedpath.join()to construct paths returned in JSON output:On Windows,
path.join('.planning', 'phases', '01-setup')returns.planning\phases\01-setup. Consumers that read these paths (Claude Code agents, downstream commands) expect forward slashes and fail to locate files.3. Shell interpolation of
$and JSON in test commands (test reliability)Tests passed arguments containing dollar signs and JSON through
execSyncwith shell interpolation:Shells interpret
$0,$2as positional parameters (expanding to empty strings or the shell name). On Windows, single quotes are not special — the JSON brackets and quotes get mangled. Both cases silently corrupt the argument before it reaches the CLI.Fix
Commit 1 (
ea0e712): Addshell: bashto both test steps in.github/workflows/test.yml. This ensures GitHub Actions uses bash on Windows runners, providing reliable glob expansion as a quick mitigation.Commit 2 (
edaa673): Createscripts/run-tests.cjs— a cross-platform test runner that resolves test files viafs.readdirSync()instead of shell globs. Updatepackage.jsonto usenode scripts/run-tests.cjsfor bothtestandtest:coveragescripts. This eliminates the dependency on shell glob expansion entirely.Commit 3 (
4c91f74): PropagateNODE_V8_COVERAGEenvironment variable in the test runner by spreadingprocess.envinto the child process. Without this,c8cannot collect coverage data from the subprocess.Commit 4 (
13a8b0e): Three sub-fixes in one commit:Path separators: Add
toPosixPath()helper tocore.cjsthat normalizespath.septo/. Apply it to all 10 path-construction sites incore.cjs(searchPhaseInDir, findPhaseInternal) andinit.cjs(cmdInitPlanPhase, cmdInitPhaseOp, cmdInitTodos, cmdInitProgress). For paths that are purely string literals with no OS-dependent segments, use direct string concatenation ('.planning/phases/' + dir) instead ofpath.join().JSON quoting: Convert 3 test calls in
frontmatter-cli.test.cjsand 2 instate.test.cjsfrom string-basedrunGsdTools()to array-based form, bypassing shell interpolation entirely.Test helper: Extend
runGsdTools()intests/helpers.cjsto acceptstring[]in addition tostring. When an array is passed, it usesexecFileSync(no shell) instead ofexecSync(shell), preventing all shell-interpolation issues.Commit 5 (
c58d9fe): Update thecmdInitTodostest assertion frompath.join('.planning', 'todos', 'pending', 'task-1.md')to the literal'.planning/todos/pending/task-1.md', matching the new forward-slash output.Edge cases handled:
toPosixPath()is a no-op on Linux/macOS (separator is already/)runGsdTools()string overload is preserved for backward compatibility with existing tests that don't need shell-safe argumentsenv: { ...process.env }in the test runner copies the full environment includingNODE_V8_COVERAGE,PATH, and any user-set variablesRelationship to Other PRs
This is PR 3 of 6 from the
dev-bugfixbranch, split for independent review:No code overlap between PRs. Each can be merged independently in any order. This PR touches files (
core.cjs,init.cjs,helpers.cjs,run-tests.cjs,test.yml,package.json) that no other PR modifies.Testing
Tests directly exercising the fixes
cmdInitTodos > returns all pending todostests/init.test.cjsfrontmatter set > handles JSON array valuetests/frontmatter-cli.test.cjsrunGsdToolsbypasses shell JSON manglingfrontmatter merge > merges multiple fieldstests/frontmatter-cli.test.cjsrunGsdToolswith JSON--dataargumentfrontmatter merge > overwrites existing fieldstests/frontmatter-cli.test.cjsstate add-decision > dollar signs preservedtests/state.test.cjs$0.50,$2.00,$5.00survive without shell expansionstate add-blocker > dollar strings preservedtests/state.test.cjs$1.00survives without shell expansionsearchPhaseInDir(6 tests)tests/commands.test.cjsfindPhaseInternal(8 tests)tests/commands.test.cjscmdInitProgress(3 tests)tests/init.test.cjscmdInitPhaseOp fallback(4 tests)tests/init.test.cjsFull suite results
Impact
npm teston Windows (PowerShell)npm run test:coverageon WindowsNODE_V8_COVERAGEpropagation".planning\\phases\\01-setup"— downstream agents fail to read".planning/phases/01-setup"— consistent cross-platform paths".planning\\todos\\pending\\task-1.md"".planning/todos/pending/task-1.md"state add-decisionwith$0.50$0to shell name,$5to emptyexecFileSyncfrontmatter set --value '["a","b"]'toPosixPath()is a no-op whenpath.sep === '/'9 files changed, 84 insertions(+), 29 deletions(-)