Skip to content

Commit 7b2fc10

Browse files
NagyViktNagyVikt
andauthored
test(claude): extract uninstallMcpServer + cover the file-deletion edge cases (#629)
* feat(claude): gx claude install auto-registers the gx MCP server in .mcp.json The gx mcp agent radar only helps if agents have it registered; manual `claude mcp add` per machine meant nobody turned it on. Now `gx claude install` wires it into the repo alongside settings/hooks/commands. - installMcpServer merges { mcpServers.gx: { command: gx, args: [mcp, serve] } } into the target .mcp.json without clobbering other servers; idempotent. - --no-mcp opts out. - gx claude check warns when missing; doctor (check --fix) repairs via install. - gx claude uninstall removes the gx server (drops .mcp.json if only ours). - AGENTS.md repo-wiring bullet + usage updated. Tests: 4 new (create/merge/idempotent/dry-run) in claude-install.test.js, 18/18; no-new-failures vs base. * test(claude): extract uninstallMcpServer + cover the file-deletion edge cases Addresses review note: the uninstall path (the error-prone 'delete file only if it held nothing but ours' guard) had no coverage. Extracted uninstallMcpServer so it is unit-testable; added 4 tests including the {$schema, mcpServers:{gx}} case where an unrelated top-level key must block file deletion. 22/22 pass. --------- Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent 609963d commit 7b2fc10

7 files changed

Lines changed: 254 additions & 2 deletions

File tree

AGENTS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ If you are a Claude Code session arriving in this repo for the first time:
3232
commit + push + PR + merge + cleanup, still use the non-negotiable
3333
`gx branch finish --via-pr --wait-for-merge --cleanup`.
3434
4. **Repo wiring**`gx claude install` writes `.claude/settings.json`,
35-
hooks, slash commands, and the gitguardex skill into a target repo.
35+
hooks, slash commands, the gitguardex skill, and a `.mcp.json` that registers
36+
the read-only `gx` MCP server (the cross-repo agent radar: `list_agents`,
37+
`who_owns`, `my_context`) into a target repo. Opt out with `--no-mcp`.
3638
`gx claude check` diagnoses drift without writing; `gx claude doctor`
3739
diagnoses and repairs.
3840

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-06-05
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
## Why
2+
3+
`gx mcp` (the read-only cross-repo agent radar) only helps if agents actually
4+
have it registered. Today that means a manual `claude mcp add gx -s user -- gx
5+
mcp serve` per machine — so in practice nobody turns it on and the collision
6+
visibility never reaches the agents that need it. `gx claude install` already
7+
wires the rest of the gitguardex Claude integration into a repo; the MCP server
8+
should ride along.
9+
10+
## What Changes
11+
12+
- `gx claude install` now also registers the `gx` MCP server in the target
13+
repo's `.mcp.json` (`{ "mcpServers": { "gx": { "command": "gx", "args":
14+
["mcp", "serve"] } } }`). It MERGES into an existing `.mcp.json` without
15+
disturbing other servers, and is idempotent.
16+
- `--no-mcp` opts out of the registration.
17+
- `gx claude check` reports a warning when the `gx` server is missing; `gx
18+
claude doctor` (check --fix) repairs it via install.
19+
- `gx claude uninstall` removes the `gx` server (and deletes `.mcp.json` if it
20+
only held ours).
21+
22+
## Impact
23+
24+
- **Affected surface**: `src/cli/commands/claude.js` only (install/check/uninstall
25+
+ usage). New exports `installMcpServer`, `MCP_REL`, `MCP_SERVER_KEY`.
26+
- **Behavior change**: installing gitguardex into a repo now adds a committed
27+
`.mcp.json`; Claude Code will prompt to approve the project MCP server. Opt out
28+
with `--no-mcp`. Read-only server, no repo mutation at runtime.
29+
- **Portability**: `.mcp.json` references `gx` on PATH; a clone without gx shows
30+
the server as unavailable (soft failure), not an error.
31+
- Verified by `test/claude-install.test.js` (create / merge / idempotent /
32+
dry-run) plus end-to-end smoke of install/merge/--no-mcp/uninstall.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
## ADDED Requirements
2+
3+
### Requirement: gx claude install registers the gx MCP server
4+
`gx claude install` SHALL register the read-only `gx` MCP server in the target
5+
repo's `.mcp.json`, unless `--no-mcp` is passed.
6+
7+
#### Scenario: Fresh repo
8+
- **WHEN** `gx claude install` runs in a repo with no `.mcp.json`
9+
- **THEN** it creates `.mcp.json` containing `mcpServers.gx = { command: "gx", args: ["mcp", "serve"] }`.
10+
11+
#### Scenario: Merge into existing config
12+
- **WHEN** `.mcp.json` already defines other MCP servers
13+
- **THEN** install adds the `gx` server and leaves the other servers unchanged.
14+
15+
#### Scenario: Idempotent
16+
- **WHEN** install runs again with the `gx` server already present and correct
17+
- **THEN** the file is unchanged.
18+
19+
#### Scenario: Opt out
20+
- **WHEN** `gx claude install --no-mcp` runs
21+
- **THEN** no `.mcp.json` is created or modified.
22+
23+
### Requirement: check and uninstall cover the MCP registration
24+
`gx claude check` SHALL report missing registration, and `gx claude uninstall`
25+
SHALL remove it.
26+
27+
#### Scenario: Drift detected
28+
- **WHEN** `gx claude check` runs and `.mcp.json` lacks the `gx` server
29+
- **THEN** it reports a warning, and `gx claude doctor` repairs it via install.
30+
31+
#### Scenario: Clean removal
32+
- **WHEN** `gx claude uninstall --yes` runs
33+
- **THEN** the `gx` server is removed from `.mcp.json`, and the file is deleted if it held only the `gx` server, while any other servers are preserved.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
## Definition of Done
2+
3+
This change is complete only when **all** of the following are true:
4+
5+
- Every checkbox below is checked.
6+
- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff.
7+
- If any step blocks (test failure, conflict, ambiguous result), append a `BLOCKED:` line under section 4 explaining the blocker and **STOP**. Do not tick remaining cleanup boxes; do not silently skip the cleanup pipeline.
8+
9+
## Handoff
10+
11+
- Handoff: change=`agent-claude-gx-claude-install-auto-registers-gx-mcp-2026-06-05-13-56`; branch=`agent/<your-name>/<branch-slug>`; scope=`TODO`; action=`continue this sandbox or finish cleanup after a usage-limit/manual takeover`.
12+
- Copy prompt: Continue `agent-claude-gx-claude-install-auto-registers-gx-mcp-2026-06-05-13-56` on branch `agent/<your-name>/<branch-slug>`. Work inside the existing sandbox, review `openspec/changes/agent-claude-gx-claude-install-auto-registers-gx-mcp-2026-06-05-13-56/tasks.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent/<your-name>/<branch-slug> --base dev --via-pr --wait-for-merge --cleanup`.
13+
14+
## 1. Specification
15+
16+
- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-claude-gx-claude-install-auto-registers-gx-mcp-2026-06-05-13-56`.
17+
- [x] 1.2 Define normative requirements in `specs/gx-claude-install-auto-registers-gx-mcp-server-in-target-mcp-json/spec.md`.
18+
19+
## 2. Implementation
20+
21+
- [x] 2.1 Implement scoped behavior changes.
22+
- [x] 2.2 Add/update focused regression coverage.
23+
24+
## 3. Verification
25+
26+
- [x] 3.1 Run targeted project verification commands.
27+
- [x] 3.2 Run `openspec validate agent-claude-gx-claude-install-auto-registers-gx-mcp-2026-06-05-13-56 --type change --strict`.
28+
- [x] 3.3 Run `openspec validate --specs`.
29+
30+
## 4. Cleanup (mandatory; run before claiming completion)
31+
32+
- [ ] 4.1 Run the cleanup pipeline: `gx branch finish --branch agent/<your-name>/<branch-slug> --base dev --via-pr --wait-for-merge --cleanup`. This handles commit -> push -> PR create -> merge wait -> worktree prune in one invocation.
33+
- [ ] 4.2 Record the PR URL and final merge state (`MERGED`) in the completion handoff.
34+
- [ ] 4.3 Confirm the sandbox worktree is gone (`git worktree list` no longer shows the agent path; `git branch -a` shows no surviving local/remote refs for the branch).

