refactor(prime-agent): extract the /mcpctl switcher to typechecked source; pi --dry-run; docs
Some checks failed
CI/CD / lint (pull_request) Successful in 1m11s
CI/CD / test (pull_request) Successful in 1m24s
CI/CD / typecheck (pull_request) Successful in 3m20s
CI/CD / smoke (pull_request) Failing after 2m1s
CI/CD / build (pull_request) Successful in 2m37s
CI/CD / publish (pull_request) Has been skipped
Some checks failed
CI/CD / lint (pull_request) Successful in 1m11s
CI/CD / test (pull_request) Successful in 1m24s
CI/CD / typecheck (pull_request) Successful in 3m20s
CI/CD / smoke (pull_request) Failing after 2m1s
CI/CD / build (pull_request) Successful in 2m37s
CI/CD / publish (pull_request) Has been skipped
The prime-agent switcher existed only as a 275-line string literal inside
prime-agent-extension.ts, so nothing typechecked or linted it — the exact gap
that let a wrong ctx.ui.select() option shape ship in the pi extension. It now
lives at src/prime-agent-ext/mcpctl-switch.ts with a generator, a tsconfig
checking it against the real @earendil-works/pi-coding-agent types, eslint
coverage and an embed-freshness test, matching pi and opencode.
The extraction was verified byte-identical before any edit, so the behaviour
shipped today is exactly what was captured. Linting it then found six problems
in code nothing had ever checked: object-truthiness null guards, a nullable
string conditional and a missing return type. All behaviour-preserving to fix,
but exactly the class of thing that ships silently when nothing is looking.
Also:
- `config pi` gains --dry-run, the last agent without it.
- The SessionStart hook installer now drops untagged duplicates of its own
exact command — rows left behind before the marker existed, or by a suite
that used to write into a real ~/.claude. Invisible in the UI; they just run
the sync twice per session. A hook the user wrote is never touched, even one
calling `mcpctl skills sync` with different flags.
- docs/claude-integration.md and docs/prime-agent-extension.md, the two
integrations that had no page.
prime-agent deliberately keeps its per-project MCP entry name rather than the
constant `mcpctl` claude and opencode now use: its switcher already unmounts the
previous project, so it never accumulates entries, and re-keying auth.json from
mcp:<project> to mcp:mcpctl would give up per-project token caching and needs a
migration. Documented as its own change rather than folded in here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVwuCjuMoA13gmzYEfcrNP
This commit is contained in:
@@ -612,7 +612,8 @@ export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?:
|
||||
.option('--skip-skills', 'Skip the initial skills sync')
|
||||
.option('--settings <path>', 'pi settings.json path (default: ~/.pi/agent/settings.json)')
|
||||
.option('--pi-dir <path>', 'Override the pi agent home (default: ~/.pi/agent)')
|
||||
.action(async (opts: { project?: string; extensionDir?: string; skipSkills?: boolean; settings?: string; piDir?: string }) => {
|
||||
.option('--dry-run', 'Print what would change without writing or syncing')
|
||||
.action(async (opts: { project?: string; extensionDir?: string; skipSkills?: boolean; settings?: string; piDir?: string; dryRun?: boolean }) => {
|
||||
if (!opts.project) {
|
||||
log('Error: --project is required for mcpctl config pi');
|
||||
process.exitCode = 1;
|
||||
@@ -628,6 +629,20 @@ export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?:
|
||||
// custom --pi-dir is used (also keeps tests off the real ~/.mcpctl).
|
||||
const statePath = piHome ? join(opts.piDir!, 'pi-state.json') : piStatePath();
|
||||
|
||||
if (opts.dryRun === true) {
|
||||
log(JSON.stringify({
|
||||
pi: {
|
||||
settingsPath,
|
||||
extensionDir: extDest,
|
||||
statePath,
|
||||
skillsDir: opts.skipSkills === true ? '<skipped>' : skillsInstall,
|
||||
source: opts.extensionDir ?? '<embedded>',
|
||||
},
|
||||
action: 'install extension files + register in settings.json (extensions + skills) + write active project + sync skills',
|
||||
}, null, 2));
|
||||
return;
|
||||
}
|
||||
|
||||
// 1. Install the extension files into ~/.pi/agent/extensions/mcpctl/.
|
||||
// Default: write the embedded sources (works from an installed binary
|
||||
// with no source tree). --extension-dir overrides with a source-tree
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -100,6 +100,32 @@ export async function installManagedSessionHook(
|
||||
}
|
||||
}
|
||||
|
||||
// Drop untagged copies of the very command we manage.
|
||||
//
|
||||
// Before this installer carried a marker — and, for a long while, whenever
|
||||
// the test suite ran against the developer's real ~/.claude — an identical
|
||||
// but untagged row could be left behind. It is invisible in the UI and simply
|
||||
// runs the sync a second time on every session start. Scoped to an exact
|
||||
// string match on our own command, so a hook someone wrote themselves (even
|
||||
// one that also calls `mcpctl skills sync`, but with different flags) is
|
||||
// never touched.
|
||||
if (foundEntry) {
|
||||
for (const group of groups) {
|
||||
if (!Array.isArray(group?.hooks)) continue;
|
||||
const kept = group.hooks.filter((e) => e[MARKER_KEY] === true || e.command !== command);
|
||||
if (kept.length !== group.hooks.length) {
|
||||
group.hooks = kept;
|
||||
entryChanged = true;
|
||||
}
|
||||
}
|
||||
// A group we emptied is noise in the file.
|
||||
const nonEmpty = groups.filter((g) => !Array.isArray(g.hooks) || g.hooks.length > 0);
|
||||
if (nonEmpty.length !== groups.length) {
|
||||
settings.hooks.SessionStart = nonEmpty;
|
||||
entryChanged = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (!foundEntry) {
|
||||
groups.push({
|
||||
hooks: [{ type: 'command', command, [MARKER_KEY]: true }],
|
||||
|
||||
48
src/cli/tests/config/prime-agent-extension-embed.test.ts
Normal file
48
src/cli/tests/config/prime-agent-extension-embed.test.ts
Normal file
@@ -0,0 +1,48 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import {
|
||||
MCPCTL_SWITCH_EXTENSION,
|
||||
MCPCTL_SWITCH_EXTENSION_FILENAME,
|
||||
} from '../../src/config/prime-agent-extension.js';
|
||||
|
||||
/**
|
||||
* `mcpctl config prime-agent` installs the *embedded* copy of the switcher, not
|
||||
* the file in src/prime-agent-ext/. Editing the source without re-running the
|
||||
* generator therefore ships stale code while the repo looks correct.
|
||||
*
|
||||
* This matters more here than for the pi and opencode extensions: until the
|
||||
* source file existed, the switcher was only a string literal, so nothing
|
||||
* typechecked or linted it at all. The guarantee only holds while the two stay
|
||||
* in sync.
|
||||
*/
|
||||
const repoRoot = join(import.meta.dirname, '..', '..', '..', '..');
|
||||
const extDir = join(repoRoot, 'src', 'prime-agent-ext');
|
||||
|
||||
describe('embedded prime-agent switcher', () => {
|
||||
it('matches the source in src/prime-agent-ext (re-run scripts/generate-prime-agent-extension.ts)', () => {
|
||||
expect(MCPCTL_SWITCH_EXTENSION, 'stale — regenerate the embed')
|
||||
.toBe(readFileSync(join(extDir, 'mcpctl-switch.ts'), 'utf-8'));
|
||||
});
|
||||
|
||||
it('installs under the name prime-agent auto-discovers', () => {
|
||||
expect(MCPCTL_SWITCH_EXTENSION_FILENAME).toBe('mcpctl-switch.ts');
|
||||
});
|
||||
|
||||
it('is self-contained — the installed file has no mcpctl imports to resolve', () => {
|
||||
expect(MCPCTL_SWITCH_EXTENSION).not.toMatch(/from '@mcpctl\//);
|
||||
expect(MCPCTL_SWITCH_EXTENSION).not.toMatch(/from '\.\.\//);
|
||||
});
|
||||
|
||||
it('switches without re-installing itself or re-scoping the launch directory', () => {
|
||||
// Rewriting the extension file prime-agent has already loaded buys nothing;
|
||||
// writing a marker would silently re-scope whatever repo it was started in.
|
||||
expect(MCPCTL_SWITCH_EXTENSION).toContain("'--skip-extension'");
|
||||
expect(MCPCTL_SWITCH_EXTENSION).toContain("'--skip-marker'");
|
||||
});
|
||||
|
||||
it('publishes the active-project indicator, which is the only visible state', () => {
|
||||
expect(MCPCTL_SWITCH_EXTENSION).toContain('setStatus');
|
||||
expect(MCPCTL_SWITCH_EXTENSION).toContain("'turn_start'");
|
||||
});
|
||||
});
|
||||
@@ -104,3 +104,48 @@ describe('sessionhook', () => {
|
||||
expect(settings.hooks.SessionStart).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('untagged duplicates of the managed hook', () => {
|
||||
let tmp2: string;
|
||||
let settings: string;
|
||||
beforeEach(async () => {
|
||||
tmp2 = await mkdtemp(join(tmpdir(), 'mcpctl-hook-dupe-'));
|
||||
settings = join(tmp2, 'settings.json');
|
||||
});
|
||||
afterEach(async () => { await rm(tmp2, { recursive: true, force: true }); });
|
||||
|
||||
it('removes an identical row left behind before the marker existed', async () => {
|
||||
// Exactly the shape found in a real ~/.claude: one tagged row, one not.
|
||||
// Invisible in the UI; it just runs the sync twice every session.
|
||||
await writeFile(settings, JSON.stringify({
|
||||
hooks: {
|
||||
SessionStart: [
|
||||
{ hooks: [{ type: 'command', command: 'mcpctl skills sync --quiet' }] },
|
||||
{ hooks: [{ type: 'command', command: 'mcpctl skills sync --quiet', [MARKER_KEY]: true }] },
|
||||
],
|
||||
},
|
||||
}));
|
||||
const { updated } = await installManagedSessionHook('mcpctl skills sync --quiet', settings);
|
||||
expect(updated).toBe(true);
|
||||
|
||||
const parsed = JSON.parse(await readFile(settings, 'utf-8')) as {
|
||||
hooks: { SessionStart: Array<{ hooks: Array<Record<string, unknown>> }> };
|
||||
};
|
||||
const rows = parsed.hooks.SessionStart.flatMap((g) => g.hooks);
|
||||
expect(rows).toEqual([{ type: 'command', command: 'mcpctl skills sync --quiet', [MARKER_KEY]: true }]);
|
||||
});
|
||||
|
||||
it('leaves a hook the user wrote alone, even one that also calls mcpctl', async () => {
|
||||
await writeFile(settings, JSON.stringify({
|
||||
hooks: {
|
||||
SessionStart: [{ hooks: [{ type: 'command', command: 'mcpctl skills sync --project mine' }] }],
|
||||
},
|
||||
}));
|
||||
await installManagedSessionHook('mcpctl skills sync --quiet', settings);
|
||||
const parsed = JSON.parse(await readFile(settings, 'utf-8')) as {
|
||||
hooks: { SessionStart: Array<{ hooks: Array<{ command: string }> }> };
|
||||
};
|
||||
const rows = parsed.hooks.SessionStart.flatMap((g) => g.hooks).map((r) => r.command);
|
||||
expect(rows).toContain('mcpctl skills sync --project mine');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user