From 1f6d827e481deb625959c4856d7a4a33cb9eb4eb Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Sat, 1 Aug 2026 02:02:18 -0500 Subject: [PATCH] fix(#2665): scrub config-location env in the #2624 in-process install block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-found during the rebase onto next, not from the review. The base range added `describe('#2624 .gsd-source marker is rewritten before staging reads it')`, which calls the real `install(true, 'claude')` IN-PROCESS and sandboxes HOME/USERPROFILE/GSD_EXPLICIT_CONFIG_DIR — but not CLAUDE_CONFIG_DIR. That is exactly the Blocker-1 shape this PR exists to close, reintroduced in new code written after the round-1 review. Measured on the rebased tree, same file, same commit: CLAUDE_CONFIG_DIR unset -> 50/50 pass CLAUDE_CONFIG_DIR set -> 47/50, and a COMPLETE global install lands in it (gsd-core/, agents/, skills/, hooks/, scripts/, gsd-file-manifest.json, gsd-install-state.json, .gsd-source, .gsd-profile) The three failures are the honest symptom rather than the problem: the install goes to the ambient config dir, so the assertions look for a marker under tmpRoot that was never written there. With scrubConfigLocationEnv() wired into the block's beforeEach/afterEach — the same pattern the sibling block at :753 already uses — the file is 50/50 under BOTH conditions and leaks zero entries. Worth stating plainly: CI cannot catch this class, since CI never has these vars set. It surfaced here only because the rebase brought the base's new tests under an ambient CLAUDE_CONFIG_DIR, which is the condition #2665's own acceptance criterion runs under. --- tests/runtime-artifact-layout.test.cjs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/runtime-artifact-layout.test.cjs b/tests/runtime-artifact-layout.test.cjs index caae32441..20c79c1ee 100644 --- a/tests/runtime-artifact-layout.test.cjs +++ b/tests/runtime-artifact-layout.test.cjs @@ -946,6 +946,7 @@ describe('#2624 .gsd-source marker is rewritten before staging reads it', () => let savedUserProfile; let savedExplicitConfigDir; let savedTestMode; + let restoreConfigLocationEnv; // Run install() with process.exit and console output mocked via t.mock (auto-restored), // per CONTRIBUTING.md test rules (no manual monkeypatch / try-finally in test bodies). @@ -970,6 +971,13 @@ describe('#2624 .gsd-source marker is rewritten before staging reads it', () => delete process.env.GSD_EXPLICIT_CONFIG_DIR; savedTestMode = process.env.GSD_TEST_MODE; process.env.GSD_TEST_MODE = '1'; + // #2665: same in-process hazard as the block above. This one calls + // install(true, 'claude') via runInstall(), and HOME/USERPROFILE alone do + // not contain it — getGlobalConfigDir is env-FIRST, so an ambient + // CLAUDE_CONFIG_DIR beats the fixture, the install lands in the developer's + // live config dir, and these tests then fail looking for a marker under + // tmpRoot that was never written there. + restoreConfigLocationEnv = scrubConfigLocationEnv(); }); afterEach(() => { @@ -981,6 +989,7 @@ describe('#2624 .gsd-source marker is rewritten before staging reads it', () => else process.env.GSD_EXPLICIT_CONFIG_DIR = savedExplicitConfigDir; if (savedTestMode === undefined) delete process.env.GSD_TEST_MODE; else process.env.GSD_TEST_MODE = savedTestMode; + restoreConfigLocationEnv(); cleanup(tmpRoot); });