src/cli/commands/claude.js

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ const SETTINGS_REL = '.claude/settings.json';
2020
const HOOKS_REL = '.claude/hooks';
2121
const COMMANDS_REL = '.claude/commands';
2222
const SKILLS_REL = '.claude/skills';
23+
// Repo-scoped MCP registration so any agent in the target repo can see the
24+
// cross-repo agent radar (`gx mcp`). Read-only server; opt out with --no-mcp.
25+
const MCP_REL = '.mcp.json';
26+
const MCP_SERVER_KEY = SHORT_TOOL_NAME;
2327

2428
const MANAGED_HOOK_FILES = [
2529
'skill_guard.py',
@@ -326,6 +330,47 @@ function describeStatus(s) {
326330
return '?';
327331
}
328332

333+
function mcpServerSpec() {
334+
return { command: SHORT_TOOL_NAME, args: ['mcp', 'serve'] };
335+
}
336+
337+
// Register the read-only `gx mcp` server in the target repo's .mcp.json so any
338+
// agent there can call list_agents / who_owns / my_context. Merges into an
339+
// existing .mcp.json without disturbing other servers; idempotent.
340+
function installMcpServer(repoRoot, { dryRun }) {
341+
const filePath = path.join(repoRoot, MCP_REL);
342+
const fileExisted = fs.existsSync(filePath);
343+
const config = readJsonIfExists(filePath) || {};
344+
config.mcpServers = config.mcpServers || {};
345+
const desired = mcpServerSpec();
346+
const current = config.mcpServers[MCP_SERVER_KEY];
347+
if (current && JSON.stringify(current) === JSON.stringify(desired)) {
348+
return { status: 'unchanged', dest: filePath };
349+
}
350+
const status = current ? 'updated' : fileExisted ? 'merged' : 'created';
351+
config.mcpServers[MCP_SERVER_KEY] = desired;
352+
writeJson(filePath, config, { dryRun });
353+
return { status, dest: filePath };
354+
}
355+
356+
// Inverse of installMcpServer: drop the gx server. Removes the whole .mcp.json
357+
// only when it held nothing but our server (no other servers AND no other
358+
// top-level keys); otherwise prunes just the gx entry and preserves the rest.
359+
function uninstallMcpServer(repoRoot, { dryRun }) {
360+
const filePath = path.join(repoRoot, MCP_REL);
361+
const config = readJsonIfExists(filePath);
362+
if (!config || !config.mcpServers || !config.mcpServers[MCP_SERVER_KEY]) {
363+
return { status: 'absent', dest: filePath };
364+
}
365+
delete config.mcpServers[MCP_SERVER_KEY];
366+
const onlyOurs = Object.keys(config.mcpServers).length === 0 && Object.keys(config).length === 1;
367+
if (!dryRun) {
368+
if (onlyOurs) fs.unlinkSync(filePath);
369+
else writeJson(filePath, config, { dryRun: false });
370+
}
371+
return { status: onlyOurs ? 'removed' : 'pruned', dest: filePath };
372+
}
373+
329374
function runInstall(rawArgs) {
330375
const opts = parseInstallArgs(rawArgs);
331376
const repoRoot = resolveRepoRoot(opts.target);
@@ -335,6 +380,9 @@ function runInstall(rawArgs) {
335380
const hookResults = installHooks(repoRoot, opts);
336381
const slashResults = installSlashCommands(repoRoot, opts);
337382
const skillResult = installAgentSkill(repoRoot, opts);
383+
const mcpResult = opts.noMcp
384+
? { status: 'skipped', dest: path.join(repoRoot, MCP_REL) }
385+
: installMcpServer(repoRoot, opts);
338386
const symlinkResult = ensureSpeckitMarkers(repoRoot, opts);
339387

340388
// Summary
@@ -354,6 +402,7 @@ function runInstall(rawArgs) {
354402
} else if (skillResult.status === 'source-missing') {
355403
logWarn('gitguardex skill source missing in package; skipped.');
356404
}
405+
logInfo(`mcp server (${MCP_REL}): ${mcpResult.status}`);
357406
logInfo(`CLAUDE.md symlink: ${symlinkResult.status}${symlinkResult.note ? ` (${symlinkResult.note})` : ''}`);
358407

359408
if (opts.json) {
@@ -363,6 +412,7 @@ function runInstall(rawArgs) {
363412
hooks: hookResults,
364413
slashCommands: slashResults,
365414
skill: skillResult,
415+
mcp: mcpResult,
366416
symlink: symlinkResult,
367417
dryRun: opts.dryRun,
368418
}, null, 2) + '\n');
@@ -435,6 +485,17 @@ function runCheck(rawArgs) {
435485
}
436486
}
437487

488+
// MCP registration check
489+
const mcpConfig = readJsonIfExists(path.join(repoRoot, MCP_REL));
490+
const hasGxMcp = Boolean(mcpConfig && mcpConfig.mcpServers && mcpConfig.mcpServers[MCP_SERVER_KEY]);
491+
if (!hasGxMcp) {
492+
issues.push({
493+
severity: 'warning',
494+
kind: 'mcp-missing',
495+
message: `${MCP_REL} does not register the '${MCP_SERVER_KEY}' MCP server (run '${SHORT_TOOL_NAME} claude install', or install --no-mcp to skip).`,
496+
});
497+
}
498+
438499
// Symlink check
439500
const symlinkResult = ensureSpeckitMarkers(repoRoot, { dryRun: true });
440501
if (symlinkResult.status === 'would-create-symlink'
@@ -518,6 +579,11 @@ function runUninstall(rawArgs) {
518579
if (!opts.dryRun) writeJson(settingsPath, settings, { dryRun: false });
519580
removed.push(`${SETTINGS_REL} (managed entries pruned)`);
520581
}
582+
// Remove the gx MCP server from .mcp.json (drop the file if it only held ours)
583+
const mcpRemoval = uninstallMcpServer(repoRoot, opts);
584+
if (mcpRemoval.status !== 'absent') {
585+
removed.push(`${MCP_REL} (${mcpRemoval.status === 'removed' ? 'removed' : `'${MCP_SERVER_KEY}' server pruned`})`);
586+
}
521587

522588
logOk(`Removed ${removed.length} item(s)${opts.dryRun ? ' (dry-run)' : ''}.`);
523589
for (const r of removed) console.log(` - ${r}`);
@@ -531,6 +597,7 @@ function parseInstallArgs(rawArgs) {
531597
json: false,
532598
yes: false,
533599
fix: false,
600+
noMcp: false,
534601
};
535602
for (let index = 0; index < rawArgs.length; index += 1) {
536603
const arg = rawArgs[index];
@@ -540,6 +607,7 @@ function parseInstallArgs(rawArgs) {
540607
if (arg === '--json') { opts.json = true; continue; }
541608
if (arg === '--yes' || arg === '-y') { opts.yes = true; continue; }
542609
if (arg === '--fix') { opts.fix = true; continue; }
610+
if (arg === '--no-mcp') { opts.noMcp = true; continue; }
543611
}
544612
return opts;
545613
}
@@ -548,14 +616,15 @@ function printUsage() {
548616
console.log(`Usage: ${SHORT_TOOL_NAME} claude <subcommand> [flags]
549617
550618
Subcommands:
551-
install install/update .claude/settings.json + hooks + slash commands.
619+
install install/update .claude/settings.json + hooks + slash commands + .mcp.json.
552620
check diagnose Claude Code wiring (read-only by default).
553621
doctor alias: 'check --fix'.
554622
uninstall remove gitguardex-managed Claude Code wiring (--yes required).
555623
556624
Flags:
557625
--target <path> Operate in a different repo directory.
558626
--force Overwrite existing managed entries instead of merging.
627+
--no-mcp Skip registering the gx MCP server in .mcp.json.
559628
--dry-run Report what would change without writing.
560629
--json Emit JSON output.
561630
--yes / -y Required for uninstall.
@@ -593,8 +662,13 @@ module.exports = {
593662
ensureSpeckitMarkers,
594663
installHooks,
595664
installSlashCommands,
665+
installMcpServer,
666+
uninstallMcpServer,
667+
mcpServerSpec,
596668
MANAGED_HOOK_FILES,
597669
MANAGED_SLASH_COMMANDS,
670+
MCP_REL,
671+
MCP_SERVER_KEY,
598672
TEMPLATE_DEFAULT_SETTINGS,
599673
EXPECTED_HOOK_MATCHERS,
600674
};

test/claude-install.test.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,81 @@ test('mergeSettings --force ignores existing settings', () => {
179179
assert.ok(mergedNonForce.hooks.PreToolUse.some((g) => g.matcher === 'Other'));
180180
});
181181

182+
test('installMcpServer registers the gx server in a fresh .mcp.json', () => {
183+
const repoRoot = makeRepo();
184+
const result = claudeModule.installMcpServer(repoRoot, { dryRun: false });
185+
assert.equal(result.status, 'created');
186+
const config = JSON.parse(fs.readFileSync(path.join(repoRoot, claudeModule.MCP_REL), 'utf8'));
187+
assert.deepEqual(config.mcpServers[claudeModule.MCP_SERVER_KEY], { command: 'gx', args: ['mcp', 'serve'] });
188+
});
189+
190+
test('installMcpServer merges into an existing .mcp.json without clobbering other servers', () => {
191+
const repoRoot = makeRepo();
192+
fs.writeFileSync(
193+
path.join(repoRoot, claudeModule.MCP_REL),
194+
JSON.stringify({ mcpServers: { other: { command: 'x' } } }, null, 2),
195+
);
196+
const result = claudeModule.installMcpServer(repoRoot, { dryRun: false });
197+
assert.equal(result.status, 'merged');
198+
const config = JSON.parse(fs.readFileSync(path.join(repoRoot, claudeModule.MCP_REL), 'utf8'));
199+
assert.deepEqual(Object.keys(config.mcpServers).sort(), ['gx', 'other']);
200+
assert.deepEqual(config.mcpServers.other, { command: 'x' }, 'existing server preserved');
201+
});
202+
203+
test('installMcpServer is idempotent on a second run', () => {
204+
const repoRoot = makeRepo();
205+
claudeModule.installMcpServer(repoRoot, { dryRun: false });
206+
const result = claudeModule.installMcpServer(repoRoot, { dryRun: false });
207+
assert.equal(result.status, 'unchanged');
208+
});
209+
210+
test('installMcpServer dry-run does not write .mcp.json', () => {
211+
const repoRoot = makeRepo();
212+
claudeModule.installMcpServer(repoRoot, { dryRun: true });
213+
assert.equal(fs.existsSync(path.join(repoRoot, claudeModule.MCP_REL)), false);
214+
});
215+
216+
test('uninstallMcpServer deletes .mcp.json when it only held the gx server', () => {
217+
const repoRoot = makeRepo();
218+
claudeModule.installMcpServer(repoRoot, { dryRun: false });
219+
const result = claudeModule.uninstallMcpServer(repoRoot, { dryRun: false });
220+
assert.equal(result.status, 'removed');
221+
assert.equal(fs.existsSync(path.join(repoRoot, claudeModule.MCP_REL)), false);
222+
});
223+
224+
test('uninstallMcpServer keeps the file (prunes only gx) when other servers exist', () => {
225+
const repoRoot = makeRepo();
226+
fs.writeFileSync(
227+
path.join(repoRoot, claudeModule.MCP_REL),
228+
JSON.stringify({ mcpServers: { other: { command: 'x' } } }, null, 2),
229+
);
230+
claudeModule.installMcpServer(repoRoot, { dryRun: false });
231+
const result = claudeModule.uninstallMcpServer(repoRoot, { dryRun: false });
232+
assert.equal(result.status, 'pruned');
233+
const config = JSON.parse(fs.readFileSync(path.join(repoRoot, claudeModule.MCP_REL), 'utf8'));
234+
assert.deepEqual(Object.keys(config.mcpServers), ['other'], 'gx removed, other kept');
235+
});
236+
237+
test('uninstallMcpServer preserves a file that has other top-level keys (no deletion)', () => {
238+
const repoRoot = makeRepo();
239+
fs.writeFileSync(
240+
path.join(repoRoot, claudeModule.MCP_REL),
241+
JSON.stringify({ $schema: 'https://example/schema.json', mcpServers: {} }, null, 2),
242+
);
243+
claudeModule.installMcpServer(repoRoot, { dryRun: false }); // adds gx
244+
const result = claudeModule.uninstallMcpServer(repoRoot, { dryRun: false });
245+
assert.equal(result.status, 'pruned', 'extra top-level key blocks file deletion');
246+
const config = JSON.parse(fs.readFileSync(path.join(repoRoot, claudeModule.MCP_REL), 'utf8'));
247+
assert.equal(config.$schema, 'https://example/schema.json', 'unrelated top-level key preserved');
248+
assert.equal(config.mcpServers[claudeModule.MCP_SERVER_KEY], undefined, 'gx removed');
249+
});
250+
251+
test('uninstallMcpServer is a no-op when no .mcp.json exists', () => {
252+
const repoRoot = makeRepo();
253+
const result = claudeModule.uninstallMcpServer(repoRoot, { dryRun: false });
254+
assert.equal(result.status, 'absent');
255+
});
256+
182257
test('agent_branch_advisor.py is a managed (distributed) hook file', () => {
183258
assert.ok(
184259
claudeModule.MANAGED_HOOK_FILES.includes('agent_branch_advisor.py'),

0 commit comments

Comments
 (0)