From 3c580b77dcc1cd3c22da51330aaa281716564350 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Sat, 1 Aug 2026 01:57:37 -0500 Subject: [PATCH] fix(#2665): derive the second config-location family instead of hand-adding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 named GSD_HOME and KIMI_SHARE_DIR as missing from the derived scrub set. Both premises confirmed; the prescribed remedy is not adopted verbatim, because adding two more literals to a four-item hand list is the pattern that reopened this bug three times. The census the derivation generalizes over was partial, so the census is what widens. Two structural gaps, both closed at the source: 1. KIMI_SHARE_DIR lived inside resolveKimiHooksTomlDir's body as an inline descriptor, resolvable but not ENUMERABLE. Hoisted to an exported KIMI_HOOKS_TOML_DESCRIPTOR and collected in NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, which TEST_ENV_BASE now derives from. kimi is the sharp case: it owns TWO config homes (KIMI_CONFIG_DIR, already registry-visible, and this one), so a registry-only derivation looks complete and is not. 2. GSD_HOME is a different FAMILY, not a missing registry entry. The registry describes where third-party runtimes keep config; GSD_HOME decides where GSD keeps its own user-owned state ($GSD_HOME/.gsd/ — consent.json, defaults.json, capability overlays), read env-first ahead of os.homedir() by capability-loader, capability-consent, capability-state, capability-writer, config-loader, install-profiles and bin/install.js. Named as GSD_LOCATION_ENV_KEYS rather than folded into the descriptor array, since it does not resolve through resolveConfigHomeFromDescriptor. GSD_AGENTS_DIR joins the same family (round 2, Minor): env-first and unconditional in getAgentsDir, misdirecting a read rather than a write. A census of every env-first first-party location var — the guard-shape question this PR owes each round — now yields exactly one remaining unguarded name, GSD_MODEL_CATALOG, and it is dead by precedence: the co-located candidate is index 0 and the loop breaks on first success, so the env var can only win on a tree that is already broken, and it redirects a read even then. Derived set: 39 -> 42 keys. resolveKimiHooksTomlDir behaviour unchanged on both the default and the KIMI_SHARE_DIR override path. --- src/runtime-homes.cts | 78 ++++++++++++++++++++++++++++++++++++++++--- tests/helpers.cjs | 20 ++++++++++- 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/src/runtime-homes.cts b/src/runtime-homes.cts index e8f37818e..0655d68fc 100644 --- a/src/runtime-homes.cts +++ b/src/runtime-homes.cts @@ -157,7 +157,7 @@ interface NoneDescriptor { skillsHome?: ConfigHomeDescriptor; } -type ConfigHomeDescriptor = +export type ConfigHomeDescriptor = | DotHomeDescriptor | DotHomeNestedDescriptor | XdgDescriptor @@ -480,6 +480,76 @@ export function resolveKimiGlobalDir(opts: ResolveKimiOpts = {}): string { ); } +/** + * Kimi CLI's own native config.toml home. Hoisted out of resolveKimiHooksTomlDir + * so it is ENUMERABLE, not merely resolvable. + * + * #2665 round 3: a config-location var that lives only inside a function body is + * invisible to every consumer that needs the SET rather than the path — the test + * scrub list and the hermeticity guard both derive from descriptors, and this one + * reached neither. `kimi` is the sharp case precisely because it owns TWO config + * homes: KIMI_CONFIG_DIR (registry-visible, already covered) and KIMI_SHARE_DIR + * (this one), so a derivation keyed only on the registry looks complete and is not. + */ +export const KIMI_HOOKS_TOML_DESCRIPTOR: ConfigHomeDescriptor = { + kind: 'dot-home', + name: '.kimi', + env: ['KIMI_SHARE_DIR'], +}; + +/** + * Kimi Code's native config.toml home — the `kimi-code` counterpart of the + * descriptor above, hoisted for exactly the same reason. + * + * #2755 landed kimi-code hooks support on `next` while this PR was open, and + * declared this descriptor as an inline object literal inside + * resolveKimiHooksTomlDir's body — the same resolvable-but-not-enumerable shape + * round 3 hoisted KIMI_SHARE_DIR out of. Hoisting it puts `KIMI_CODE_HOME` into + * the derived scrub set and the hermeticity guard's watch roots in the SAME + * commit, which is the property NON_REGISTRY_CONFIG_HOME_DESCRIPTORS exists to + * guarantee. Each product's env var stays scoped to that product (#2755). + */ +export const KIMI_CODE_HOOKS_TOML_DESCRIPTOR: ConfigHomeDescriptor = { + kind: 'dot-home', + name: '.kimi-code', + env: ['KIMI_CODE_HOME'], +}; + +/** + * Config-home descriptors resolved OUTSIDE the capability registry. + * + * Anything added here is picked up by every derived consumer in the same commit — + * which is the property that makes the derivation structurally incapable of being + * narrower than the surface it guards. Adding a hardcoded resolver WITHOUT adding + * its descriptor here is the defect this array exists to make hard. + */ +export const NON_REGISTRY_CONFIG_HOME_DESCRIPTORS: ConfigHomeDescriptor[] = [ + KIMI_HOOKS_TOML_DESCRIPTOR, + KIMI_CODE_HOOKS_TOML_DESCRIPTOR, +]; + +/** + * GSD's OWN location vars — a second family, not runtime configHomes. + * + * #2665 round 3: the registry describes where each *third-party runtime* keeps its + * config. It says nothing about where GSD keeps its own user-owned state, and that + * is a separate env-first surface: + * + * GSD_HOME — `process.env['GSD_HOME'] || os.homedir()`, the root of + * `$GSD_HOME/.gsd/` (consent.json, defaults.json, capability + * overlays). Read env-first by capability-loader, capability-consent, + * capability-state, capability-writer, config-loader, install-profiles + * and bin/install.js. A WRITE surface. + * GSD_AGENTS_DIR — `if (process.env['GSD_AGENTS_DIR']) return it`, priority 1 in + * getAgentsDir. Misdirects a READ rather than a write, hence lower + * severity — but it is env-first and unconditional, so it belongs + * to the same class. + * + * Deliberately NOT folded into the descriptor array above: these do not resolve + * through resolveConfigHomeFromDescriptor and have no `kind`/`name` shape. + */ +export const GSD_LOCATION_ENV_KEYS: readonly string[] = ['GSD_HOME', 'GSD_AGENTS_DIR']; + /** * Resolve the directory holding the Kimi product's OWN native config.toml — * the file that product itself reads for providers/models/hooks/etc, and the @@ -516,9 +586,9 @@ export function resolveKimiHooksTomlDir(opts: ResolveKimiHooksTomlOpts = {}): st // Explicit comparison rather than an object lookup keyed on `runtime`: the // value originates from argv, and an index would resolve inherited keys // (`constructor`, `__proto__`) to something that is not a descriptor. - const descriptor: DotHomeDescriptor = opts.runtime === 'kimi-code' - ? { kind: 'dot-home', name: '.kimi-code', env: ['KIMI_CODE_HOME'] } - : { kind: 'dot-home', name: '.kimi', env: ['KIMI_SHARE_DIR'] }; + const descriptor: ConfigHomeDescriptor = opts.runtime === 'kimi-code' + ? KIMI_CODE_HOOKS_TOML_DESCRIPTOR + : KIMI_HOOKS_TOML_DESCRIPTOR; return resolveConfigHomeFromDescriptor(descriptor, { env, home }); } diff --git a/tests/helpers.cjs b/tests/helpers.cjs index db83f2779..8b4a27f42 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -42,12 +42,22 @@ const SESSION_IDENTITY_ENV_KEYS = [ // incapable of being narrower than the surface it guards: adding a capability // that declares a new configHome env var extends this set in the same commit. const { runtimes } = require('../gsd-core/bin/lib/capability-registry.cjs'); +const { + NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, + GSD_LOCATION_ENV_KEYS, +} = require('../gsd-core/bin/lib/runtime-homes.cjs'); -// Config-location vars the registry does NOT carry, each with its reader: +// Config-location vars that are neither in the registry nor descriptor-shaped, +// each with its reader: // GROK_AGENTS_HOME — hardcoded `grok` branch in getGlobalConfigDir (src/runtime-homes.cts) // GSD_RUNTIME — selects WHICH runtime home resolves (src/model-resolver.cts) // GSD_PROJECT — planningDir() project segment (src/planning-workspace.cts) // GSD_WORKSTREAM — planningDir() workstream segment (src/planning-workspace.cts) +// +// #2665 round 3: this list shrinks as sources become enumerable, and that direction +// is the point. KIMI_SHARE_DIR was NOT added here — it now derives from +// NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, because hand-adding each var a reviewer +// names is precisely what reopened this bug three times. const NON_REGISTRY_CONFIG_LOCATION_ENV_KEYS = [ 'GROK_AGENTS_HOME', 'GSD_RUNTIME', @@ -57,7 +67,15 @@ const NON_REGISTRY_CONFIG_LOCATION_ENV_KEYS = [ const CONFIG_LOCATION_ENV_KEYS = [ ...new Set([ + // 1. Every runtime descriptor the capability registry carries. ...Object.values(runtimes).flatMap((r) => r?.runtime?.configHome?.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 ?? []), + // 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, + // 4. The residue that is neither registry-carried nor descriptor-shaped. ...NON_REGISTRY_CONFIG_LOCATION_ENV_KEYS, ]), ].sort();