diff --git a/.changeset/humble-cranes-click.md b/.changeset/humble-cranes-click.md new file mode 100644 index 000000000..49e4e0e74 --- /dev/null +++ b/.changeset/humble-cranes-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4509 +--- +**Whitespace-only planning scope values no longer create phantom directories** — `GSD_WORKSTREAM`, `GSD_PROJECT`, and their explicit equivalents now fall back to the root scope, while padded real names are normalized consistently. diff --git a/src/config-loader.cts b/src/config-loader.cts index 3674e48bd..924c1891e 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -683,7 +683,7 @@ function loadConfigResolved(cwd: string, options: Record = {}): : (options['workstreamContext'] && Object.prototype.hasOwnProperty.call(options['workstreamContext'], 'ws')) ? (options['workstreamContext'] as Record)['ws'] : (process.env['GSD_WORKSTREAM'] || null); - const ws = typeof activeWorkstream === 'string' ? activeWorkstream : (activeWorkstream === null ? null : null); + const ws = typeof activeWorkstream === 'string' ? activeWorkstream.trim() || null : null; // wsRequested: true when caller explicitly requested a non-empty workstream. // Used for source labeling (Fix 4) and early absent-dir intercept (Fix 2). const wsRequested = ws != null && ws !== ''; diff --git a/src/config.cts b/src/config.cts index f9d1076b1..bf349ac76 100644 --- a/src/config.cts +++ b/src/config.cts @@ -21,7 +21,7 @@ const { CONFIG_DEFAULTS } = configLoader; import { platformWriteSync, platformEnsureDir } from './shell-command-projection.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); -const { planningDir, planningRoot, withPlanningLock } = planningWorkspace; +const { planningDir, planningRoot, resolveEnvWorkstream, withPlanningLock } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports import modelProfiles = require('./model-profiles.cjs'); const { VALID_PROFILES, getAgentToModelMapForProfile, formatAgentToModelMapAsTable } = modelProfiles; @@ -1269,7 +1269,7 @@ function resolveFromRootConfig(cwd: string, kp: string): { found: boolean; value // diverges from planningRoot without a workstream and loadConfigResolved does NOT // inherit root — matching the runtime's own `if (ws)` gate keeps the two surfaces // from diverging on the project-scoped (non-workstream) case. - if (!process.env['GSD_WORKSTREAM']) return { found: false, value: undefined }; + if (!resolveEnvWorkstream()) return { found: false, value: undefined }; const root = planningRoot(cwd); const rootConfigPath = path.join(root, 'config.json'); let rootConfig: Record; @@ -1388,7 +1388,7 @@ function cmdConfigPath(cwd: string, _raw: boolean, workstreamContext: Workstream * (caller uses `await` which is safe on a sync return value). */ function cmdMigrateConfig(cwd: string, raw: boolean): void { - const ws = process.env['GSD_WORKSTREAM'] || null; + const ws = resolveEnvWorkstream(); // #3749: resolve the migration target through the project-aware resolver so // GSD_PROJECT scopes the write; migrateOnDisk itself cannot (see its // configPathOverride note). diff --git a/src/init.cts b/src/init.cts index a6dc04269..9a295dc98 100644 --- a/src/init.cts +++ b/src/init.cts @@ -107,6 +107,7 @@ const { todosDir, listAvailableWorkstreams, peekActiveWorkstream, + resolveEnvWorkstream, diagnoseUnresolvedActiveWorkstream, describeUnresolvedWorkstreamReason, findContextMdIn, @@ -1552,7 +1553,7 @@ function cmdInitNewMilestone(cwd: string, raw: boolean, options: Record = {}): void { // #3579 root-cause fix: read-only informational field — peek, don't // self-heal (see cmdInitNewMilestone's identical rationale above). - const resolvedWorkstream = process.env['GSD_WORKSTREAM'] || peekActiveWorkstream(cwd); + const resolvedWorkstream = resolveEnvWorkstream() ?? peekActiveWorkstream(cwd); const workstreamActive = !!resolvedWorkstream; const result: Record = { @@ -3470,7 +3471,7 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record 0 && !_resolvedWorkstream) { // #3579: getActiveWorkstream now inherits a pointer-less session's read // from the shared .planning/active-workstream marker, so reaching this diff --git a/src/phase.cts b/src/phase.cts index 67b90e559..902c2e4da 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -100,7 +100,7 @@ const { computeHaltPropagation, buildSummaryFileIndex, isSummaryFileHalted, isSu const { planningDir, withPlanningLock, listAvailableWorkstreams, peekActiveWorkstream, diagnoseUnresolvedActiveWorkstream, describeUnresolvedWorkstreamReason, - resolvePhaseIdConvention, + resolveEnvWorkstream, resolvePhaseIdConvention, } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports -- milestone-lock.cjs is an export= CommonJS module import milestoneLockMod = require('./milestone-lock.cjs'); @@ -3358,7 +3358,7 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // non-mutating peek so an unresolvable pointer isn't self-healed (cleared) // here and then found "absent" by diagnoseUnresolvedActiveWorkstream below, // which would misreport a present-but-bad marker as no marker at all. - const resolvedWorkstream = process.env['GSD_WORKSTREAM'] || peekActiveWorkstream(cwd); + const resolvedWorkstream = resolveEnvWorkstream() ?? peekActiveWorkstream(cwd); if (availableWorkstreams.length > 0 && !resolvedWorkstream) { // #3579: getActiveWorkstream now inherits a pointer-less session's read // from the shared .planning/active-workstream marker, so reaching this diff --git a/src/planning-workspace.cts b/src/planning-workspace.cts index e87cb2168..a7143244c 100644 --- a/src/planning-workspace.cts +++ b/src/planning-workspace.cts @@ -139,12 +139,15 @@ type WorkstreamAdapterOpts = Record; * two-readers-two-bases lesson). */ function resolveEnvWorkstream(): string | null { - return process.env['GSD_WORKSTREAM'] ?? null; + const value = process.env['GSD_WORKSTREAM']?.trim(); + return value || null; } function planningDir(cwd: string, ws?: string | null, project?: string | null): string { - if (project === undefined) project = process.env['GSD_PROJECT'] ?? null; + if (project === undefined) project = process.env['GSD_PROJECT']?.trim() || null; + else if (typeof project === 'string') project = project.trim() || null; if (ws === undefined) ws = resolveEnvWorkstream(); + else if (typeof ws === 'string') ws = ws.trim() || null; // Reject path separators and traversal components in project/workstream names const BAD_SEGMENT = /[/\\]|\.\./; @@ -210,7 +213,7 @@ function worktreesOptedOutUnguarded(cwd: string): boolean { }; const scoped = ownKey(readCfg(path.join(planningDir(cwd), 'config.json'))); if (scoped.present) return scoped.value === false; - if (process.env['GSD_WORKSTREAM']) { + if (resolveEnvWorkstream() !== null) { const root = ownKey(readCfg(path.join(planningRoot(cwd), 'config.json'))); if (root.present) return root.value === false; } diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index 89d8a0aad..da05da8ee 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -494,6 +494,26 @@ describe('loadConfigResolved — provenance', () => { assert.equal(result.degraded, false); }); + test('whitespace-only explicit and environment workstreams resolve and label the root (#4462)', () => { + writeConfig(tmpDir, { model_profile: 'quality' }); + for (const workstream of [' ', '\t', '\n', ' \t\n ']) { + const explicit = loadConfigResolved(tmpDir, { workstream }); + assert.equal(explicit.source, 'root'); + assert.equal(explicit.degraded, false); + + const original = process.env.GSD_WORKSTREAM; + try { + process.env.GSD_WORKSTREAM = workstream; + const ambient = loadConfigResolved(tmpDir); + assert.equal(ambient.source, 'root'); + assert.equal(ambient.degraded, false); + } finally { + if (original === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = original; + } + } + }); + test('Fix 2a: GSD_WORKSTREAM set to nonexistent workstream (dir absent) → source:"root", degraded:true', () => { writeConfig(tmpDir, { model_profile: 'root-value' }); const origWs = process.env['GSD_WORKSTREAM']; diff --git a/tests/init.test.cjs b/tests/init.test.cjs index dbb9698bd..8b934f065 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -2968,6 +2968,21 @@ describe('#1912 — init.progress fails safe in workstream mode with no active w assert.match(result.error || '', /workstream|--ws/i, 'error should name the workstream requirement'); }); + test('treats a whitespace environment workstream as unset and refuses root progress', (t) => { + seedWs('alpha', 'v9.0'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'milestone: v7.1\nstatus: executing\n'); + const previous = process.env.GSD_WORKSTREAM; + t.after(() => { + if (previous === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = previous; + }); + process.env.GSD_WORKSTREAM = ' '; + + const result = runGsdTools('init progress', tmpDir); + assert.equal(result.success, false, 'a whitespace workstream must not bypass the root-write guard'); + assert.match(result.error || '', /workstream|--ws/i); + }); + test('succeeds with --ws (reads the named workstream, not root)', () => { seedWs('alpha', 'v9.0'); fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'milestone: v7.1\nstatus: executing\n'); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 5532e18e3..a617566ad 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2948,6 +2948,28 @@ describe('phase add --ws workstream-scoped allocation vs sibling git worktrees ( ); }); + test('whitespace-only GSD_WORKSTREAM uses the root sibling horizon (#4462)', () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4462-blank-ws-')); + activeDirs.push(repoDir); + initWsRepo(repoDir, 2, 'ws-alpha', 39); + addSiblingAtHead(repoDir); + + const result = runGsdTools( + ['query', 'phase.add', 'Root next'], + repoDir, + { GSD_WORKSTREAM: ' ' }, + ); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_number, 3); + assert.strictEqual(output.directory, '.planning/phases/03-root-next'); + assert.ok( + !fs.existsSync(path.join(repoDir, '.planning', 'workstreams', ' ')), + 'an effectively-empty env value must not mint a whitespace-named workstream', + ); + }); + test('phase next-decimal --ws is unaffected (planningDir-scoped already)', () => { const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4225-dec-')); activeDirs.push(repoDir); @@ -6203,6 +6225,22 @@ describe('#2028 — phase complete milestone-end + workstream guard', () => { assert.match(result.error || '', /workstream|--ws/i, 'error should name the workstream requirement'); }); + test('refuses to write root when GSD_WORKSTREAM is whitespace only', (t) => { + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'alpha'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '# Roadmap\n\n### Phase 1: A\n**Goal:** x\n'); + const previous = process.env.GSD_WORKSTREAM; + t.after(() => { + if (previous === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = previous; + }); + process.env.GSD_WORKSTREAM = ' \t '; + + const result = runGsdTools('phase complete 1', tmpDir); + assert.equal(result.success, false, 'a whitespace workstream must not write the shared root'); + assert.match(result.error || '', /workstream|--ws/i); + }); + // An explicit --ws satisfies the guard (it sets GSD_WORKSTREAM upstream) AND // targets that workstream — the write must land in the workstream's own // STATE.md/ROADMAP.md, leaving root untouched. diff --git a/tests/planning-workspace.test.cjs b/tests/planning-workspace.test.cjs index 4de0e71f7..18e3cb605 100644 --- a/tests/planning-workspace.test.cjs +++ b/tests/planning-workspace.test.cjs @@ -1,5 +1,6 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); +const fc = require('fast-check'); const fs = require('fs'); const os = require('os'); const path = require('path'); @@ -53,6 +54,56 @@ describe('planning-workspace: planningDir/planningPaths parity', () => { assert.throws(() => planningDir(cwd, 'foo/bar', null), /invalid path characters/); assert.throws(() => planningDir(cwd, 'foo\\bar', null), /invalid path characters/); }); + + test('normalizes whitespace-only and padded environment scope names (#4462)', () => { + for (const whitespace of [' ', '\t', '\n', ' \t\n ']) { + process.env.GSD_WORKSTREAM = whitespace; + process.env.GSD_PROJECT = whitespace; + assert.strictEqual(planningDir(cwd), path.join(cwd, '.planning')); + } + + process.env.GSD_WORKSTREAM = ' feature-x '; + process.env.GSD_PROJECT = ' my-app '; + assert.strictEqual( + planningDir(cwd), + path.join(cwd, '.planning', 'my-app', 'workstreams', 'feature-x'), + ); + }); + + test('normalizes explicit project and workstream arguments before routing (#4462)', () => { + assert.strictEqual(planningDir(cwd, '\t', '\n'), path.join(cwd, '.planning')); + assert.strictEqual( + planningDir(cwd, ' feature-x ', ' my-app '), + path.join(cwd, '.planning', 'my-app', 'workstreams', 'feature-x'), + ); + }); + + test('normalizes arbitrary whitespace-padded environment workstream names idempotently (#4462)', () => { + const whitespace = fc.array(fc.constantFrom(' ', '\t', '\n', '\r'), { maxLength: 8 }) + .map((chars) => chars.join('')); + const segment = fc.array(fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz0123456789_-'), { + minLength: 1, + maxLength: 24, + }).map((chars) => chars.join('')); + + fc.assert(fc.property(whitespace, segment, whitespace, (leading, name, trailing) => { + process.env.GSD_WORKSTREAM = `${leading}${name}${trailing}`; + assert.strictEqual( + planningDir(cwd), + path.join(cwd, '.planning', 'workstreams', name), + 'the environment value must resolve exactly as its trimmed form', + ); + })); + + fc.assert(fc.property(whitespace, (value) => { + process.env.GSD_WORKSTREAM = value; + assert.strictEqual( + planningDir(cwd), + path.join(cwd, '.planning'), + 'an all-whitespace environment value must be indistinguishable from unset', + ); + })); + }); }); describe('planning-workspace: session adapter precedence', () => {