From d7dafafd03076c579f1eca46e2db3a01f8eda2cb Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 29 May 2026 17:12:50 -0400 Subject: [PATCH] fix(#442): --config-dir= no longer truncates paths containing equals signs (#475) Extract a pure `parseConfigDirFromArgs(argsArray)` seam from the closure-based `parseConfigDirArg()` and fix the equals-form parser to use `slice(indexOf('=') + 1)` instead of `split('=')[1]`, so that paths like `/tmp/gsd=a` or `/tmp/a=b=c` are preserved in full. Both `--config-dir=` and `-c=` are fixed. The pure seam is exported via `module.exports` so the 12-case unit test can assert on typed return values without spawning a child process. Co-authored-by: CI Rebase Check Co-authored-by: Claude Opus 4.8 (1M context) --- .../442-config-dir-equals-truncation.md | 5 + bin/install.js | 50 +++++++--- ...bug-442-config-dir-equals-in-path.test.cjs | 99 +++++++++++++++++++ 3 files changed, 139 insertions(+), 15 deletions(-) create mode 100644 .changeset/442-config-dir-equals-truncation.md create mode 100644 tests/bug-442-config-dir-equals-in-path.test.cjs diff --git a/.changeset/442-config-dir-equals-truncation.md b/.changeset/442-config-dir-equals-truncation.md new file mode 100644 index 000000000..10f94f321 --- /dev/null +++ b/.changeset/442-config-dir-equals-truncation.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 475 +--- +**`--config-dir=` no longer truncates paths containing `=`** — the equals-form parser now splits on the first `=` only. diff --git a/bin/install.js b/bin/install.js index f229c0061..e33bb4c81 100755 --- a/bin/install.js +++ b/bin/install.js @@ -619,30 +619,49 @@ const banner = '\n' + ' A meta-prompting, context engineering and spec-driven\n' + ' development system for Claude Code, OpenCode, Gemini, Kilo, Codex, Copilot, Antigravity, Cursor, Windsurf, Augment, Trae, Qwen Code, Hermes Agent, Cline and CodeBuddy by TÂCHES.\n'; -// Parse --config-dir argument -function parseConfigDirArg() { - const configDirIndex = args.findIndex(arg => arg === '--config-dir' || arg === '-c'); +// Pure seam: parse --config-dir / -c from an arbitrary args array. +// Returns the path string, '' for an empty equals-form value, or null when the +// flag is absent. Space-separated form returns null (not an error string) when +// the next token is missing or flag-looking — callers that want process.exit +// behaviour must check after calling this function. +// Exported via module.exports so unit tests can exercise it directly. +function parseConfigDirFromArgs(argsArray) { + const configDirIndex = argsArray.findIndex(arg => arg === '--config-dir' || arg === '-c'); if (configDirIndex !== -1) { - const nextArg = args[configDirIndex + 1]; - // Error if --config-dir is provided without a value or next arg is another flag + const nextArg = argsArray[configDirIndex + 1]; + // No value / next token is a flag → signal "missing" by returning null if (!nextArg || nextArg.startsWith('-')) { - console.error(` ${yellow}--config-dir requires a path argument${reset}`); - process.exit(1); + return null; } return nextArg; } - // Also handle --config-dir=value format - const configDirArg = args.find(arg => arg.startsWith('--config-dir=') || arg.startsWith('-c=')); + // Handle --config-dir=value and -c=value format. + // Use indexOf('=') + 1 so that = signs inside the path value are preserved. + const configDirArg = argsArray.find(arg => arg.startsWith('--config-dir=') || arg.startsWith('-c=')); if (configDirArg) { - const value = configDirArg.split('=')[1]; - if (!value) { - console.error(` ${yellow}--config-dir requires a non-empty path${reset}`); - process.exit(1); - } - return value; + return configDirArg.slice(configDirArg.indexOf('=') + 1); } return null; } + +// Parse --config-dir argument +function parseConfigDirArg() { + const result = parseConfigDirFromArgs(args); + if (result === null) { + // Check if the space-separated form was present but missing a value + const configDirIndex = args.findIndex(arg => arg === '--config-dir' || arg === '-c'); + if (configDirIndex !== -1) { + console.error(` ${yellow}--config-dir requires a path argument${reset}`); + process.exit(1); + } + return null; + } + if (result === '') { + console.error(` ${yellow}--config-dir requires a non-empty path${reset}`); + process.exit(1); + } + return result; +} const explicitConfigDir = parseConfigDirArg(); const hasHelp = args.includes('--help') || args.includes('-h'); const forceStatusline = args.includes('--force-statusline'); @@ -11799,6 +11818,7 @@ module.exports = { readGsdCommandNames, installRuntimeArtifacts, uninstallRuntimeArtifacts, + parseConfigDirFromArgs, }; // Main logic — only run when not loaded as a module for testing diff --git a/tests/bug-442-config-dir-equals-in-path.test.cjs b/tests/bug-442-config-dir-equals-in-path.test.cjs new file mode 100644 index 000000000..1bf000113 --- /dev/null +++ b/tests/bug-442-config-dir-equals-in-path.test.cjs @@ -0,0 +1,99 @@ +'use strict'; +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); + +// parseConfigDirArg is not exported directly from bin/install.js (it closes +// over the module-level `args` array). We expose a pure seam here: +// parseConfigDirFromArgs(args) that mirrors the function's logic so we can +// test the equals-form parsing without spawning a child process. +// +// The implementation under test is inlined below (RED: before the fix it will +// reproduce the truncation bug). Once the fix lands, we swap in the real +// implementation via require. + +/** + * Pure seam that replicates the equals-form parse logic from bin/install.js. + * We import it via a thin wrapper so that the function can be tested without + * executing the entire install script. + * + * During RED the bug is: `split('=')[1]` drops everything after the second `=`. + */ +const { parseConfigDirFromArgs } = require('../bin/install.js'); + +describe('bug-442: --config-dir= equals-form path parsing', () => { + // ── Happy-path: single = in path ───────────────────────────────────────── + test('--config-dir= with one = in value returns full value', () => { + const result = parseConfigDirFromArgs(['--config-dir=/tmp/gsd=a']); + assert.equal(result, '/tmp/gsd=a'); + }); + + // ── Happy-path: multiple = in path ─────────────────────────────────────── + test('--config-dir= with multiple = in value returns full value', () => { + const result = parseConfigDirFromArgs(['--config-dir=/tmp/a=b=c']); + assert.equal(result, '/tmp/a=b=c'); + }); + + // ── Short form -c= ──────────────────────────────────────────────────────── + test('-c= with = in value returns full value', () => { + const result = parseConfigDirFromArgs(['-c=/tmp/gsd=a']); + assert.equal(result, '/tmp/gsd=a'); + }); + + test('-c= with multiple = in value returns full value', () => { + const result = parseConfigDirFromArgs(['-c=/tmp/a=b=c']); + assert.equal(result, '/tmp/a=b=c'); + }); + + // ── Contract: empty value ───────────────────────────────────────────────── + // --config-dir= (no value after the =) → returns empty string ''. + // The caller (parseConfigDirArg) treats '' as missing and errors; the seam + // itself should faithfully return '' rather than null/undefined so the + // caller can make the error decision. + test('--config-dir= with no value returns empty string', () => { + const result = parseConfigDirFromArgs(['--config-dir=']); + assert.equal(result, ''); + }); + + test('-c= with no value returns empty string', () => { + const result = parseConfigDirFromArgs(['-c=']); + assert.equal(result, ''); + }); + + // ── Space-separated form is unaffected (regression guard) ───────────────── + test('--config-dir space-separated still returns the path', () => { + const result = parseConfigDirFromArgs(['--config-dir', '/tmp/gsd=a']); + assert.equal(result, '/tmp/gsd=a'); + }); + + test('-c space-separated still returns the path', () => { + const result = parseConfigDirFromArgs(['-c', '/tmp/gsd=a']); + assert.equal(result, '/tmp/gsd=a'); + }); + + // ── No config-dir flag → null ───────────────────────────────────────────── + test('returns null when no --config-dir flag is present', () => { + const result = parseConfigDirFromArgs(['--global', '--claude']); + assert.equal(result, null); + }); + + // ── Negative matrix (CLI edge cases) ───────────────────────────────────── + // Flag-looking value after space form: next arg starts with - → null (no + // valid value; the real function would process.exit but the seam returns null + // so tests stay in-process). + test('space form with next arg being a flag returns null (flag-looking value)', () => { + const result = parseConfigDirFromArgs(['--config-dir', '--other-flag']); + assert.equal(result, null); + }); + + // Equals form where value is a path with no = (plain path, no regression) + test('--config-dir= without any = in path still works', () => { + const result = parseConfigDirFromArgs(['--config-dir=/tmp/plain']); + assert.equal(result, '/tmp/plain'); + }); + + // Flag appears after other args (positional ordering should not matter) + test('--config-dir= flag after other args is parsed correctly', () => { + const result = parseConfigDirFromArgs(['--global', '--config-dir=/tmp/a=b', '--claude']); + assert.equal(result, '/tmp/a=b'); + }); +});