fix(claude): stop stale .mcp.json residue outranking a project switch, and gate stale builds #106

Merged
michal merged 3 commits from worktree-fix-statusline-project into main 2026-08-10 15:52:51 +00:00
7 changed files with 414 additions and 31 deletions
Showing only changes of commit 2a7bba11ea - Show all commits

View File

@@ -101,13 +101,48 @@ to open, and re-scoping it would silently change which skills sync into it.
⏵⏵ bypass permissions on · ← for agents
```
`mcpctl statusline` resolves the project from a directory-scoped `.mcp.json`
first (a repo that pinned itself wins), then the user-scope entry in
`.claude.json`, then a `.mcpctl-project` marker up the tree so a checkout that is scoped but not yet
wired still reports. It reads the directory from the JSON Claude Code pipes in,
so it follows `/cwd` rather than reporting wherever the binary was launched, and
prints **nothing** when no project is active — an empty status line beats one
saying "none" on every unrelated repo.
`mcpctl statusline` reads the directory from the JSON Claude Code pipes in, so it
follows `/cwd` rather than reporting wherever the binary was launched, and prints
**nothing** when no project is active — an empty status line beats one saying
"none" on every unrelated repo.
It then takes the project from the most deliberate source that names one:
1. a canonical `mcpctl` entry in that directory's `.mcp.json` — a repo that
pinned itself wins, and it is the scope Claude Code itself prefers when both
define that server name;
2. the user-scope entry in `.claude.json` — what `config claude --project`
writes, so a switch takes effect everywhere it is not overridden;
3. a **legacy** project-named entry in `.mcp.json` (`homeautomation`,
`docmost`, …), left by an mcpctl older than the constant server name;
4. a `.mcpctl-project` marker up the tree, so a checkout that is scoped but not
yet wired still reports.
> **Legacy entries rank below user scope on purpose.** They used to outrank it,
> which made switching look broken: a user-scope switch never rewrites a
> checkout's `.mcp.json`, so the leftover kept naming the old project for good.
> A pin is a decision; residue is not.
A server Claude Code has switched off for that directory (`disabledMcpServers` /
`disabledMcpjsonServers`) is skipped at every step — a disabled server is not
mounted, so naming its project would be a lie. A `.mcp.json` server that is in
neither list is still awaiting its approval prompt and does count, since blanking
the status line on a fresh checkout is the more confusing failure.
### When a directory contradicts a switch
Claude Code merges the two scopes rather than picking one, so switching in user
scope cannot clean up what a directory declares. `config claude` says so rather
than reporting plain success:
```
Warning: /path/to/repo/.mcp.json still registers 'homeautomation' for this
directory — mounted alongside 'sre', not replaced by it.
Re-run with --scope project to retire it, or delete the entry by hand.
```
A canonical entry pinned to another project gets the stronger wording — it
*overrides* the switch in that directory rather than sitting beside it.
### It is never installed over yours

View File

@@ -46,6 +46,8 @@ import {
mergeUserScopeServer,
userScopeProject,
activeProjectIn,
canonicalProjectIn,
legacyEntriesIn,
claudeJsonPath,
type McpJson,
type ClaudeJson,
@@ -104,11 +106,51 @@ function readMcpJson(path: string): McpJson | null {
}
}
/**
* Warnings about a `.mcp.json` in `dir` that contradicts a user-scope switch to
* `project`.
*
* Claude Code merges the two scopes rather than picking one, so a
* directory-scoped entry does not go away when you switch globally:
* - a canonical `mcpctl` entry shares the name, and project scope wins — the
* switch has no effect in this directory at all;
* - a legacy project-named entry has a *different* name, so it is simply
* mounted alongside and the old project keeps answering here.
* Either way the user is owed the file path, because nothing else will tell
* them. Exported for tests.
*/
export function shadowWarnings(dir: string, project: string | undefined): string[] {
if (project === undefined || project === '') return [];
const path = join(dir, '.mcp.json');
const parsed = readMcpJson(path);
if (parsed === null) return [];
const pinned = canonicalProjectIn(parsed);
if (pinned !== null && pinned !== project) {
return [
`Warning: ${path} pins '${MCPCTL_SERVER_NAME}' to '${pinned}' for this directory, which overrides the switch here.`,
` Re-run with --scope project to repoint it, or delete the '${MCPCTL_SERVER_NAME}' entry to follow the user-scope project.`,
];
}
const stale = legacyEntriesIn(parsed).filter((e) => e.project !== project);
if (stale.length > 0) {
const names = stale.map((e) => `'${e.server}'`).join(', ');
return [
`Warning: ${path} still registers ${names} for this directory — mounted alongside '${project}', not replaced by it.`,
` Re-run with --scope project to retire ${stale.length === 1 ? 'it' : 'them'}, or delete the ${stale.length === 1 ? 'entry' : 'entries'} by hand.`,
];
}
return [];
}
export interface ConfigCommandDeps {
configDeps: Partial<ConfigLoaderDeps>;
log: (...args: string[]) => void;
/** API client for the skills sync side-effect of `config claude --project`. Optional so existing call sites work; without it we skip the sync step. */
apiClient?: ApiClient;
/** Working directory to check for a shadowing `.mcp.json`. Injectable so tests need not chdir. */
cwd?: () => string;
}
export interface ConfigApiDeps {
@@ -124,6 +166,7 @@ const defaultDeps: ConfigCommandDeps = {
export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?: ConfigApiDeps): Command {
const { configDeps, log } = { ...defaultDeps, ...deps };
const cwd = deps?.cwd ?? ((): string => process.cwd());
// PR-5: api client used by `mcpctl config claude --project` to run the
// initial skills sync after wiring the .mcp.json. Threaded through from
// index.ts; falls back to apiDeps.client when not explicitly passed (the
@@ -406,6 +449,11 @@ export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?:
if (userScope) {
// The whole point of user scope: you do this once, not per checkout.
log('This applies in every directory — no need to re-run it per repo.');
// ...except where a directory-scoped entry contradicts it. That file
// is never rewritten by a user-scope switch, so staying silent is how
// a switch ends up looking like it did nothing: the status line keeps
// naming the old project, and its server keeps answering here.
for (const line of shadowWarnings(cwd(), opts.project)) log(line);
}
// PR-5: write project marker, run initial skills sync, install

View File

@@ -2,7 +2,16 @@ import { Command } from 'commander';
import { readFileSync } from 'node:fs';
import { join, resolve } from 'node:path';
import { homedir } from 'node:os';
import { activeProjectIn, claudeJsonPath, userScopeProject, type McpJson, type ClaudeJson } from '../config/claude-mcp.js';
import {
MCPCTL_SERVER_NAME,
canonicalProjectIn,
claudeJsonPath,
disabledServersFor,
legacyEntriesIn,
userScopeProject,
type McpJson,
type ClaudeJson,
} from '../config/claude-mcp.js';
import { findProjectMarker } from '../utils/project-marker.js';
/**
@@ -16,9 +25,10 @@ import { findProjectMarker } from '../utils/project-marker.js';
* `setStatus`.
*
* Claude Code pipes a JSON blob in on stdin (session id, model, workspace). We
* only need the directory the project is whatever `.mcp.json` there mounts,
* falling back to a `.mcpctl-project` marker up the tree so a checkout that is
* scoped but not yet wired still reports.
* only need the directory; the project is then resolved from the most
* deliberate source that names one — see the ranking in the action below.
* Whatever it reports has to be a project that is genuinely mounted, so a
* server Claude Code has switched off for that directory is skipped.
*
* Prints nothing at all when no project is active: an empty status line is
* better than one that says "none" on every unrelated repo you open.
@@ -53,25 +63,47 @@ export function resolveDirectory(input: StatusLineInput, fallback: string): stri
return input.workspace?.current_dir ?? input.workspace?.project_dir ?? input.cwd ?? fallback;
}
/** The project Claude Code's user-scope config mounts, or null. */
export function projectFromUserScope(path: string): string | null {
/** Claude Code's user-scope config, or null if it is missing or unreadable. */
export function readClaudeJson(path: string): ClaudeJson | null {
try {
return userScopeProject(JSON.parse(readFileSync(path, 'utf-8')) as ClaudeJson);
return JSON.parse(readFileSync(path, 'utf-8')) as ClaudeJson;
} catch {
return null;
}
}
/** The project `.mcp.json` in `dir` mounts, or null. */
export function projectFromMcpJson(dir: string): string | null {
/** The project Claude Code's user-scope config mounts, or null. */
export function projectFromUserScope(doc: ClaudeJson | null): string | null {
return userScopeProject(doc);
}
/** The `.mcp.json` in `dir`, or null if there isn't a readable one. */
export function readDirMcpJson(dir: string): McpJson | null {
try {
const parsed = JSON.parse(readFileSync(join(dir, '.mcp.json'), 'utf-8')) as McpJson;
return activeProjectIn(parsed);
return JSON.parse(readFileSync(join(dir, '.mcp.json'), 'utf-8')) as McpJson;
} catch {
return null;
}
}
/**
* The project the canonical `mcpctl` entry in `dir`'s `.mcp.json` pins, or null
* — skipped when Claude Code has that server switched off for `dir`.
*/
export function projectFromDirPin(mcpJson: McpJson | null, disabled: Set<string>): string | null {
if (disabled.has(MCPCTL_SERVER_NAME)) return null;
return canonicalProjectIn(mcpJson);
}
/**
* The project a *legacy* project-named entry in `dir`'s `.mcp.json` mounts, or
* null. Disabled entries are skipped, so a leftover the user already turned off
* in `/mcp` stops being reported.
*/
export function projectFromDirLegacy(mcpJson: McpJson | null, disabled: Set<string>): string | null {
return legacyEntriesIn(mcpJson).find((e) => !disabled.has(e.server))?.project ?? null;
}
/** Format for the status line. Empty string means "render nothing". */
export function formatStatus(project: string | null, prefix: string): string {
return project !== null && project !== '' ? `${prefix}${project}` : '';
@@ -96,12 +128,29 @@ export function createStatuslineCommand(deps?: Partial<StatuslineDeps>): Command
const input = opts.directory !== undefined ? {} : await readStdinJson();
const dir = opts.directory !== undefined ? resolve(opts.directory) : resolveDirectory(input, cwd());
// Directory-scoped wiring wins: a repo with its own .mcp.json entry has
// deliberately pinned itself, and that beats the global default.
let project = projectFromMcpJson(dir) ?? projectFromUserScope(claudeJsonPath());
const claudeJson = readClaudeJson(claudeJsonPath());
const mcpJson = readDirMcpJson(dir);
const disabled = disabledServersFor(claudeJson, dir);
// Ranked by how deliberate each source is, because a switch has to be
// able to win:
// 1. a canonical `mcpctl` entry in this directory's .mcp.json — a
// deliberate pin, and the scope Claude Code itself prefers when both
// define the same server name;
// 2. user scope — what `config claude --project` writes, so switching
// projects must beat anything less deliberate than a pin;
// 3. a *legacy* project-named entry in .mcp.json. This used to outrank
// user scope, which made a switch look like it had done nothing: the
// residue an older mcpctl left in a checkout is not a pin, and never
// gets rewritten by a user-scope switch, so it reported the old
// project forever;
// 4. the .mcpctl-project marker — the other thing `config claude`
// writes, and what skills sync already trusts.
let project =
projectFromDirPin(mcpJson, disabled)
?? projectFromUserScope(claudeJson)
?? projectFromDirLegacy(mcpJson, disabled);
if (project === null) {
// Not wired here (or wired above this directory) — the marker is the
// other thing `config claude` writes, and skills sync already trusts it.
const marker = await findProjectMarker(dir, homeDir()).catch(() => null);
project = marker?.project ?? null;
}

View File

@@ -100,16 +100,30 @@ export function isLegacyMcpctlEntry(name: string, entry: unknown): boolean {
return projectOfEntry(entry) === name;
}
/** The project the canonical `mcpctl` entry mounts, or null if there isn't one. */
export function canonicalProjectIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): string | null {
return projectOfEntry(config?.mcpServers?.[MCPCTL_SERVER_NAME]);
}
/**
* Legacy project-named entries still present, in file order.
*
* Kept separate from the canonical entry because the two mean different things
* to a reader: the canonical entry is a deliberate pin, a legacy entry is
* residue from an older mcpctl that nothing has cleaned up yet. Callers that
* rank sources (the status line) must be able to tell them apart.
*/
export function legacyEntriesIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): { server: string; project: string }[] {
const servers = config?.mcpServers;
if (!servers) return [];
return Object.entries(servers)
.filter(([name, entry]) => isLegacyMcpctlEntry(name, entry))
.map(([name]) => ({ server: name, project: name }));
}
/** The project currently mounted by `.mcp.json`, preferring the canonical entry. */
export function activeProjectIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): string | null {
const servers = config?.mcpServers;
if (!servers) return null;
const canonical = projectOfEntry(servers[MCPCTL_SERVER_NAME]);
if (canonical !== null) return canonical;
for (const [name, entry] of Object.entries(servers)) {
if (isLegacyMcpctlEntry(name, entry)) return name;
}
return null;
return canonicalProjectIn(config) ?? legacyEntriesIn(config)[0]?.project ?? null;
}
export interface MergeResult {
@@ -169,12 +183,39 @@ export function claudeJsonPath(env: NodeJS.ProcessEnv = process.env, homeDir?: s
: join(home, '.claude.json');
}
/** Per-directory state Claude Code keeps in `.claude.json`'s `projects` map. */
export interface ClaudeProjectEntry {
/** Servers switched off for this directory, whatever scope they came from. */
disabledMcpServers?: string[];
/** `.mcp.json` servers declined at the approval prompt. */
disabledMcpjsonServers?: string[];
[key: string]: unknown;
}
/** Shape of the bits of `.claude.json` we touch. Everything else is preserved. */
export interface ClaudeJson {
mcpServers?: Record<string, McpServerEntry>;
projects?: Record<string, ClaudeProjectEntry>;
[key: string]: unknown;
}
/**
* Server names Claude Code has switched off in `dir`.
*
* A disabled server is not mounted, so naming its project as "active" is a
* plain lie — this is what lets the status line skip one. Only *explicit*
* disables count: a `.mcp.json` server in neither list is pending its approval
* prompt, and treating pending as off would blank the status line on a fresh
* checkout, which is the more confusing failure.
*/
export function disabledServersFor(doc: ClaudeJson | null | undefined, dir: string): Set<string> {
const entry = doc?.projects?.[dir];
return new Set([
...(Array.isArray(entry?.disabledMcpServers) ? entry.disabledMcpServers : []),
...(Array.isArray(entry?.disabledMcpjsonServers) ? entry.disabledMcpjsonServers : []),
]);
}
/**
* Set the user-scope entry, returning the new document and any legacy
* project-named entries retired from it.

View File

@@ -359,4 +359,46 @@ describe('config claude — user scope', () => {
process.exitCode = prevExit;
expect(output.join('\n')).toContain("unknown --scope 'global'");
});
// A user-scope switch never rewrites a directory's .mcp.json, so anything of
// ours left in one keeps answering in that directory. Saying so is the only
// way the user finds out — the switch otherwise reports plain success.
describe('warns when the working directory contradicts the switch', () => {
const switchTo = async (project: string): Promise<string> => {
await createConfigCommand({ configDeps: {}, log, cwd: () => tmpDir })
.parseAsync(['claude', '--project', project, '--skip-skills', '--skip-ui'], { from: 'user' });
return output.join('\n');
};
it('names a legacy entry that stays mounted alongside the new project', async () => {
writeFileSync(join(tmpDir, '.mcp.json'), JSON.stringify({
mcpServers: { homeautomation: { command: 'mcpctl', args: ['mcp', '-p', 'homeautomation'] } },
}));
const out = await switchTo('sre');
expect(out).toContain(join(tmpDir, '.mcp.json'));
expect(out).toContain("'homeautomation'");
expect(out).toContain('mounted alongside');
});
it('says a canonical pin overrides the switch in that directory', async () => {
writeFileSync(join(tmpDir, '.mcp.json'), JSON.stringify({
mcpServers: { mcpctl: { command: 'mcpctl', args: ['mcp', '-p', 'docmost'] } },
}));
expect(await switchTo('sre')).toContain('overrides the switch here');
});
it('stays quiet when the directory already agrees, or wires nothing of ours', async () => {
writeFileSync(join(tmpDir, '.mcp.json'), JSON.stringify({
mcpServers: {
mcpctl: { command: 'mcpctl', args: ['mcp', '-p', 'sre'] },
'their-server': { command: 'docker', args: ['run', 'x'] },
},
}));
expect(await switchTo('sre')).not.toContain('Warning:');
});
it('stays quiet when there is no .mcp.json at all', async () => {
expect(await switchTo('sre')).not.toContain('Warning:');
});
});
});

View File

@@ -0,0 +1,121 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { createStatuslineCommand } from '../../src/commands/statusline.js';
/**
* The status line is what tells you which project you are in, so the property
* under test throughout is: after a switch, does it name the project you
* switched to?
*
* These drive the real command rather than the resolution helpers, because the
* bug they cover was in the *ranking* of sources, not in any one source.
*/
const bridge = (project: string): Record<string, unknown> => ({
command: 'mcpctl',
args: ['mcp', '-p', project],
});
let home: string;
let dir: string;
/** Claude Code's user-scope config, at the path `claudeJsonPath()` resolves. */
function writeClaudeJson(doc: unknown): void {
writeFileSync(join(home, '.claude.json'), JSON.stringify(doc));
}
function writeMcpJson(doc: unknown): void {
writeFileSync(join(dir, '.mcp.json'), JSON.stringify(doc));
}
/** Run `statusline` for `dir` and return exactly what it printed. */
async function statusline(): Promise<string> {
const out: string[] = [];
const cmd = createStatuslineCommand({ log: (l) => out.push(l), cwd: () => dir, homeDir: () => home });
await cmd.parseAsync(['--directory', dir], { from: 'user' });
return out.join('');
}
beforeEach(() => {
home = mkdtempSync(join(tmpdir(), 'mcpctl-statusline-home-'));
dir = mkdtempSync(join(tmpdir(), 'mcpctl-statusline-dir-'));
// Point claudeJsonPath() at the fake home; the CLI reads $CLAUDE_CONFIG_DIR
// first, which keeps this off the developer's real ~/.claude.json.
process.env['CLAUDE_CONFIG_DIR'] = home;
});
afterEach(() => {
delete process.env['CLAUDE_CONFIG_DIR'];
rmSync(home, { recursive: true, force: true });
rmSync(dir, { recursive: true, force: true });
});
describe('mcpctl statusline', () => {
it('reports the user-scope project when the directory wires nothing', async () => {
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
expect(await statusline()).toBe('mcpctl:sre');
});
it('prints nothing at all when no project is active', async () => {
writeClaudeJson({ mcpServers: {} });
expect(await statusline()).toBe('');
});
it('lets a canonical .mcp.json pin override the user-scope project', async () => {
// Same server name in both scopes: Claude Code prefers project scope, so a
// deliberate pin is genuinely what is mounted here.
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
writeMcpJson({ mcpServers: { mcpctl: bridge('docmost') } });
expect(await statusline()).toBe('mcpctl:docmost');
});
it('does not let a legacy project-named entry outrank a user-scope switch', async () => {
// The regression: an older mcpctl wrote `homeautomation` into a checkout,
// and a user-scope switch never rewrites that file — so the status line
// reported the old project forever and the switch looked like a no-op.
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
writeMcpJson({ mcpServers: { homeautomation: bridge('homeautomation') } });
expect(await statusline()).toBe('mcpctl:sre');
});
it('still reports a legacy entry when nothing more deliberate names a project', async () => {
writeClaudeJson({ mcpServers: {} });
writeMcpJson({ mcpServers: { homeautomation: bridge('homeautomation') } });
expect(await statusline()).toBe('mcpctl:homeautomation');
});
it('skips a directory server Claude Code has switched off', async () => {
// A disabled server is not mounted, so naming its project is a lie.
writeClaudeJson({
mcpServers: {},
projects: { [dir]: { disabledMcpServers: ['homeautomation'] } },
});
writeMcpJson({ mcpServers: { homeautomation: bridge('homeautomation') } });
expect(await statusline()).toBe('');
});
it('skips a disabled pin and falls through to the user-scope project', async () => {
writeClaudeJson({
mcpServers: { mcpctl: bridge('sre') },
projects: { [dir]: { disabledMcpjsonServers: ['mcpctl'] } },
});
writeMcpJson({ mcpServers: { mcpctl: bridge('docmost') } });
expect(await statusline()).toBe('mcpctl:sre');
});
it('falls back to a .mcpctl-project marker when nothing is wired', async () => {
writeClaudeJson({ mcpServers: {} });
writeFileSync(join(dir, '.mcpctl-project'), 'lab\n');
expect(await statusline()).toBe('mcpctl:lab');
});
it('honours a custom prefix', async () => {
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
const out: string[] = [];
const cmd = createStatuslineCommand({ log: (l) => out.push(l), cwd: () => dir, homeDir: () => home });
await cmd.parseAsync(['--directory', dir, '--prefix', 'proj '], { from: 'user' });
expect(out.join('')).toBe('proj sre');
});
});

View File

@@ -5,6 +5,9 @@ import {
projectOfEntry,
isLegacyMcpctlEntry,
activeProjectIn,
canonicalProjectIn,
legacyEntriesIn,
disabledServersFor,
} from '../../src/config/claude-mcp.js';
const bridge = (project: string): Record<string, unknown> => ({
@@ -62,6 +65,50 @@ describe('activeProjectIn', () => {
});
});
describe('canonicalProjectIn / legacyEntriesIn', () => {
it('tells a deliberate pin apart from pre-migration residue', () => {
const config = { mcpServers: { [MCPCTL_SERVER_NAME]: bridge('sre'), homeautomation: bridge('homeautomation') } };
expect(canonicalProjectIn(config)).toBe('sre');
expect(legacyEntriesIn(config)).toEqual([{ server: 'homeautomation', project: 'homeautomation' }]);
});
it('reports no canonical entry when only legacy ones are present', () => {
const config = { mcpServers: { docmost: bridge('docmost') } };
expect(canonicalProjectIn(config)).toBeNull();
expect(legacyEntriesIn(config)).toEqual([{ server: 'docmost', project: 'docmost' }]);
});
it('leaves servers that are not ours out of both', () => {
const config = { mcpServers: { other: { command: 'echo' } } };
expect(canonicalProjectIn(config)).toBeNull();
expect(legacyEntriesIn(config)).toEqual([]);
expect(legacyEntriesIn(null)).toEqual([]);
});
});
describe('disabledServersFor', () => {
const doc = {
projects: {
'/repo': { disabledMcpServers: ['homeautomation'], disabledMcpjsonServers: ['mcpctl'] },
'/other': { disabledMcpServers: ['sre'] },
},
};
it('unions both of Claude Code\'s disable lists for that directory', () => {
expect([...disabledServersFor(doc, '/repo')].sort()).toEqual(['homeautomation', 'mcpctl']);
});
it('is scoped to the directory asked about', () => {
expect([...disabledServersFor(doc, '/other')]).toEqual(['sre']);
expect([...disabledServersFor(doc, '/unknown')]).toEqual([]);
expect([...disabledServersFor(null, '/repo')]).toEqual([]);
});
it('survives a malformed entry rather than throwing on the status line', () => {
expect([...disabledServersFor({ projects: { '/repo': { disabledMcpServers: 'nope' } } }, '/repo')]).toEqual([]);
});
});
describe('mergeMcpctlServers', () => {
it('writes one constant entry regardless of project', () => {
const { config } = mergeMcpctlServers(null, { project: 'my-fancy-project' });