diff --git a/scripts/live-config-guard.cjs b/scripts/live-config-guard.cjs index cdfe4d65f..eec8f65f5 100644 --- a/scripts/live-config-guard.cjs +++ b/scripts/live-config-guard.cjs @@ -116,6 +116,14 @@ function resolveLiveConfigRoots(deps = {}) { const roots = new Set(); // 'grok' is a hardcoded branch of getGlobalConfigDir with no registry entry. + // + // DELIBERATE NON-ROOT: getGlobalSkillsBase(runtime) is NOT added here. The + // skills base (e.g. codex's ~/.agents/skills) is not a config ROOT, and the + // snapshot applies the config-root layout (GSD_OWNED_ENTRIES x + // GSD_PREFIXED_PARENTS) beneath every root it is given — measured on a + // sandboxed HOME, adding it both false-positives on `/gsd-core` + // and misses a real `/gsd-help` write. Watching skills bases + // needs its own layout, like resolveExtraWatchTargets — a separate change. for (const runtime of [...Object.keys(runtimes || {}), 'grok']) { try { const dir = getGlobalConfigDir(runtime); diff --git a/tests/helpers-process-isolation.test.cjs b/tests/helpers-process-isolation.test.cjs index cd93b61d3..d18e4f2b9 100644 --- a/tests/helpers-process-isolation.test.cjs +++ b/tests/helpers-process-isolation.test.cjs @@ -154,6 +154,50 @@ describe('#2665: TEST_ENV_BASE config-location coverage', () => { ); }); + test('skillsHome env vars are walked on BOTH descriptor rungs', () => { + // Round 4. A configHome descriptor can nest a second, independently-resolved + // descriptor (skillsHome → resolveSkillsBaseFromDescriptor), which carries + // its own env array. Walking configHome.env alone is the identical + // walk-one-field gap-shape rounds 2-3 closed for the registry and the + // non-registry set. Inert today — only kilo declares skillsHome, with + // env: [] — so this asserts the DERIVATION reaches the field, not that any + // var currently flows from it: every skillsHome-declared var (registry and + // non-registry alike) must land in TEST_ENV_BASE the moment one exists. + const { runtimes } = require('../gsd-core/bin/lib/capability-registry.cjs'); + const { + NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, + } = require('../gsd-core/bin/lib/runtime-homes.cjs'); + + const declared = [ + ...new Set([ + ...Object.values(runtimes).flatMap( + (r) => r?.runtime?.configHome?.skillsHome?.env ?? [], + ), + ...NON_REGISTRY_CONFIG_HOME_DESCRIPTORS.flatMap( + (d) => d?.skillsHome?.env ?? [], + ), + ]), + ]; + + // Anti-vacuity: at least one runtime must actually DECLARE skillsHome, or a + // registry reshape could rename the field and retire this test silently. + const declaringRuntimes = Object.values(runtimes).filter( + (r) => r?.runtime?.configHome?.skillsHome !== undefined, + ); + assert.ok( + declaringRuntimes.length >= 1, + 'expected at least one registry runtime to declare configHome.skillsHome — ' + + 'if the field moved, this derivation needs updating, not deleting', + ); + + const missing = declared.filter((k) => !(k in TEST_ENV_BASE)); + assert.deepStrictEqual( + missing, + [], + `skillsHome-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'); diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 8b4a27f42..64161dee9 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -67,11 +67,23 @@ const NON_REGISTRY_CONFIG_LOCATION_ENV_KEYS = [ const CONFIG_LOCATION_ENV_KEYS = [ ...new Set([ - // 1. Every runtime descriptor the capability registry carries. + // 1. Every runtime descriptor the capability registry carries — including + // the nested skillsHome descriptor, which resolves independently of + // configHome (resolveSkillsBaseFromDescriptor) and can carry its own + // env array. Inert today (only kilo declares skillsHome, with env: []), + // but walking configHome.env alone is the identical gap-shape this PR + // closed twice already, one field over. (#2665 round 4) ...Object.values(runtimes).flatMap((r) => r?.runtime?.configHome?.env ?? []), + ...Object.values(runtimes).flatMap( + (r) => r?.runtime?.configHome?.skillsHome?.env ?? [], + ), // 2. Descriptor-shaped config homes resolved OUTSIDE the registry (kimi's // native config.toml home via KIMI_SHARE_DIR). Derived, not hand-listed. - ...NON_REGISTRY_CONFIG_HOME_DESCRIPTORS.flatMap((d) => d?.env ?? []), + // Same skillsHome walk as rung 1 — a descriptor is a descriptor. + ...NON_REGISTRY_CONFIG_HOME_DESCRIPTORS.flatMap((d) => [ + ...(d?.env ?? []), + ...(d?.skillsHome?.env ?? []), + ]), // 3. GSD's OWN location vars — a different family: they decide where GSD keeps // user-owned state ($GSD_HOME/.gsd/), not where a runtime keeps its config. ...GSD_LOCATION_ENV_KEYS,