fix(#3664): warn when config-dir targets a foreign-agent destination (#3794)

* test(#3664): failing-first rows for the config-dir foreign-agent warning

* fix(#3664): warn when config-dir targets a foreign-agent destination

* test(#3664): fold the foreign-agent warning rows into the install-regressions suite

* fix(#3664): close review findings — kimi-agents kind, gsd.md ownership, e2e gate

* test(#3664): sync boolean call sites and the path-vocab registries

* chore(#3664): backfill changeset pr number

* test(#3663): skip the posix case-pin on win32 where folding is the fix

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-24 02:45:28 -04:00
committed by GitHub
parent 314ea20fa4
commit 9d65cd5404
6 changed files with 238 additions and 3 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3794
---
**Installing with --config-dir into a directory that already holds another harness's agent files now warns instead of failing silently** — the installer says the emitted artifacts are shaped for the selected runtime and their tool IDs and MCP grants may be inert or invalid for the destination harness, then proceeds. Fresh custom directories and GSD-only directories stay silent. (#3664)

View File

@@ -669,6 +669,53 @@ function _resolveScopeSafe(id, runtime) {
}
}
/**
* Resolve a layout kind's on-disk destination directory. Shared by the
* skills-root resolution above and the #3664 warning below so the
* home-override + destSubpath join exists once (#3659-class re-encoding guard).
*/
function _kindDestDir(layout, kindName, targetDir) {
const kind = layout.kinds.find((k) => k.kind === kindName);
if (!kind) return null;
return path.join(kind.home || targetDir, kind.destSubpath);
}
/**
* #3664 — warn (never refuse) when `--config-dir` points the install at a
* directory whose agent destination already holds FOREIGN (non-GSD) agent
* files — the fingerprint of another harness's config home (e.g. ~/.junie,
* ~/.factory) or a hand-curated agents dir. GSD emits the selected runtime's
* artifacts verbatim: tool IDs (`Skill`) and MCP grants (`mcp__server__tool`)
* that are inert or invalid in a foreign harness surface only at dispatch
* time, months later. Warn-and-proceed is the issue-sanctioned option (b):
* a fresh custom dir (the documented brand-specific-dir use), a gsd-only dir
* (updates, the --all shared dir — including kimi's root `gsd.md`, which is
* GSD-owned despite the bare `gsd` stem), and the no-flag default-home path
* (users keep personal agents in ~/.claude/agents) all stay silent. Degrades
* silently on any resolution failure — the warn path never blocks install.
*/
function warnIfForeignAgentDest(runtime, targetDir, scope, explicitConfigDir) {
if (explicitConfigDir !== true) return;
try {
const layout = resolveRuntimeArtifactLayout(runtime, targetDir, scope);
// kimi's global agents kind is `kimi-agents` (#2095 EoS), every other
// runtime's is `agents`.
const agentsDir = _kindDestDir(layout, 'agents', targetDir)
|| _kindDestDir(layout, 'kimi-agents', targetDir);
if (!agentsDir) return;
if (!fs.existsSync(agentsDir) || !fs.statSync(agentsDir).isDirectory()) return;
const foreign = fs
.readdirSync(agentsDir)
.filter((f) => f.endsWith('.md') && !f.startsWith('gsd-') && f !== 'gsd.md');
if (foreign.length === 0) return;
console.log(
` ${yellow}⚠${reset} ${bold}${targetDir}${reset} already contains ${foreign.length} non-GSD agent file(s) — this may be another harness's config home. GSD emits artifacts shaped for ${runtime}: tool IDs and MCP grants may be inert or invalid for whatever harness reads this directory (#3664).`
);
} catch (_) {
/* never block install on the warning path */
}
}
/**
* Resolve the ACTUAL on-disk skills-install directory for a runtime, honoring a
* skills-kind `home` override (ADR-1239 upgrade 3 / #2088: e.g. Codex skills ->
@@ -679,8 +726,8 @@ function _resolveScopeSafe(id, runtime) {
function _resolveSkillsRootDir(runtime, targetDir, scope) {
try {
const layout = resolveRuntimeArtifactLayout(runtime, targetDir, scope);
const skillsKind = layout.kinds.find((k) => k.kind === 'skills');
if (skillsKind) return path.join(skillsKind.home || targetDir, skillsKind.destSubpath);
const skillsDir = _kindDestDir(layout, 'skills', targetDir);
if (skillsDir) return skillsDir;
} catch (_e) { /* fall through to the configDir default */ }
return path.join(targetDir, 'skills');
}
@@ -10219,6 +10266,12 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) {
? process.cwd()
: path.join(process.cwd(), dirName);
// #3664: a --config-dir destination holding foreign agent files gets an
// explicit install-time warning — never a silent Claude-shaped emit.
if (isGlobal) {
warnIfForeignAgentDest(runtime, targetDir, _installScopeId, Boolean(explicitConfigDir));
}
// #2875 (#1874-F19 anti-inertness, test-matrix C7): recover any user
// artifact orphaned by a PRIOR install run that died between staging and
// its own restore/discard, BEFORE this run's own preserve step stages
@@ -13567,6 +13620,8 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) {
module.exports = {
// #3677 — hyphen-namespace normalization seam for agent bodies
shouldNormalizeHyphenNamespaceInAgentBody,
// #3664: --config-dir foreign-agent-destination warning (warn-and-proceed)
warnIfForeignAgentDest,
normalizeAgentBodyForRuntime,
yamlIdentifier,
getCodexSkillAdapterHeader,

View File

@@ -60,6 +60,8 @@ const PATH_RETURNING_FNS = [
// #2088 (ADR-1239 upgrade 3): resolves the on-disk skills-install dir honoring
// a skills-kind `home` override (e.g. Codex → $HOME/.agents/skills).
'_resolveSkillsRootDir',
// #3664: shared kind-destination resolver (skills/agents/kimi-agents kinds).
'_kindDestDir',
'getGlobalSkillDir',
'getGlobalSkillDisplayPath',
'resolveSkillsBaseFromDescriptor',

View File

@@ -576,6 +576,14 @@ describe('W027 — #3663 isActiveWorktreePath casing/separator normalization', (
});
test('W027 still fires for differently-cased paths on posix (case-sensitive pin)', (t) => {
// POSIX-only by definition: on win32 the #3663 fix correctly FOLDS the
// casing, so the differently-cased path IS the active worktree and must
// NOT be flagged — that win32 behavior is pinned by the predicate rows
// above (isActiveWorktreePath … 'win32').
if (process.platform === 'win32') {
t.skip('win32 folds casing by design (#3663); the posix case-sensitivity pin has no meaning there');
return;
}
const cwd = createTempDir('gsd-3663-w027-posix-case-');
t.after(() => cleanup(cwd));
fs.mkdirSync(planningDirOf(cwd), { recursive: true });

View File

@@ -6,6 +6,7 @@
* regressions file for the installer module cluster.
*
* Defects covered:
* #3664 — --config-dir foreign-agent destination warning (warn-and-proceed)
* #3664 Defect #1 — stale skills/gsd/gsd-<stem>/ dirs on Hermes upgrade
* #3664 Defect #2 — --hermes --profile=core falls through to wrong path
* #2973 M1–M3 — dev-preferences migration at profile=core for hermes/qwen/claude
@@ -20,6 +21,7 @@ const fs = require('node:fs');
const path = require('node:path');
const { runNode } = require('./helpers/process-seam.cjs');
const { throwIfFailed } = require('./helpers/git-fixture.cjs');
const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const { createTempDir, cleanup } = require('./helpers.cjs');
const {
@@ -53,7 +55,6 @@ const {
const INSTALL_SCRIPT = path.join(__dirname, '..', 'bin', 'install.js');
// #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs.
const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const HOOKS_SRC = path.join(__dirname, '..', 'hooks');
const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd');
const MANIFEST = loadSkillsManifest(REAL_COMMANDS_DIR);
@@ -1685,3 +1686,165 @@ describe('#3329 regression: stale managed .sh hook commands are reconciled on in
assert.strictEqual(settings.hooks.SessionStart[0].hooks[0].command, original);
});
});
// ─── #3664 — config-dir foreign-agent destination warning ───────────────────
//
// --config-dir aimed at a directory that is not a supported runtime's config
// home must never be a SILENT success: the installer emits the selected
// runtime's artifacts verbatim (Claude-only Skill tool IDs, mcp__server__tool
// grants), which are inert or invalid in a foreign harness and surface only
// at dispatch time. Warn-and-proceed when the destination already holds
// foreign (non-GSD) agent files; fresh custom dirs, gsd-only dirs (updates,
// the --all shared dir), the no-flag default-home path, and test temp dirs
// stay silent. Folded into this suite per the lint-test-file-count cap
// (primary + one integration per production module).
const installerMod = require('../bin/install.js');
function capturedLogs(t, fn) {
const lines = [];
const mock = t.mock.method(console, 'log', (...args) => {
lines.push(args.join(' '));
});
const result = fn();
mock.mock.restore();
return { lines, result };
}
function makeForeignDest(t, name, agentFiles) {
const dest = createTempDir(`gsd-3664-${name}-`);
const agentsDir = path.join(dest, 'agents');
fs.mkdirSync(agentsDir, { recursive: true });
for (const file of agentFiles) {
fs.writeFileSync(path.join(agentsDir, file), '---\ntools: Read\n---\nbody\n');
}
t.after(() => cleanup(dest));
return { dest, agentsDir };
}
describe('#3664 — config-dir foreign-agent destination warning', () => {
test('warns when the destination holds foreign agent files', (t) => {
const { dest } = makeForeignDest(t, 'foreign', ['junie-guide.md']);
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('claude', dest, 'global', true),
);
const warning = lines.find((l) => l.includes('(#3664)'));
assert.ok(warning, `expected a #3664 warning, got: ${lines.join(' | ') || '(none)'}`);
assert.ok(warning.includes('claude'), `warning must name the selected runtime: ${warning}`);
assert.ok(
/tool IDs and MCP grants may (be inert|not apply)/.test(warning),
`warning must name the tool/MCP risk: ${warning}`,
);
});
test('installer warns and proceeds on a foreign-agent destination', (t) => {
const home = createTempDir('gsd-3664-e2e-home-');
t.after(() => cleanup(home));
const dest = createTempDir('gsd-3664-e2e-dest-');
t.after(() => cleanup(dest));
fs.mkdirSync(path.join(dest, 'agents'), { recursive: true });
fs.writeFileSync(path.join(dest, 'agents', 'junie-guide.md'), '---\ntools: Read\n---\nbody\n');
const result = runNode(
[path.join(__dirname, '..', 'bin', 'install.js'), '--claude', '--global', '--config-dir', dest],
{
env: {
...process.env,
HOME: home,
USERPROFILE: home,
GSD_HOME: home,
CI: '1',
},
timeoutMs: INSTALL_TIMEOUT_MS,
},
);
assert.equal(result.exitCode, 0, `warn-and-proceed must exit 0; stderr: ${result.stderr.slice(0, 500)}`);
assert.ok(
result.stdout.includes('(#3664)'),
`stdout must carry the #3664 warning: ${result.stdout.slice(-800)}`,
);
const emitted = fs.existsSync(path.join(dest, 'agents'))
? fs.readdirSync(path.join(dest, 'agents')).filter((f) => /^gsd-.*\.md$/.test(f))
: [];
assert.ok(emitted.length > 0, 'install must still emit the gsd-* agents');
assert.ok(fs.existsSync(path.join(dest, 'agents', 'junie-guide.md')));
});
test('silent on a fresh custom dir', (t) => {
const { dest } = makeForeignDest(t, 'fresh', []);
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('claude', dest, 'global', true),
);
assert.ok(!lines.some((l) => l.includes('(#3664)')), `fresh dir must be silent: ${lines.join(' | ')}`);
});
test('silent on a gsd-only agents dir', (t) => {
const { dest } = makeForeignDest(t, 'gsdonly', ['gsd-executor.md', 'gsd-verifier.md']);
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('claude', dest, 'global', true),
);
assert.ok(!lines.some((l) => l.includes('(#3664)')), `gsd-only dir must be silent: ${lines.join(' | ')}`);
});
test('silent when the config-dir flag was not passed', (t) => {
const { dest } = makeForeignDest(t, 'noflag', ['personal-agent.md']);
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('claude', dest, 'global', false),
);
assert.ok(!lines.some((l) => l.includes('(#3664)')), `no-flag path must be silent: ${lines.join(' | ')}`);
});
test('warns on mixed gsd and personal agents', (t) => {
const { dest } = makeForeignDest(t, 'mixed', ['gsd-executor.md', 'my-own-agent.md']);
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('claude', dest, 'global', true),
);
const warning = lines.find((l) => l.includes('(#3664)'));
assert.ok(warning, 'mixed dir with a foreign agent must warn');
assert.ok(warning.includes('contains 1 non-GSD agent file'), `warning reports the foreign count: ${warning}`);
});
test('ignores non-markdown files', (t) => {
const { dest, agentsDir } = makeForeignDest(t, 'nonmd', []);
fs.writeFileSync(path.join(agentsDir, 'notes.txt'), 'not an agent');
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('claude', dest, 'global', true),
);
assert.ok(!lines.some((l) => l.includes('(#3664)')), `non-agent files are not harness evidence: ${lines.join(' | ')}`);
});
test('warns for the kimi runtime (kimi-agents kind)', (t) => {
const { dest } = makeForeignDest(t, 'kimi-foreign', ['junie-guide.md']);
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('kimi', dest, 'global', true),
);
assert.ok(
lines.some((l) => l.includes('(#3664)') && l.includes('kimi')),
`kimi's kimi-agents kind must take the gate: ${lines.join(' | ')}`,
);
});
test('silent on kimi\'s own gsd.md in a shared multi-runtime dir', (t) => {
// kimi's root agent is agents/gsd.md — a bare stem, no gsd- prefix. A
// shared --all dir accumulates it alongside gsd-*.md; none of it is
// foreign, so every runtime's install into that dir must stay silent.
const { dest } = makeForeignDest(t, 'shared-kimi', ['gsd-executor.md', 'gsd.md']);
const { lines } = capturedLogs(t, () => {
installerMod.warnIfForeignAgentDest('kimi', dest, 'global', true);
installerMod.warnIfForeignAgentDest('claude', dest, 'global', true);
});
assert.ok(
!lines.some((l) => l.includes('(#3664)')),
`GSD-owned gsd.md must not read as a foreign agent: ${lines.join(' | ')}`,
);
});
test('degrades silently when the layout cannot resolve', (t) => {
const dest = createTempDir('gsd-3664-unknown-');
t.after(() => cleanup(dest));
const { lines } = capturedLogs(t, () =>
installerMod.warnIfForeignAgentDest('not-a-registered-runtime', dest, 'global', true),
);
assert.ok(!lines.some((l) => l.includes('(#3664)')), `unresolvable layout must degrade silently: ${lines.join(' | ')}`);
});
});

View File

@@ -50,6 +50,8 @@ const INSTALL_JS_PATH_HELPERS = [
// #2088 (ADR-1239 upgrade 3): resolves the skills-install dir honoring a
// skills-kind `home` override (e.g. Codex → $HOME/.agents/skills).
'_resolveSkillsRootDir',
// #3664: shared kind-destination resolver (skills/agents/kimi-agents kinds).
'_kindDestDir',
];
describe('portability-vocab drift guard', () => {