From a7b288b9dffd44e1525fb0cde7e94b0f49fc0597 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 14:04:26 -0400 Subject: [PATCH] fix(#866): route profile-pipeline temp output under reaped root + fixture teardown (#879) Closes #866 --- .changeset/866-profile-pipeline-temp-root.md | 5 + src/profile-pipeline.cts | 8 +- ...ug-866-profile-pipeline-temp-root.test.cjs | 140 ++++++++++++++++++ tests/changeset-github-release-notes.test.cjs | 14 +- tests/enh-2415-claude-md-link-mode.test.cjs | 7 +- tests/enh-2446-milestones-drift.test.cjs | 7 +- tests/enh-2448-artifact-registry.test.cjs | 7 +- 7 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 .changeset/866-profile-pipeline-temp-root.md create mode 100644 tests/bug-866-profile-pipeline-temp-root.test.cjs diff --git a/.changeset/866-profile-pipeline-temp-root.md b/.changeset/866-profile-pipeline-temp-root.md new file mode 100644 index 000000000..59584d4f7 --- /dev/null +++ b/.changeset/866-profile-pipeline-temp-root.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 879 +--- +**profile-pipeline temp output now lands under the reaped GSD temp root.** `cmdExtractMessages` and `cmdProfileSample` previously created their output directories directly in `os.tmpdir()` root (`gsd-pipeline-*` / `gsd-profile-*`), which `reapStaleTempFiles` never scans (it only scans `GSD_TEMP_DIR = os.tmpdir()/gsd`). The directories accumulated forever. Both sites now call `ensureGsdTempDir()` and create under `GSD_TEMP_DIR`. Also adds missing `after`/`afterEach` teardown to four test fixtures that leaked `gsd-*` temp dirs on every `npm test` run. (#866) diff --git a/src/profile-pipeline.cts b/src/profile-pipeline.cts index 7de1b232f..0a7b84655 100644 --- a/src/profile-pipeline.cts +++ b/src/profile-pipeline.cts @@ -18,7 +18,7 @@ import os from 'node:os'; import readline from 'node:readline'; // eslint-disable-next-line @typescript-eslint/no-require-imports import ioModule = require('./io.cjs'); -const { output, error, reapStaleTempFiles } = ioModule; +const { output, error, reapStaleTempFiles, ensureGsdTempDir, GSD_TEMP_DIR } = ioModule; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -413,7 +413,8 @@ async function cmdExtractMessages(projectArg: string, options: { sessionId?: str } reapStaleTempFiles('gsd-pipeline-', { dirsOnly: true }); - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-pipeline-')); + ensureGsdTempDir(); + const tmpDir = fs.mkdtempSync(path.join(GSD_TEMP_DIR, 'gsd-pipeline-')); const outputPath = path.join(tmpDir, 'extracted-messages.jsonl'); let sessionsProcessed = 0; @@ -601,7 +602,8 @@ async function cmdProfileSample(overridePath: string | null | undefined, options } reapStaleTempFiles('gsd-profile-', { dirsOnly: true }); - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-')); + ensureGsdTempDir(); + const tmpDir = fs.mkdtempSync(path.join(GSD_TEMP_DIR, 'gsd-profile-')); const outputPath = path.join(tmpDir, 'profile-sample.jsonl'); for (const msg of allMessages) { fs.appendFileSync(outputPath, JSON.stringify(msg) + '\n'); diff --git a/tests/bug-866-profile-pipeline-temp-root.test.cjs b/tests/bug-866-profile-pipeline-temp-root.test.cjs new file mode 100644 index 000000000..ac3ef5d66 --- /dev/null +++ b/tests/bug-866-profile-pipeline-temp-root.test.cjs @@ -0,0 +1,140 @@ +'use strict'; +/** + * Regression test for bug #866: profile-pipeline temp output dirs must be + * created under GSD_TEMP_DIR (path.join(os.tmpdir(), 'gsd')), not directly + * under os.tmpdir() root where reapStaleTempFiles() never scans. + * + * Hardening (adversarial-review follow-up): + * - TMPDIR/TEMP/TMP are redirected to a fixture-scoped directory so the child + * process's os.tmpdir() returns an isolated root. This prevents the test from + * touching the real shared temp root and keeps it out of the production + * reaper's view. + * - Both sides of the startsWith assertion are realpath-normalized to kill the + * macOS /var ↔ /private/var symlink flakiness. + * - An explicit exitCode === 0 assertion is added before JSON.parse so a + * non-zero early-exit produces a clear failure rather than a confusing parse + * error. + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempDir, cleanup } = require('./helpers.cjs'); + +describe('bug-866: profile-pipeline temp dirs under GSD_TEMP_DIR', () => { + let tmpDir; + let isolatedSysTmp; + + beforeEach(() => { + tmpDir = createTempDir('gsd-866-'); + // Create an isolated os.tmpdir() root inside the fixture so the child + // process never writes to the real shared temp dir. + isolatedSysTmp = path.join(tmpDir, 'systmp'); + fs.mkdirSync(isolatedSysTmp, { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // Helper: create a minimal synthetic sessions directory structure + function createSessions(root) { + const sessionsDir = path.join(root, 'projects'); + const projectDir = path.join(sessionsDir, 'test-project-866'); + fs.mkdirSync(projectDir, { recursive: true }); + const messages = [ + { type: 'user', userType: 'external', message: { content: 'fix the login bug' }, timestamp: Date.now() }, + { type: 'assistant', message: { content: 'Sure.' }, timestamp: Date.now() }, + ]; + fs.writeFileSync( + path.join(projectDir, 'session-001.jsonl'), + messages.map(m => JSON.stringify(m)).join('\n') + ); + return sessionsDir; + } + + test('extract-messages output_file is under GSD_TEMP_DIR, not os.tmpdir() root', () => { + const sessionsDir = createSessions(tmpDir); + + // Pass the isolated tmp root so the child's os.tmpdir() = isolatedSysTmp. + // Belt-and-suspenders: set all three env vars Node checks (TMPDIR=POSIX, + // TEMP+TMP=Windows). + const result = runGsdTools( + ['extract-messages', 'test-project-866', '--path', sessionsDir, '--raw'], + tmpDir, + { TMPDIR: isolatedSysTmp, TEMP: isolatedSysTmp, TMP: isolatedSysTmp } + ); + + // Explicit exitCode check first — parse errors are confusing on non-zero exit. + assert.strictEqual(result.exitCode, 0, `extract-messages must exit 0; error: ${result.error}`); + assert.ok(result.success, `extract-messages failed: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok(out.output_file, 'should have output_file in result'); + + const outputFile = out.output_file; + + // The expected GSD_TEMP_DIR from the child's perspective: isolatedSysTmp/gsd + const expectedGsdTempDir = path.join(isolatedSysTmp, 'gsd'); + + // Normalize both sides via realpath to kill macOS /var↔/private/var symlink + // flakiness. Realpath the existing output_file's parent directory (the file + // itself may have been cleaned up already, but the dir will exist). + const expectedRoot = fs.realpathSync(expectedGsdTempDir); + const outputDir = path.dirname(outputFile); + // The output dir must exist since the tool just wrote there; realpath it. + const actualDir = fs.realpathSync(outputDir); + + assert.ok( + actualDir.startsWith(expectedRoot + path.sep) || actualDir === expectedRoot, + `output_file "${outputFile}" must be under GSD_TEMP_DIR "${expectedGsdTempDir}" (realpath: ${expectedRoot}); got dir "${actualDir}"` + ); + + // Must NOT be directly under the isolated systmp root (i.e., no gsd-pipeline-* + // at depth 1 of isolatedSysTmp). + const rel = path.relative(isolatedSysTmp, outputFile); + const depth1Dir = rel.split(path.sep)[0]; + assert.ok( + !depth1Dir.startsWith('gsd-pipeline-'), + `output_file must not be in isolatedSysTmp/gsd-pipeline-* but got depth-1 dir: "${depth1Dir}"` + ); + }); + + test('profile-sample output_file is under GSD_TEMP_DIR, not os.tmpdir() root', () => { + const sessionsDir = createSessions(tmpDir); + + const result = runGsdTools( + ['profile-sample', '--path', sessionsDir, '--raw'], + tmpDir, + { TMPDIR: isolatedSysTmp, TEMP: isolatedSysTmp, TMP: isolatedSysTmp } + ); + + // Explicit exitCode check first. + assert.strictEqual(result.exitCode, 0, `profile-sample must exit 0; error: ${result.error}`); + assert.ok(result.success, `profile-sample failed: ${result.error}`); + + const out = JSON.parse(result.output); + assert.ok(out.output_file, 'should have output_file in result'); + + const outputFile = out.output_file; + + const expectedGsdTempDir = path.join(isolatedSysTmp, 'gsd'); + + const expectedRoot = fs.realpathSync(expectedGsdTempDir); + const outputDir = path.dirname(outputFile); + const actualDir = fs.realpathSync(outputDir); + + assert.ok( + actualDir.startsWith(expectedRoot + path.sep) || actualDir === expectedRoot, + `output_file "${outputFile}" must be under GSD_TEMP_DIR "${expectedGsdTempDir}" (realpath: ${expectedRoot}); got dir "${actualDir}"` + ); + + const rel = path.relative(isolatedSysTmp, outputFile); + const depth1Dir = rel.split(path.sep)[0]; + assert.ok( + !depth1Dir.startsWith('gsd-profile-'), + `output_file must not be in isolatedSysTmp/gsd-profile-* but got depth-1 dir: "${depth1Dir}"` + ); + }); +}); diff --git a/tests/changeset-github-release-notes.test.cjs b/tests/changeset-github-release-notes.test.cjs index ac7d59422..4ffc973e0 100644 --- a/tests/changeset-github-release-notes.test.cjs +++ b/tests/changeset-github-release-notes.test.cjs @@ -1,12 +1,13 @@ 'use strict'; process.env.GSD_TEST_MODE = '1'; -const { test, describe } = require('node:test'); +const { test, describe, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const cp = require('node:child_process'); +const helpers = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'cli.cjs'); @@ -63,8 +64,11 @@ function createTaggedRepo() { } describe('changeset github release notes: tag-range renderer (#3382)', () => { + let _repo; + afterEach(() => { helpers.cleanup(_repo); _repo = undefined; }); + test('loads changed changeset slugs from a git tag range', () => { - const repo = createTaggedRepo(); + const repo = (_repo = createTaggedRepo()); const result = loadFragmentsFromRange({ repo, fromRef: 'v1.0.0', toRef: 'v1.0.1' }); assert.deepEqual(result.failures, []); @@ -78,7 +82,7 @@ describe('changeset github release notes: tag-range renderer (#3382)', () => { }); test('builds grouped GitHub release-note IR from parsed fragments', () => { - const repo = createTaggedRepo(); + const repo = (_repo = createTaggedRepo()); const { fragments } = loadFragmentsFromRange({ repo, fromRef: 'v1.0.0', toRef: 'v1.0.1' }); const ir = buildGithubReleaseNotesIr({ fragments }); @@ -95,7 +99,7 @@ describe('changeset github release notes: tag-range renderer (#3382)', () => { }); test('CLI writes a notes file suitable for gh release edit --notes-file', () => { - const repo = createTaggedRepo(); + const repo = (_repo = createTaggedRepo()); const output = path.join(repo, 'release-notes.md'); const result = cp.spawnSync( process.execPath, @@ -130,7 +134,7 @@ describe('changeset github release notes: tag-range renderer (#3382)', () => { }); test('rejects unsafe git refs before rendering a range', () => { - const repo = createTaggedRepo(); + const repo = (_repo = createTaggedRepo()); assert.throws( () => loadFragmentsFromRange({ repo, fromRef: '--help', toRef: 'v1.0.1' }), /Invalid git ref/, diff --git a/tests/enh-2415-claude-md-link-mode.test.cjs b/tests/enh-2415-claude-md-link-mode.test.cjs index cb032ab13..4f293c3e3 100644 --- a/tests/enh-2415-claude-md-link-mode.test.cjs +++ b/tests/enh-2415-claude-md-link-mode.test.cjs @@ -10,16 +10,21 @@ * content when claude_md_assembly.mode is "link". */ -const { test } = require('node:test'); +const { test, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const helpers = require('./helpers.cjs'); const { cmdGenerateClaudeMd } = require('../gsd-core/bin/lib/profile-output.cjs'); +const _dirsToClean = []; +after(() => { for (const d of _dirsToClean) helpers.cleanup(d); }); + function makeTempProject(files = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2415-')); + _dirsToClean.push(dir); fs.mkdirSync(path.join(dir, '.planning', 'codebase'), { recursive: true }); for (const [rel, content] of Object.entries(files)) { const abs = path.join(dir, rel); diff --git a/tests/enh-2446-milestones-drift.test.cjs b/tests/enh-2446-milestones-drift.test.cjs index aba5d6100..4b3df5c77 100644 --- a/tests/enh-2446-milestones-drift.test.cjs +++ b/tests/enh-2446-milestones-drift.test.cjs @@ -8,16 +8,21 @@ * Tests for gsd-health MILESTONES.md drift detection (#2446). */ -const { test } = require('node:test'); +const { test, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const helpers = require('./helpers.cjs'); const { cmdValidateHealth } = require('../gsd-core/bin/lib/verify.cjs'); +const _dirsToClean = []; +after(() => { for (const d of _dirsToClean) helpers.cleanup(d); }); + function makeTempProject(files = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2446-')); + _dirsToClean.push(dir); fs.mkdirSync(path.join(dir, '.planning', 'milestones'), { recursive: true }); for (const [rel, content] of Object.entries(files)) { const abs = path.join(dir, rel); diff --git a/tests/enh-2448-artifact-registry.test.cjs b/tests/enh-2448-artifact-registry.test.cjs index 4bd07c1e8..d8e802343 100644 --- a/tests/enh-2448-artifact-registry.test.cjs +++ b/tests/enh-2448-artifact-registry.test.cjs @@ -8,17 +8,22 @@ * Tests for canonical artifact registry and gsd-health W019 lint (#2448). */ -const { test, describe } = require('node:test'); +const { test, describe, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const helpers = require('./helpers.cjs'); const { isCanonicalPlanningFile, CANONICAL_EXACT } = require('../gsd-core/bin/lib/artifacts.cjs'); const { cmdValidateHealth } = require('../gsd-core/bin/lib/verify.cjs'); +const _dirsToClean = []; +after(() => { for (const d of _dirsToClean) helpers.cleanup(d); }); + function makeTempProject(files = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2448-')); + _dirsToClean.push(dir); fs.mkdirSync(path.join(dir, '.planning', 'phases'), { recursive: true }); for (const [rel, content] of Object.entries(files)) { const abs = path.join(dir, rel);