From 706bd2ab4e2e0a0b63a9422598e57f961a435ee2 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Sat, 1 Aug 2026 03:14:21 -0500 Subject: [PATCH] refactor(#2665): derive the guard's non-root targets from the descriptor array too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 38c9395d, found while fact-checking the round-3 response rather than by a test. That commit made TEST_ENV_BASE derive its keys from NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, but had the guard call resolveKimiHooksTomlDir directly. Both halves covered kimi, so nothing was broken — but only one of them would pick up a SECOND descriptor. That is the same partial-enumeration defect that put KIMI_SHARE_DIR outside the scrub set, reintroduced one layer over, in the very commit that closed it. resolveExtraWatchTargets now iterates the array and resolves each descriptor through resolveConfigHomeFromDescriptor, so the scrub set and the guard derive from one source and cannot drift apart. Verified: a synthetic second descriptor is picked up automatically (it was not before); kimi's target is unchanged on both the default (~/.kimi/config.toml) and KIMI_SHARE_DIR override paths. The new test asserts one target per descriptor plus the store root. The COUNT is the load-bearing half — every per-descriptor assertion passes vacuously today with a single entry, so only the count fails when the array grows and the guard does not follow. NAMED RESIDUAL, documented at NON_REGISTRY_OWNED_FILE: this assumes every non-registry descriptor is written the same way (config.toml). A descriptor whose owned file differs needs a per-descriptor mapping. It fails toward under-watching rather than false positives, so it is called out rather than left to be discovered. --- scripts/live-config-guard.cjs | 40 +++++++++++++++++++++++++++----- tests/live-config-guard.test.cjs | 33 ++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/scripts/live-config-guard.cjs b/scripts/live-config-guard.cjs index b0e054bc4..883aab154 100644 --- a/scripts/live-config-guard.cjs +++ b/scripts/live-config-guard.cjs @@ -74,6 +74,19 @@ const GSD_OWNED_ENTRIES = ['gsd-core', 'gsd-file-manifest.json', 'gsd-pristine'] const GSD_PREFIXED_PARENTS = ['agents', 'commands', 'skills']; const GSD_ARTIFACT_PREFIX = 'gsd-'; +/** + * The file GSD writes into a NON-REGISTRY config home. + * + * Today's only such descriptor is kimi's `~/.kimi` (KIMI_SHARE_DIR), where GSD + * writes its native `[[hooks]]` block into `config.toml`. NAMED RESIDUAL: this + * assumes every non-registry descriptor is written the same way. A future + * descriptor whose owned file differs needs a per-descriptor mapping here — the + * consequence of getting it wrong is under-watching (a missed leak), not a false + * positive, so it fails in the quiet direction and is called out rather than + * left to be discovered. + */ +const NON_REGISTRY_OWNED_FILE = 'config.toml'; + /** Bounds on the recursive walk, so a pathological tree cannot stall the suite. */ const MAX_ENTRIES = 20000; const MAX_DEPTH = 12; @@ -143,12 +156,27 @@ function resolveExtraWatchTargets(deps = {}) { const targets = [path.resolve(path.join(env.GSD_HOME || homedir(), '.gsd'))]; try { - const { resolveKimiHooksTomlDir } = require(path.join(libDir, 'runtime-homes.cjs')); - // Thread the SAME injected env/home the GSD_HOME line above uses. Calling it - // bare reads process.env and os.homedir() regardless of `deps`, which leaves - // the seam untestable and the two targets resolved against different worlds. - const kimiDir = resolveKimiHooksTomlDir({ env, home: homedir() }); - targets.push(path.resolve(path.join(kimiDir, 'config.toml'))); + const { + NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, + resolveConfigHomeFromDescriptor, + } = require(path.join(libDir, 'runtime-homes.cjs')); + + // ITERATE the descriptor array rather than naming one resolver. Calling + // resolveKimiHooksTomlDir directly would cover today's only entry and + // silently miss tomorrow's — the same partial-enumeration defect that put + // KIMI_SHARE_DIR outside the scrub set in the first place, reintroduced one + // layer over. TEST_ENV_BASE derives its keys from this array; deriving the + // guard's paths from it keeps the two halves from drifting apart. + // + // Thread the SAME injected env/home used above: resolving bare would read + // process.env and os.homedir() regardless of `deps`, leaving the seam + // untestable and the targets resolved against different worlds. + for (const descriptor of NON_REGISTRY_CONFIG_HOME_DESCRIPTORS) { + const dir = resolveConfigHomeFromDescriptor(descriptor, { env, home: homedir() }); + // GSD writes ONE named file into these third-party roots; the root itself + // belongs to the runtime, so it is never watched wholesale. + targets.push(path.resolve(path.join(dir, NON_REGISTRY_OWNED_FILE))); + } } catch { // Unbuilt tree — same posture as resolveLiveConfigRoots: advisory, never fatal. } diff --git a/tests/live-config-guard.test.cjs b/tests/live-config-guard.test.cjs index 9760f4767..41ebccca0 100644 --- a/tests/live-config-guard.test.cjs +++ b/tests/live-config-guard.test.cjs @@ -196,6 +196,39 @@ describe('#2665: guard watches non-root write surfaces', () => { } }); + test('extra targets are DERIVED from the descriptor array, not a named resolver', () => { + const { + NON_REGISTRY_CONFIG_HOME_DESCRIPTORS, + resolveConfigHomeFromDescriptor, + } = require('../gsd-core/bin/lib/runtime-homes.cjs'); + const home = tmpRoot(); + try { + const env = { GSD_HOME: home }; + const targets = resolveExtraWatchTargets({ env, os: { homedir: () => home } }); + + // Every descriptor in the array must contribute a target. Calling one + // named resolver instead would cover today's single entry and silently + // miss tomorrow's — the same partial-enumeration defect that put + // KIMI_SHARE_DIR outside the scrub set, one layer over. + for (const d of NON_REGISTRY_CONFIG_HOME_DESCRIPTORS) { + const dir = resolveConfigHomeFromDescriptor(d, { env, home }); + assert.ok( + targets.includes(path.resolve(path.join(dir, 'config.toml'))), + `descriptor ${JSON.stringify(d.env)} contributed no watch target`, + ); + } + // The count is what actually catches a regression to a hardcoded call: + // it fails the moment the array grows and the guard does not follow. + assert.strictEqual( + targets.length, + 1 + NON_REGISTRY_CONFIG_HOME_DESCRIPTORS.length, + 'expected the GSD store root plus exactly one target per descriptor', + ); + } finally { + cleanup(home); + } + }); + test('GSD_HOME falls back to homedir when unset', () => { const home = tmpRoot(); try {