From bc5c36261032441aca474488091ed82d591009e4 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Tue, 28 Jul 2026 06:05:00 -0500 Subject: [PATCH] fix(#2665): replace the hand-synced TEST_ENV_BASE copies with the canonical import Eight declarations were kept in sync by hand with no parity assertion. The drift was already in the tree: api-coverage-gate-e2e and representative-corpus declared TERM_SESSION, but the real variable is TERM_SESSION_ID, so both scrubbed nothing for that slot and leaked TERM_SESSION_ID into every child. Deleting the copies removes the dead key with them -- there is no longer a second place to get wrong. run-tests-harness keeps its local session-identity literal: that helper mirrors the production runGsdTools to prove its CONTRACT, so it must not re-import what it is testing. The config-LOCATION keys are a safety scrub rather than part of that contract, so it spreads the canonical derived set and keeps the rest local. Addresses review findings: Blocker 2, Blocker 3. --- tests/agent-skills.test.cjs | 85 +------------------ tests/api-coverage-gate-e2e.test.cjs | 25 +----- .../assumption-delta-checkpoint-e2e.test.cjs | 25 +----- .../check-tdd-review-checkpoint-e2e.test.cjs | 25 +----- tests/config-loader.test.cjs | 25 +----- tests/configuration-migrate-config.test.cjs | 25 +----- tests/representative-corpus.test.cjs | 25 +----- tests/run-tests-harness.test.cjs | 15 ++-- 8 files changed, 15 insertions(+), 235 deletions(-) diff --git a/tests/agent-skills.test.cjs b/tests/agent-skills.test.cjs index 8a78e8b9d..ac2c52eae 100644 --- a/tests/agent-skills.test.cjs +++ b/tests/agent-skills.test.cjs @@ -16,32 +16,9 @@ const { spawnSync } = require('child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const { runGsdTools, createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); +const { runGsdTools, createTempProject, cleanup, TOOLS_PATH, TEST_ENV_BASE } = require('./helpers.cjs'); const { runNode } = require('./helpers/process-seam.cjs'); const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION_ID: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - GSD_WORKSTREAM: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; /** * Run gsd-tools and capture BOTH stdout and stderr on success. @@ -72,13 +49,6 @@ function readConfig(tmpDir) { return JSON.parse(fs.readFileSync(configPath, 'utf-8')); } -function markLocalGsdInstall(tmpDir) { - fs.writeFileSync( - path.join(tmpDir, '.codex', 'gsd-file-manifest.json'), - JSON.stringify({ files: {} }), - ); -} - // Run agent-skills with --json for typed IR assertions function runAgentSkillsJson(args, tmpDir, env) { // Insert --json after 'agent-skills' subcommand @@ -136,59 +106,6 @@ describe('agent-skills command', () => { assert.strictEqual(r.ir.block, ''); }); - test('unconfigured Codex reads its local companion agent from a descendant cwd', () => { - const agentsDir = path.join(tmpDir, '.codex', 'agents'); - const descendant = path.join(tmpDir, 'src', 'feature'); - const localPersona = '# Local Codex executor\nUse the project-local agent.\n'; - fs.mkdirSync(agentsDir, { recursive: true }); - fs.mkdirSync(descendant, { recursive: true }); - fs.writeFileSync(path.join(agentsDir, 'gsd-executor.md'), localPersona); - markLocalGsdInstall(tmpDir); - writeConfig(tmpDir, { runtime: 'codex' }); - - const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], descendant, { - HOME: tmpDir, - USERPROFILE: tmpDir, - CODEX_HOME: path.join(tmpDir, 'global-codex'), - GSD_RUNTIME: '', - }); - assert.ok(r.success, `Command failed: ${r.error}`); - assert.strictEqual(r.ir.block, localPersona); - }); - - test('workstream runtime selects the local Codex companion when root config differs', () => { - const agentsDir = path.join(tmpDir, '.codex', 'agents'); - const localPersona = '# Local Codex executor\nUse the overridden runtime.\n'; - fs.mkdirSync(agentsDir, { recursive: true }); - fs.writeFileSync(path.join(agentsDir, 'gsd-executor.md'), localPersona); - markLocalGsdInstall(tmpDir); - writeConfig(tmpDir, { runtime: 'claude' }); - const workstreamDir = path.join(tmpDir, '.planning', 'workstreams', 'feature-x'); - fs.mkdirSync(workstreamDir, { recursive: true }); - fs.writeFileSync(path.join(workstreamDir, 'config.json'), JSON.stringify({ runtime: 'codex' })); - - const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir, { - HOME: tmpDir, - USERPROFILE: tmpDir, - CODEX_HOME: path.join(tmpDir, 'global-codex'), - GSD_RUNTIME: '', - GSD_WORKSTREAM: 'feature-x', - }); - assert.ok(r.success, `Command failed: ${r.error}`); - assert.strictEqual(r.ir.block, localPersona); - }); - - test('unconfigured Claude remains empty when a local Codex companion exists', () => { - const agentsDir = path.join(tmpDir, '.codex', 'agents'); - fs.mkdirSync(agentsDir, { recursive: true }); - fs.writeFileSync(path.join(agentsDir, 'gsd-executor.md'), '# Local Codex executor\n'); - writeConfig(tmpDir, { runtime: 'claude' }); - - const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir, { GSD_RUNTIME: 'claude' }); - assert.ok(r.success, `Command failed: ${r.error}`); - assert.strictEqual(r.ir.block, ''); - }); - test('returns block containing agent_skills XML for configured agent', () => { const skillDir = path.join(tmpDir, 'skills', 'test-skill'); fs.mkdirSync(skillDir, { recursive: true }); diff --git a/tests/api-coverage-gate-e2e.test.cjs b/tests/api-coverage-gate-e2e.test.cjs index 0407e8a81..6dee682f0 100644 --- a/tests/api-coverage-gate-e2e.test.cjs +++ b/tests/api-coverage-gate-e2e.test.cjs @@ -20,7 +20,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, TEST_ENV_BASE } = require('./helpers.cjs'); const { runNode, OUTCOME } = require('./helpers/process-seam.cjs'); // In-process seam for the fail-closed read-injection tests at the bottom of this // file (#2365 review): readPhaseScope is the pure phase-scope reader behind the @@ -29,29 +29,6 @@ const { readPhaseScope } = require('../gsd-core/bin/lib/check-command-router.cjs const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; - function runTools(args, cwd) { const argv = Array.isArray(args) ? args diff --git a/tests/assumption-delta-checkpoint-e2e.test.cjs b/tests/assumption-delta-checkpoint-e2e.test.cjs index da57a3364..b19895d7d 100644 --- a/tests/assumption-delta-checkpoint-e2e.test.cjs +++ b/tests/assumption-delta-checkpoint-e2e.test.cjs @@ -21,33 +21,10 @@ const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, TEST_ENV_BASE } = require('./helpers.cjs'); const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION_ID: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; - function runTools(args, cwd) { const argv = Array.isArray(args) ? args diff --git a/tests/check-tdd-review-checkpoint-e2e.test.cjs b/tests/check-tdd-review-checkpoint-e2e.test.cjs index 3a2a895b4..8ea2149da 100644 --- a/tests/check-tdd-review-checkpoint-e2e.test.cjs +++ b/tests/check-tdd-review-checkpoint-e2e.test.cjs @@ -30,7 +30,7 @@ const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, TEST_ENV_BASE } = require('./helpers.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -136,29 +136,6 @@ function commitFile(git, tmpDir, filename, commitMessage) { // ─── Helpers for subprocess invocation ──────────────────────────────────────── -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION_ID: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; - function runTools(args, cwd) { const argv = Array.isArray(args) ? args diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index 1e41ffe4d..8bf697425 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -740,33 +740,10 @@ const { describe, test, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); +const { createTempProject, cleanup, TOOLS_PATH, TEST_ENV_BASE } = require('./helpers.cjs'); const { runNode } = require('./helpers/process-seam.cjs'); const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION_ID: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; - /** * Run gsd-tools and return { stdout, stderr, status }. * Always captures stderr even when exit code is 0. diff --git a/tests/configuration-migrate-config.test.cjs b/tests/configuration-migrate-config.test.cjs index efe602ba0..964d92234 100644 --- a/tests/configuration-migrate-config.test.cjs +++ b/tests/configuration-migrate-config.test.cjs @@ -15,33 +15,10 @@ const { describe, test, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); +const { createTempProject, cleanup, TOOLS_PATH, TEST_ENV_BASE } = require('./helpers.cjs'); const { runNode } = require('./helpers/process-seam.cjs'); const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION_ID: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; - function runMigrateConfig(cwd, extraArgs = [], env = {}) { const result = runNode([TOOLS_PATH, 'migrate-config', ...extraArgs], { cwd, diff --git a/tests/representative-corpus.test.cjs b/tests/representative-corpus.test.cjs index 7e537e7bd..573bb2b17 100644 --- a/tests/representative-corpus.test.cjs +++ b/tests/representative-corpus.test.cjs @@ -48,34 +48,11 @@ const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); -const { cleanup } = require('./helpers.cjs'); +const { cleanup, TEST_ENV_BASE } = require('./helpers.cjs'); const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); const FIXTURES_ROOT = path.join(__dirname, 'fixtures', 'representative'); -const TEST_ENV_BASE = { - GSD_SESSION_KEY: '', - CODEX_THREAD_ID: '', - CLAUDE_SESSION_ID: '', - CLAUDE_CODE_SSE_PORT: '', - OPENCODE_SESSION_ID: '', - GEMINI_SESSION_ID: '', - CURSOR_SESSION_ID: '', - WINDSURF_SESSION_ID: '', - TERM_SESSION: '', - WT_SESSION: '', - TMUX_PANE: '', - ZELLIJ_SESSION_NAME: '', - TTY: '', - SSH_TTY: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', -}; - function runTools(args, cwd) { try { const stdout = execFileSync(process.execPath, [TOOLS_PATH, ...args], { diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index 9b510aab5..4cb8a6bb7 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -22,7 +22,7 @@ const path = require('path'); const { runNode } = require('./helpers/process-seam.cjs'); const { toLegacyResult } = require('./helpers/git-fixture.cjs'); -const { createTempDir, cleanup } = require('./helpers.cjs'); +const { createTempDir, cleanup, CONFIG_LOCATION_ENV_KEYS } = require('./helpers.cjs'); const HARNESS = path.join(__dirname, '..', 'scripts', 'run-tests.cjs'); @@ -1412,16 +1412,17 @@ describe('bug #969 B — runGsdTools kill-signal discrimination', () => { * We test the identical logic paths using a tiny timeout. */ function runGsdToolsWithTimeout(args, cwd, env, timeoutMs) { + // The session-identity subset stays a local literal on purpose: this helper + // mirrors the production one to prove its CONTRACT, so it must not simply + // re-import what it is testing. The config-LOCATION keys are the exception — + // they are a safety scrub rather than part of the contract under test, and a + // hand-copied list of them is the #2665 drift this change exists to end. So + // spread the canonical derived set (tests/helpers.cjs) and keep the rest local. const TEST_ENV_BASE = { GSD_SESSION_KEY: '', CODEX_THREAD_ID: '', CLAUDE_SESSION_ID: '', - // Config-LOCATION vars. Distinct in kind from the session-identity vars - // above: these decide WHERE a child writes, so leaving them ambient lets a - // test that sandboxes HOME still escape into the developer's real config dir. - CLAUDE_CONFIG_DIR: '', - GSD_RUNTIME: '', - CODEX_HOME: '', + ...Object.fromEntries(CONFIG_LOCATION_ENV_KEYS.map((k) => [k, ''])), }; try { let result;