Two tests for round 3's change (round 2 Blockers 1 and 2): KIMI_SHARE_DIR must arrive via NON_REGISTRY_CONFIG_HOME_DESCRIPTORS rather than a literal, and every GSD_LOCATION_ENV_KEYS entry must be blanked. Both assert a floor on their source first, so a renamed export fails loudly instead of passing vacuously. Negative-controlled against the registry-only derivation: both fail there, and only those two. And the Nit, which is the more useful half. This block asserts that TEST_ENV_BASE is not narrower than the enumerations it derives from. It cannot prove those enumerations are complete — a var no enumeration carries is invisible to every test here, and they stay green. That is exactly how round 2 found GSD_HOME and KIMI_SHARE_DIR while this block was fully green: one belonged to no enumeration at all, the other sat inside a function body where nothing could enumerate it. So the scope boundary is now written down at the top of the block, naming where the completeness question is actually answered — a source census re-derived each round, and live-config-guard.cjs observing real writes at runtime — so that a green run here is not misread as "the set is exhaustive." Deliberately NOT added: an assertion per reviewer-named variable. That is the hand-maintained list wearing a test's clothes, and it fails the same way.
194 lines
8.4 KiB
JavaScript
194 lines
8.4 KiB
JavaScript
const { test, describe } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const path = require('node:path');
|
|
|
|
const {
|
|
withIsolatedProcessState,
|
|
TEST_ENV_BASE,
|
|
CONFIG_LOCATION_ENV_KEYS,
|
|
scrubConfigLocationEnv,
|
|
} = require('./helpers.cjs');
|
|
|
|
describe('withIsolatedProcessState', () => {
|
|
test('restores env, cwd, and exitCode after callback', () => {
|
|
const originalCwd = process.cwd();
|
|
const originalExitCode = process.exitCode;
|
|
const originalMarker = process.env.GSD_TEST_ISOLATION_MARKER;
|
|
|
|
const tempCwd = path.dirname(originalCwd);
|
|
|
|
withIsolatedProcessState(() => {
|
|
process.env.GSD_TEST_ISOLATION_MARKER = 'changed';
|
|
process.exitCode = 73;
|
|
process.chdir(tempCwd);
|
|
});
|
|
|
|
assert.strictEqual(process.cwd(), originalCwd);
|
|
assert.strictEqual(process.exitCode, originalExitCode);
|
|
assert.strictEqual(process.env.GSD_TEST_ISOLATION_MARKER, originalMarker);
|
|
});
|
|
|
|
test('restores state even when callback throws', () => {
|
|
const originalCwd = process.cwd();
|
|
const originalPath = process.env.PATH;
|
|
|
|
assert.throws(() => {
|
|
withIsolatedProcessState(() => {
|
|
process.env.PATH = '';
|
|
process.chdir(path.dirname(originalCwd));
|
|
throw new Error('boom');
|
|
});
|
|
}, /boom/);
|
|
|
|
assert.strictEqual(process.cwd(), originalCwd);
|
|
assert.strictEqual(process.env.PATH, originalPath);
|
|
});
|
|
});
|
|
|
|
// ─── #2665: the config-location scrub is DERIVED, and stays that way ──────────
|
|
//
|
|
// The recurrence guard. #2665 documents two prior authors independently
|
|
// diagnosing this class and each fixing only the instance in front of them;
|
|
// this is the third pass. A hand-maintained scrub list cannot be defended by
|
|
// review alone, so the invariant is asserted instead of trusted.
|
|
//
|
|
// SCOPE BOUNDARY — read this before trusting a green run here.
|
|
//
|
|
// Every test below asserts that TEST_ENV_BASE covers some ENUMERATION (the
|
|
// capability registry, the non-registry descriptor set, GSD's own location
|
|
// keys). Each therefore proves only that the scrub set is not narrower than the
|
|
// enumeration it derives from. NONE of them can prove the enumeration is itself
|
|
// complete: a config-location var that no enumeration carries is invisible to
|
|
// all of them, and they stay green.
|
|
//
|
|
// That is not hypothetical — it is how round 2 found GSD_HOME and
|
|
// KIMI_SHARE_DIR while this block was fully green. GSD_HOME belonged to no
|
|
// enumeration at all (it is GSD's own store root, not a runtime configHome);
|
|
// KIMI_SHARE_DIR sat inside a function body where nothing could enumerate it.
|
|
// Round 3's fix was to make both enumerable rather than to add two assertions,
|
|
// precisely because an assertion added per reviewer-named var is the
|
|
// hand-maintained list wearing a test's clothes.
|
|
//
|
|
// The completeness question — "is every env-first first-party location var in
|
|
// SOME enumeration?" — is answered by a source census re-derived each round
|
|
// (see the PR discussion), and by scripts/live-config-guard.cjs at runtime,
|
|
// which observes actual writes rather than reasoning about names. Neither lives
|
|
// here, and this block should not be read as standing in for them.
|
|
describe('#2665: TEST_ENV_BASE config-location coverage', () => {
|
|
test('every runtime configHome env var in the registry is scrubbed', () => {
|
|
const { runtimes } = require('../gsd-core/bin/lib/capability-registry.cjs');
|
|
|
|
const declared = [
|
|
...new Set(
|
|
Object.values(runtimes).flatMap((r) => r?.runtime?.configHome?.env ?? []),
|
|
),
|
|
].sort();
|
|
|
|
// Guards the guard: an empty/renamed registry shape would make the
|
|
// assertion below vacuously true and silently retire this test.
|
|
assert.ok(
|
|
declared.length >= 15,
|
|
`expected the registry to declare many configHome env vars, got ${declared.length} — ` +
|
|
'if the registry shape changed, this derivation needs updating, not deleting',
|
|
);
|
|
|
|
const missing = declared.filter((k) => !(k in TEST_ENV_BASE));
|
|
assert.deepStrictEqual(
|
|
missing,
|
|
[],
|
|
`config-location env vars reachable by the resolver but not scrubbed: ${missing.join(', ')}. ` +
|
|
'TEST_ENV_BASE derives this set from the capability registry — a gap here means the ' +
|
|
'derivation broke, not that the list needs a manual entry.',
|
|
);
|
|
});
|
|
|
|
test('every scrubbed config-location var is blanked, not merely present', () => {
|
|
for (const key of CONFIG_LOCATION_ENV_KEYS) {
|
|
assert.strictEqual(
|
|
TEST_ENV_BASE[key],
|
|
'',
|
|
`${key} must be blanked ('') so the child sees a falsy value on the env-first branch`,
|
|
);
|
|
}
|
|
});
|
|
|
|
test('the non-registry config-location vars are covered too', () => {
|
|
// These have no capability descriptor, so the registry derivation alone
|
|
// cannot reach them: GROK_AGENTS_HOME is a hardcoded branch of
|
|
// getGlobalConfigDir, GSD_RUNTIME selects which runtime home resolves, and
|
|
// GSD_PROJECT / GSD_WORKSTREAM move a child's .planning root
|
|
// (src/planning-workspace.cts). Named explicitly so deleting one from the
|
|
// helper is a test failure rather than a silent narrowing.
|
|
for (const key of ['GROK_AGENTS_HOME', 'GSD_RUNTIME', 'GSD_PROJECT', 'GSD_WORKSTREAM']) {
|
|
assert.strictEqual(TEST_ENV_BASE[key], '', `${key} must be scrubbed`);
|
|
}
|
|
});
|
|
|
|
test('descriptor-shaped config homes OUTSIDE the registry are derived, not listed', () => {
|
|
const {
|
|
NON_REGISTRY_CONFIG_HOME_DESCRIPTORS,
|
|
} = require('../gsd-core/bin/lib/runtime-homes.cjs');
|
|
|
|
// Round 3. kimi owns TWO config homes: KIMI_CONFIG_DIR (registry-visible) and
|
|
// KIMI_SHARE_DIR (a hardcoded descriptor inside resolveKimiHooksTomlDir, which
|
|
// decides where its native config.toml — carrying GSD's [[hooks]] block — is
|
|
// written). The registry-only derivation reached the first and not the second,
|
|
// so it looked structurally complete while missing a live write surface.
|
|
const declared = [
|
|
...new Set(NON_REGISTRY_CONFIG_HOME_DESCRIPTORS.flatMap((d) => d?.env ?? [])),
|
|
];
|
|
assert.ok(
|
|
declared.length >= 1,
|
|
'expected at least one non-registry descriptor — an empty array makes this vacuous',
|
|
);
|
|
assert.ok(
|
|
declared.includes('KIMI_SHARE_DIR'),
|
|
`KIMI_SHARE_DIR must come from the descriptor set, got ${declared.join(', ')}`,
|
|
);
|
|
|
|
const missing = declared.filter((k) => !(k in TEST_ENV_BASE));
|
|
assert.deepStrictEqual(
|
|
missing,
|
|
[],
|
|
`descriptor-declared config-location vars not scrubbed: ${missing.join(', ')}`,
|
|
);
|
|
});
|
|
|
|
test("GSD's OWN location vars are scrubbed (a second family, not a registry gap)", () => {
|
|
const { GSD_LOCATION_ENV_KEYS } = require('../gsd-core/bin/lib/runtime-homes.cjs');
|
|
|
|
// GSD_HOME decides where GSD keeps user-owned state ($GSD_HOME/.gsd/ —
|
|
// consent.json, defaults.json, capability overlays) and is read env-FIRST,
|
|
// ahead of os.homedir(), across capability-loader / capability-consent /
|
|
// capability-state / capability-writer / config-loader / install-profiles /
|
|
// bin/install.js. GSD_AGENTS_DIR is priority 1 in getAgentsDir. Neither is a
|
|
// runtime configHome, so no amount of registry derivation reaches them.
|
|
assert.ok(GSD_LOCATION_ENV_KEYS.includes('GSD_HOME'));
|
|
for (const key of GSD_LOCATION_ENV_KEYS) {
|
|
assert.strictEqual(TEST_ENV_BASE[key], '', `${key} must be scrubbed`);
|
|
}
|
|
});
|
|
|
|
test('scrubConfigLocationEnv clears and restores the parent process env', () => {
|
|
// The in-process half of the fix (Blocker 1): TEST_ENV_BASE only reaches
|
|
// children, so a test calling install() in-process needs the PARENT's env
|
|
// cleared. Round-trip both states — set and unset — because restoring an
|
|
// originally-unset var as '' rather than deleting it is itself a leak.
|
|
withIsolatedProcessState(() => {
|
|
process.env.CLAUDE_CONFIG_DIR = '/tmp/ambient-claude';
|
|
delete process.env.CODEX_HOME;
|
|
|
|
const restore = scrubConfigLocationEnv();
|
|
assert.strictEqual(process.env.CLAUDE_CONFIG_DIR, undefined,
|
|
'a set config-location var must be deleted, not blanked, on the parent');
|
|
assert.strictEqual(process.env.CODEX_HOME, undefined);
|
|
|
|
restore();
|
|
assert.strictEqual(process.env.CLAUDE_CONFIG_DIR, '/tmp/ambient-claude',
|
|
'restore must put back the original value');
|
|
assert.ok(!('CODEX_HOME' in process.env),
|
|
'restore must leave an originally-unset var unset, not set it to empty string');
|
|
});
|
|
});
|
|
});
|