diff --git a/docs/readme-reference.md b/docs/readme-reference.md index 332e70c8e..10af0d5e1 100644 --- a/docs/readme-reference.md +++ b/docs/readme-reference.md @@ -1206,6 +1206,13 @@ node --experimental-strip-types --check extensions/startup-banner.ts npm pack --dry-run ``` +`pnpm test` isolates the `unit-tests` stage from inherited `GIT_*` and +`GENTLE_PI_AGENTS_*` variables and supplies an empty temporary +`GENTLE_PI_CONFIG_HOME`, removed when the stage exits. Other environment +variables and the provider-contract/runtime-harness stages remain unchanged. +Direct `node --test` invocations bypass this isolation. Product Git-environment +safety checks are unchanged. + ### Cross-lane checks `tests/crosslane/cross-lane.mjs` (run with `pnpm test:cross-lane`) is a single fixture parity check, not a live battery. It imports `decodeReviewLastEventClosureV1` from the pinned decoder lane, decodes the vendored fixture `tests/fixtures/devbinary/last-event-capture-result-approved.captured.json`, asserts the approved `review/capture-result` closure shape (operation, state, and the `sha256:` store revision), and exits. It needs no `gentle-ai` binary and runs offline; the pinned decoder lane only ever sees vendored fixtures. diff --git a/scripts/run-test-suite.mjs b/scripts/run-test-suite.mjs index 52909f652..c92ec6794 100644 --- a/scripts/run-test-suite.mjs +++ b/scripts/run-test-suite.mjs @@ -10,7 +10,9 @@ // POSIX and Windows CI. import { spawn } from "node:child_process"; -import { readFileSync, realpathSync } from "node:fs"; +import { mkdtempSync, readFileSync, realpathSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { fileURLToPath } from "node:url"; export const DEFAULT_STAGES = Object.freeze([ @@ -20,14 +22,26 @@ export const DEFAULT_STAGES = Object.freeze([ ]); export async function runStage(stage) { - return await new Promise((resolve) => { - const child = spawn(stage.command, { shell: true, stdio: "inherit" }); - child.on("close", (code) => resolve({ name: stage.name, code: code ?? 1 })); - child.on("error", (error) => { - console.error(`\n[${stage.name}] spawn failed: ${error.message}`); - resolve({ name: stage.name, code: 1 }); + // Unit fixtures must not inherit the terminal's Git overrides, child-session + // identity, or personal guardrails. Other stages retain their existing env. + const configHome = stage.name === "unit-tests" ? mkdtempSync(join(tmpdir(), "gentle-pi-unit-config-")) : undefined; + const env = configHome ? Object.fromEntries(Object.entries(process.env).filter(([key]) => { + const normalizedKey = key.toUpperCase(); + return !normalizedKey.startsWith("GIT_") && !normalizedKey.startsWith("GENTLE_PI_AGENTS_") && normalizedKey !== "GENTLE_PI_CONFIG_HOME"; + })) : process.env; + if (configHome) env.GENTLE_PI_CONFIG_HOME = configHome; + try { + return await new Promise((resolve) => { + const child = spawn(stage.command, { shell: true, stdio: "inherit", env }); + child.on("close", (code) => resolve({ name: stage.name, code: code ?? 1 })); + child.on("error", (error) => { + console.error(`\n[${stage.name}] spawn failed: ${error.message}`); + resolve({ name: stage.name, code: 1 }); + }); }); - }); + } finally { + if (configHome) rmSync(configHome, { recursive: true, force: true }); + } } export async function runTestSuite(stages = DEFAULT_STAGES, { runStageImpl = runStage, write = (line) => console.log(line) } = {}) { diff --git a/tests/run-test-suite.test.ts b/tests/run-test-suite.test.ts index 76befadad..432eb79bd 100644 --- a/tests/run-test-suite.test.ts +++ b/tests/run-test-suite.test.ts @@ -1,16 +1,14 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import test from "node:test"; import { DEFAULT_STAGES, runTestSuite } from "../scripts/run-test-suite.mjs"; -// `pnpm test` chains its three stages; #1285 requires that a stage-1 failure -// never suppresses the later stages. These tests cover the runner's -// orchestration contract with fake stage implementations, so no real test -// process is spawned. +// #1285 requires that a stage-1 failure never suppresses later stages. +// Orchestration tests use fake stages; CLI tests exercise real child processes. function fakeRunStage(codes) { let index = 0; @@ -113,6 +111,86 @@ test("direct invocation still runs stages when reached through a file symlink", assert.match(result.stdout ?? "", /all stages passed/); }); +function probeStageEnvironment(t: test.TestContext, env: NodeJS.ProcessEnv) { + const stagesPath = writeStagesFile(t, []); + const probePath = join(dirname(stagesPath), "probe.mjs"); + writeFileSync(probePath, ` +import { existsSync, readdirSync } from "node:fs"; +const home = process.env.GENTLE_PI_CONFIG_HOME; +console.log("ENV_PROBE=" + JSON.stringify({ + git: Object.fromEntries(Object.entries(process.env).filter(([key]) => key.toUpperCase().startsWith("GIT_"))), + agents: Object.fromEntries(Object.entries(process.env).filter(([key]) => key.toUpperCase().startsWith("GENTLE_PI_AGENTS_"))), + home, + files: home && existsSync(home) ? readdirSync(home) : null, + sentinel: process.env.TEST_ENV_SENTINEL, +})); +`); + writeFileSync(stagesPath, JSON.stringify([ + { name: "unit-tests", command: `node "${probePath}"` }, + { name: "provider-contract", command: `node "${probePath}"` }, + ])); + const result = spawnSync(process.execPath, [runnerPath, stagesPath], { + encoding: "utf8", env: { ...process.env, ...env, TEST_ENV_SENTINEL: "preserved" }, + }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stderr, ""); + assert.match(result.stdout, /PASS unit-tests/); + assert.match(result.stdout, /PASS provider-contract/); + const probes = result.stdout.split(/\r?\n/).filter((line) => line.startsWith("ENV_PROBE=")) + .map((line) => JSON.parse(line.slice("ENV_PROBE=".length))); + assert.equal(probes.length, 2); + for (const probe of probes) assert.equal(probe.sentinel, "preserved"); + return probes; +} + +test("unit stage strips inherited Git configuration without changing later stages", (t) => { + const env = { + GIT_CONFIG_COUNT: "2", + GIT_CONFIG_KEY_0: "credential.interactive", GIT_CONFIG_VALUE_0: "false", + GIT_CONFIG_KEY_1: "credential.guiPrompt", GIT_CONFIG_VALUE_1: "false", + GIT_DIR: "inherited-repository", GIT_AUTHOR_NAME: "inherited-author", + }; + const [unit, provider] = probeStageEnvironment(t, env); + assert.deepEqual(unit.git, {}); + for (const [key, value] of Object.entries(env)) assert.equal(provider.git[key], value); +}); + +test("unit stage strips inherited subagent context without changing later stages", (t) => { + const env = { GENTLE_PI_AGENTS_CHILD: "1", GENTLE_PI_AGENTS_OWNED_IPC: "1" }; + const [unit, provider] = probeStageEnvironment(t, env); + assert.deepEqual(unit.agents, {}); + for (const [key, value] of Object.entries(env)) assert.equal(provider.agents[key], value); +}); + +test("unit stage accepts an uncreated inherited config home without creating it", (t) => { + const dir = mkdtempSync(join(tmpdir(), "gentle-pi-caller-config-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const home = join(dir, "not-created"); + assert.equal(existsSync(home), false); + const [unit, provider] = probeStageEnvironment(t, { GENTLE_PI_CONFIG_HOME: home }); + assert.notEqual(unit.home, home); + assert.deepEqual(unit.files, []); + assert.equal(existsSync(unit.home), false); + assert.equal(provider.home, home); + assert.equal(provider.files, null); + assert.equal(existsSync(home), false); +}); + +test("unit stage uses an empty temporary config home and removes it after exit", (t) => { + const home = mkdtempSync(join(tmpdir(), "gentle-pi-caller-config-")); + t.after(() => rmSync(home, { recursive: true, force: true })); + const configPath = join(home, "runtime-guardrails.json"); + const config = JSON.stringify({ autonomousMode: true, guardedCommands: { gitPush: "allow", gitRebase: "allow", gitBranchDeleteForce: "allow" } }); + writeFileSync(configPath, config); + const [unit, provider] = probeStageEnvironment(t, { GENTLE_PI_CONFIG_HOME: home }); + assert.notEqual(unit.home, home); + assert.deepEqual(unit.files, []); + assert.equal(existsSync(unit.home), false); + assert.equal(provider.home, home); + assert.deepEqual(provider.files, ["runtime-guardrails.json"]); + assert.equal(readFileSync(configPath, "utf8"), config); +}); + test("direct invocation rejects a malformed stages file with a non-zero exit", (t) => { const dir = mkdtempSync(join(tmpdir(), "gentle-pi-run-test-suite-")); t.after(() => rmSync(dir, { recursive: true, force: true }));