Files
mcpctl/src/cli/tests/config/pi-extension-embed.test.ts

74 lines
3.5 KiB
TypeScript
Raw Normal View History

fix(pi): repair the /mcpctl menu, skills target, and typecheck the extension The pi extension shipped in `src/pi-ext/` was covered by no tsconfig and no eslint config, so nothing ever checked it against pi's API. Pointing tsc at the published @earendil-works/pi-coding-agent types found the command surface to be inert. Fixes: - `/mcpctl` did nothing. `ctx.ui.select` takes `string[]` and returns the chosen string; it was called with `{value,label}` objects, so the menu rendered five `[object Object]` rows and `choice === "status"` never matched any branch. Labels are now plain strings mapped back to actions. - The headless branch returned a status string from a handler typed `Promise<void>`; pi drops it. Reports via notify instead. - "Sync skills" omitted `--agent pi`, writing into ~/.claude/skills — in an integration whose stated purpose is to not depend on ~/.claude — and said so in its own success message. It also ran execSync with `stdio: "inherit"`, painting raw output over pi's TUI, and interpolated the project name into a shell string. Now execFile with `--agent pi` and captured output. - Tool results typed `content[].type` as `string`; pi's AgentToolResult wants the `"text"` literal. - `callTool` asserted `Promise<unknown>` to `ToolCallResult`. - Sanitising MCP tool names to `[a-z0-9_]` can collide (`docs.search` vs `docs-search`). The colliding tool was silently never registered but still reported active, so its calls were forwarded to the first tool. Names are now disambiguated and tracked with the MCP tool they forward to. - `registerWithPi` rewrote settings.json even when nothing changed. Since parsing strips `//` comments, a no-op run destroyed them. Guards, so this class of bug can't return: - `src/pi-ext/tsconfig.json` checks the extension against the real published pi types (dev dependency, not a shim — a shim drifting from the published API is the exact failure being guarded). Wired into `pnpm typecheck`. - eslint now covers `src/pi-ext/*.ts` like every other source file. - A test fails if the embedded copy in `config/pi-extension.ts` is stale; editing the sources without regenerating silently shipped old code. Also: the branch added `config pi` without regenerating shell completions (the committed-completions test was failing), and the doc advertised `mcpctl pi sync-skills`, which does not exist. Both corrected, plus a note on the session-token vs `mcpctl_pat_` bearer difference that would bite against an authenticated `mcplocal serve`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
2026-08-08 17:30:42 +01:00
import { describe, it, expect } from 'vitest';
import { readFileSync } from 'node:fs';
import { join } from 'node:path';
import { PI_EXTENSION_FILES, PI_EXTENSION_FILENAMES } from '../../src/config/pi-extension.js';
/**
* `mcpctl config pi` installs the *embedded* copy of the extension, not the
* files in src/pi-ext/. Editing the sources without re-running the generator
* therefore ships stale code to users while the repo looks correct and the
* embedded copy is the one thing no typecheck covers. Same guarantee the
* completions check gives.
*/
const repoRoot = join(import.meta.dirname, '..', '..', '..', '..');
const piExtDir = join(repoRoot, 'src', 'pi-ext');
describe('embedded pi extension', () => {
it('matches the sources in src/pi-ext (re-run scripts/generate-pi-extension.ts)', () => {
for (const name of PI_EXTENSION_FILENAMES) {
const onDisk = readFileSync(join(piExtDir, name), 'utf-8');
expect(PI_EXTENSION_FILES[name], `${name} is embedded`).toBeDefined();
expect(PI_EXTENSION_FILES[name], `${name} is stale — regenerate the embed`).toBe(onDisk);
}
});
it('embeds every file the extension needs to load', () => {
// mcpctl-pi.ts imports ./mcp-http.js — installing one without the other
// yields an extension pi cannot load.
expect(Object.keys(PI_EXTENSION_FILES).sort()).toEqual(['mcp-http.ts', 'mcpctl-pi.ts']);
expect(PI_EXTENSION_FILES['mcpctl-pi.ts']).toContain('./mcp-http.js');
});
fix(pi-ext): stop importing @earendil-works/pi-ai at runtime The extension failed to load outright on older pi installs: Failed to load extension ".../mcpctl-pi.ts": Cannot find module '@earendil-works/pi-ai' pi doesn't resolve an extension's bare specifiers the ordinary way — it hands jiti a hard-coded alias table built from its own dependencies, and that table differs between pi distributions. `@earendil-works/pi-coding- agent` (0.84.1) aliases both the `@earendil-works/*` and legacy `@mariozechner/*` names; `@mariozechner/pi-coding-agent` (0.73.1) aliases only the old ones. Neither resolves the other's namespace, so a single import outside the intersection takes the whole extension down: every tool, the /mcpctl command, and the status line, all gone. The only thing we used from pi-ai was `StringEnum`, a six-line wrapper over `Type.Unsafe`. Inlined as a local `stringEnum` with byte-identical output, so `typebox` — aliased by every published pi — is now the sole bare runtime import. The call site also passes `description` through, which the pi-ai version was silently dropping. Guarded in tests/config/pi-extension-embed.test.ts: any runtime import in the embedded sources that isn't `node:`, relative, or typebox now fails. Verified against both installs with the same active project: 0.73.1 reproduced the error verbatim before the change and loads cleanly after, and 0.84.1 keeps registering the gate tool exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014tsRTqhEC7YYYYaP3cBqo8
2026-08-10 00:15:04 +01:00
/**
* pi resolves an extension's bare specifiers through a hard-coded alias table
* in its own loader, and that table is not the same across pi distributions:
* `@earendil-works/*` exists only in the newer packages, `@mariozechner/*`
* installs alias only the old names, and neither resolves the other. An
* import of a package outside the intersection makes the whole extension fail
* to load with `Cannot find module` every tool gone, on someone else's pi.
*
* `typebox` is aliased by every published pi, so it is the only safe bare
* runtime import. Type-only imports are erased before jiti resolves anything,
* so they may name whatever they like.
*/
it('imports nothing at runtime that some pi build cannot resolve', () => {
// `import x from "s"` / `import {..} from "s"` (but not `import type`),
// plus the side-effect form `import "s"`.
const runtimeImport =
/^\s*import\s+(?!type\s)[^;]*?from\s*["']([^"']+)["']|^\s*import\s*["']([^"']+)["']/gm;
const allowed = /^(node:|\.\/|\.\.\/|typebox$|typebox\/)/;
for (const name of PI_EXTENSION_FILENAMES) {
const src = PI_EXTENSION_FILES[name] ?? '';
for (const match of src.matchAll(runtimeImport)) {
const specifier = match[1] ?? match[2] ?? '';
expect(specifier, `${name} runtime-imports ${specifier}`).toMatch(allowed);
}
}
});
fix(pi): repair the /mcpctl menu, skills target, and typecheck the extension The pi extension shipped in `src/pi-ext/` was covered by no tsconfig and no eslint config, so nothing ever checked it against pi's API. Pointing tsc at the published @earendil-works/pi-coding-agent types found the command surface to be inert. Fixes: - `/mcpctl` did nothing. `ctx.ui.select` takes `string[]` and returns the chosen string; it was called with `{value,label}` objects, so the menu rendered five `[object Object]` rows and `choice === "status"` never matched any branch. Labels are now plain strings mapped back to actions. - The headless branch returned a status string from a handler typed `Promise<void>`; pi drops it. Reports via notify instead. - "Sync skills" omitted `--agent pi`, writing into ~/.claude/skills — in an integration whose stated purpose is to not depend on ~/.claude — and said so in its own success message. It also ran execSync with `stdio: "inherit"`, painting raw output over pi's TUI, and interpolated the project name into a shell string. Now execFile with `--agent pi` and captured output. - Tool results typed `content[].type` as `string`; pi's AgentToolResult wants the `"text"` literal. - `callTool` asserted `Promise<unknown>` to `ToolCallResult`. - Sanitising MCP tool names to `[a-z0-9_]` can collide (`docs.search` vs `docs-search`). The colliding tool was silently never registered but still reported active, so its calls were forwarded to the first tool. Names are now disambiguated and tracked with the MCP tool they forward to. - `registerWithPi` rewrote settings.json even when nothing changed. Since parsing strips `//` comments, a no-op run destroyed them. Guards, so this class of bug can't return: - `src/pi-ext/tsconfig.json` checks the extension against the real published pi types (dev dependency, not a shim — a shim drifting from the published API is the exact failure being guarded). Wired into `pnpm typecheck`. - eslint now covers `src/pi-ext/*.ts` like every other source file. - A test fails if the embedded copy in `config/pi-extension.ts` is stale; editing the sources without regenerating silently shipped old code. Also: the branch added `config pi` without regenerating shell completions (the committed-completions test was failing), and the doc advertised `mcpctl pi sync-skills`, which does not exist. Both corrected, plus a note on the session-token vs `mcpctl_pat_` bearer difference that would bite against an authenticated `mcplocal serve`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
2026-08-08 17:30:42 +01:00
it('carries the fixes the pi API requires', () => {
const main = PI_EXTENSION_FILES['mcpctl-pi.ts'] ?? '';
// ctx.ui.select takes string[] and returns the chosen string.
expect(main).not.toMatch(/select\([^)]*\[\s*\{\s*value:/);
// Skills must land in pi's tree, never ~/.claude: the sync passes
// --agent pi and no longer tells the user it wrote to Claude's tree.
expect(main).toContain('"--agent", "pi"');
expect(main).not.toContain('(into ~/.claude/skills)');
// Spawned without a shell, so the project name is never interpolated into
// a command string. Matches the call, not prose mentioning it.
expect(main).toContain('execFile("mcpctl", args');
expect(main).not.toMatch(/execSync\s*\(/);
});
});