diff --git a/packages/cli/README.md b/packages/cli/README.md index 534b5bbb..a3b9ceed 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -529,7 +529,7 @@ agent-workforce agent --clean [@] ### Session layout -Both the skill install root and the clean mount live under a single +Both the skill install root and the sandbox mount live under a single session directory. The session id (`--`) is generated once and both paths are derived from it: diff --git a/packages/cli/package.json b/packages/cli/package.json index e1d78db8..16c9b8b2 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -15,7 +15,7 @@ "dependencies": { "@agentworkforce/harness-kit": "workspace:*", "@agentworkforce/workload-router": "workspace:*", - "@relayfile/local-mount": "^0.2.0" + "@relayfile/local-mount": "^0.5.0" }, "repository": { "type": "git", diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index 66724238..8d5e3576 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -1,7 +1,13 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { CLEAN_IGNORED_PATTERNS, decideCleanMode, parseAgentArgs } from './cli.js'; +import { + CLEAN_IGNORED_PATTERNS, + SKILL_INSTALL_IGNORED_PATTERNS, + decideCleanMode, + parseAgentArgs, + resolveSystemPromptPlaceholders +} from './cli.js'; // The conflict-detection path inside parseAgentArgs uses the module-local // `die()` helper, which calls process.exit(1) after writing to stderr. Tests @@ -117,25 +123,36 @@ test('parseAgentArgs: -- stops flag parsing, positional args after are preserved assert.deepEqual(positional, ['--install-in-repo', 'posthog']); }); -test('decideCleanMode: clean=false → no-op regardless of harness', () => { +test('decideCleanMode: claude + clean=false → no mount', () => { assert.deepEqual(decideCleanMode('claude', false), { useClean: false }); +}); + +test('decideCleanMode: codex + clean=false → no mount', () => { assert.deepEqual(decideCleanMode('codex', false), { useClean: false }); - assert.deepEqual(decideCleanMode('opencode', false), { useClean: false }); +}); + +test('decideCleanMode: opencode defaults to mount (skills would otherwise land in repo)', () => { + // Opencode has no installRoot support in the SDK, so the mount is the only + // way to keep `.opencode/skills/`, `.agents/skills/`, prpm.lock, etc. out + // of the real repo. Default-on for non-in-repo runs. + assert.deepEqual(decideCleanMode('opencode', false), { useClean: true }); + assert.deepEqual(decideCleanMode('opencode', true), { useClean: true }); +}); + +test('decideCleanMode: opencode + --install-in-repo → no mount', () => { + assert.deepEqual(decideCleanMode('opencode', false, true), { useClean: false }); + assert.deepEqual(decideCleanMode('opencode', true, true), { useClean: false }); }); test('decideCleanMode: claude + clean → engaged', () => { assert.deepEqual(decideCleanMode('claude', true), { useClean: true }); }); -test('decideCleanMode: non-claude + clean → disengaged with warning naming harness', () => { +test('decideCleanMode: codex + clean → disengaged with warning naming harness', () => { const codex = decideCleanMode('codex', true); assert.equal(codex.useClean, false); assert.match(codex.warning ?? '', /claude harness/); assert.match(codex.warning ?? '', /codex/); - - const opencode = decideCleanMode('opencode', true); - assert.equal(opencode.useClean, false); - assert.match(opencode.warning ?? '', /opencode/); }); test('CLEAN_IGNORED_PATTERNS: covers the declared repo-level claude config files', () => { @@ -149,6 +166,46 @@ test('CLEAN_IGNORED_PATTERNS: covers the declared repo-level claude config files ]); }); +test('resolveSystemPromptPlaceholders: substitutes with the active harness', () => { + const input = + 'produce the exact install command: `npx -y prpm install --as ` for prpm using the active harness'; + const out = resolveSystemPromptPlaceholders(input, 'opencode'); + assert.match(out, /--as opencode/); + assert.ok(!out.includes(''), 'expected placeholder to be resolved'); + // Other angle-bracket placeholders (, , etc.) are deliberately + // preserved — they are LLM-facing template variables. + assert.ok(out.includes('')); +}); + +test('resolveSystemPromptPlaceholders: resolves every occurrence (not just the first)', () => { + const out = resolveSystemPromptPlaceholders(' then again', 'codex'); + assert.equal(out, 'codex then codex again'); +}); + +test('resolveSystemPromptPlaceholders: leaves prompts without the placeholder untouched', () => { + const original = 'You are a code reviewer. No placeholders here.'; + assert.equal(resolveSystemPromptPlaceholders(original, 'claude'), original); +}); + +test('SKILL_INSTALL_IGNORED_PATTERNS: keeps skill-install artifacts out of the real repo', () => { + // Pinned — non-claude sessions rely on these to prevent `.opencode/skills/`, + // `.agents/skills/`, and skill.sh per-provider symlink farms from being + // copied into the mount or synced back on exit. Shrinking this set + // re-introduces repo pollution from `npx prpm install` / `npx skills add`; + // expand via review, not silently. + assert.deepEqual([...SKILL_INSTALL_IGNORED_PATTERNS], [ + '.agents', + '.claude/skills', + '.factory/skills', + '.kiro/skills', + 'skills', + '.opencode', + '.skills', + 'prpm.lock', + 'skills-lock.json' + ]); +}); + // Integration-ish subprocess helper: spawn the built CLI, collect stderr, // and return once the child exits. We force the harness binary to fail to // spawn (PATH scrubbed) so these runs terminate quickly regardless of what @@ -216,7 +273,7 @@ test('main: --clean on an interactive non-claude session warns and proceeds with // npm-provenance-publisher@best runs on codex. --clean should warn and the // run should continue down the non-mount spawn path (which then fails to // spawn codex because PATH is scrubbed). We should NEVER see a - // "clean mount → …" line, which the clean branch emits before calling + // "sandbox mount → …" line, which the mount branch emits before calling // launchOnMount. const { stderr } = await runCliCapturingStderr([ 'agent', @@ -229,7 +286,7 @@ test('main: --clean on an interactive non-claude session warns and proceeds with 'expected non-claude clean warning to surface' ); assert.ok( - !/clean mount →/.test(stderr), - `expected the clean mount branch to be skipped; saw stderr:\n${stderr}` + !/sandbox mount →/.test(stderr), + `expected the mount branch to be skipped; saw stderr:\n${stderr}` ); }); diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 212724dc..7b600bb3 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import { spawn, spawnSync } from 'node:child_process'; import { randomBytes } from 'node:crypto'; +import { rmSync } from 'node:fs'; import { constants, homedir } from 'node:os'; import { join, resolve as resolvePath } from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -42,19 +43,28 @@ Commands: Flags: --install-in-repo Install skills into the repo's - .claude/skills/ (legacy). By default, + harness-conventional directory + (.claude/skills, .opencode/skills, + .agents/skills, etc.). By default, interactive claude sessions stage skills under ~/.agent-workforce/ - sessions// and pass --plugin-dir - so the repo is never touched. + sessions// and pass --plugin-dir; + interactive opencode sessions run + inside a @relayfile/local-mount + sandbox so npx prpm install / npx + skills add writes never touch the + real repo. --clean Launch interactive claude inside a @relayfile/local-mount sandbox that - hides the repo's CLAUDE.md, + also hides the repo's CLAUDE.md, CLAUDE.local.md, .claude, and .mcp.json from the session. Persona skills and keychain auth are - preserved. Claude interactive only; - incompatible with --install-in-repo. + preserved. No-op for opencode + (mount is already on by default); + codex still warns and proceeds + without a mount. Incompatible + with --install-in-repo. list [flags] List available personas from the cascade (pwd → home → library). By default shows one row per persona at the recommended tier for its intent; pass --all to see every @@ -144,11 +154,31 @@ function parseSelector(sel: string): ResolvedTarget { return { kind, spec: result, tier }; } +/** + * Resolve the `` placeholder used in persona systemPrompts. + * Personas embed `` inside example install commands (e.g. + * `npx prpm install --as `) so the docstring stays + * harness-agnostic in source. The active harness is known at selection + * time, so swap it in here to give the model a concrete command. + * + * Exported for test coverage. Only `` is resolved today; other + * angle-bracketed tokens in the prompt (e.g. ``, ``, + * ``) are deliberately left as LLM-facing placeholders. + */ +export function resolveSystemPromptPlaceholders(prompt: string, harness: Harness): string { + return prompt.replaceAll('', harness); +} + function buildSelection(spec: PersonaSpec, tier: PersonaTier, kind: 'repo' | 'local'): PersonaSelection { + const rawRuntime = spec.tiers[tier]; + const runtime = { + ...rawRuntime, + systemPrompt: resolveSystemPromptPlaceholders(rawRuntime.systemPrompt, rawRuntime.harness) + }; return { personaId: spec.id, tier, - runtime: spec.tiers[tier], + runtime, skills: spec.skills, rationale: kind === 'local' ? `local-override: ${spec.id}` : `cli-tier-override: ${tier}`, ...(spec.env ? { env: spec.env } : {}), @@ -209,18 +239,61 @@ function signalExitCode(signal: NodeJS.Signals | null): number { return 128 + (num ?? 1); } -function runInstall(command: readonly string[], label: string): void { +/** + * Derive a meaningful exit code from a `spawnSync` result. `spawnSync` + * sets `status` to null and `signal` to the signal name (e.g. `SIGINT`) + * when the child was killed before it could set its own exit code, so + * a naive `res.status ?? 1` collapses Ctrl-C / SIGTERM onto generic + * failure instead of the conventional 128+N. + */ +function subprocessExitCode(res: ReturnType): number { + if (res.status !== null) return res.status; + if (res.signal) return signalExitCode(res.signal); + return 1; +} + +function runInstall(command: readonly string[], label: string, cwd?: string): void { const [bin, ...args] = command; if (!bin) return; process.stderr.write(`• ${label}\n`); - const res = spawnSync(bin, args, { stdio: 'inherit', shell: false }); - if (res.status !== 0) { - const code = res.status ?? 1; + const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, ...(cwd ? { cwd } : {}) }); + const code = subprocessExitCode(res); + if (code !== 0) { process.stderr.write(`${label} failed (exit ${code}). Aborting.\n`); process.exit(code); } } +/** + * Thrown by `runInstallOrThrow` when the install subprocess exits non-zero. + * Carries the underlying exit code so the mount-branch catch can surface it + * to the user instead of collapsing every failure onto 127. + */ +class InstallCommandError extends Error { + readonly exitCode: number; + constructor(label: string, exitCode: number) { + super(`${label} failed (exit ${exitCode})`); + this.name = 'InstallCommandError'; + this.exitCode = exitCode; + } +} + +/** + * Install variant that throws instead of calling `process.exit` on failure. + * Used inside `launchOnMount`'s `onBeforeLaunch`, so mount teardown runs + * before the error surfaces. + */ +function runInstallOrThrow(command: readonly string[], label: string, cwd: string): void { + const [bin, ...args] = command; + if (!bin) return; + process.stderr.write(`• ${label}\n`); + const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, cwd }); + const code = subprocessExitCode(res); + if (code !== 0) { + throw new InstallCommandError(label, code); + } +} + function runCleanup(command: readonly string[], commandString: string): void { if (commandString === ':') return; const [bin, ...args] = command; @@ -234,10 +307,17 @@ function runCleanup(command: readonly string[], commandString: string): void { * created. The workload-router cleanup only covers the install subtree * (`/claude/plugin`), so without this step empty parent dirs * would accumulate under `~/.agent-workforce/sessions/`. + * + * Uses `fs.rmSync` rather than `spawnSync('rm', …)` so the teardown works + * on Windows where `rm` isn't on PATH. */ function removeSessionRoot(sessionRoot: string | undefined): void { if (!sessionRoot) return; - spawnSync('rm', ['-rf', sessionRoot], { stdio: 'ignore', shell: false }); + try { + rmSync(sessionRoot, { recursive: true, force: true }); + } catch { + /* best-effort — if teardown fails the dir is harmless under ~/.agent-workforce/sessions */ + } } /** @@ -277,23 +357,66 @@ export const CLEAN_IGNORED_PATTERNS = [ ] as const; /** - * Decide whether `--clean` should engage for this run. Returns a warning - * string when the flag was requested but cannot apply (e.g. non-claude - * harness); callers emit the warning to stderr and proceed with - * `useClean: false`. Pure — no side effects, trivially testable. + * Skill-install artifacts that should never be copied into the mount nor + * synced back to the real repo. Applied to non-claude interactive sessions + * that rely on the mount to keep `npx skills add` / `npx prpm install` + * writes out of the user's project tree. Covers every per-provider output + * root that skill.sh / prpm scatter into on install — missing one here + * re-introduces repo pollution, so this list is deliberately superset-y. + * Claude sessions use `installRoot` for out-of-repo staging instead, so + * these patterns don't apply there. + */ +export const SKILL_INSTALL_IGNORED_PATTERNS = [ + // skill.sh universal install root + per-harness symlink farms + '.agents', + '.claude/skills', + '.factory/skills', + '.kiro/skills', + 'skills', + // prpm `--as ` output roots + '.opencode', + '.skills', + // provider lockfiles written at the repo root + 'prpm.lock', + 'skills-lock.json' +] as const; + +/** + * Decide whether to run the interactive session inside a + * `@relayfile/local-mount` sandbox. + * + * - Claude: mount only engages when the user passes `--clean` explicitly + * (its purpose there is to hide CLAUDE.md / .claude / .mcp.json from the + * session). Out-of-repo skill staging is handled separately via + * `installRoot` + `--plugin-dir`. + * - Opencode: the SDK cannot stage skills out-of-repo for this harness + * (there is no `installRoot` support), so the mount is the only way to + * keep `npx prpm install` / `npx skills add` writes out of the project. + * Default to mount unless the user opts in with `--install-in-repo`. + * - Codex: no auto-mount yet. `--clean` still emits the existing + * "claude-only" warning so current behavior is preserved. + * + * Pure — no side effects, trivially testable. */ export function decideCleanMode( harness: Harness, - clean: boolean + clean: boolean, + installInRepo = false ): { useClean: boolean; warning?: string } { - if (!clean) return { useClean: false }; - if (harness !== 'claude') { + if (harness === 'claude') { + return { useClean: clean }; + } + if (harness === 'opencode') { + if (installInRepo) return { useClean: false }; + return { useClean: true }; + } + if (clean) { return { useClean: false, warning: `--clean is only supported for the claude harness (this session uses ${harness}). Ignoring flag.` }; } - return { useClean: true }; + return { useClean: false }; } async function runInteractive( @@ -301,20 +424,31 @@ async function runInteractive( options: { installInRepo?: boolean; clean?: boolean } = {} ): Promise { const { runtime, personaId, tier } = selection; - // Out-of-repo staging only applies to the claude harness today (it's the - // only one with a `--plugin-dir` hook). For codex/opencode, keep the legacy - // behavior of installing into the repo's harness-conventional directory. - // The --install-in-repo flag forces legacy behavior across the board. - const useSessionDir = !options.installInRepo && runtime.harness === 'claude'; - const sessionRoot = useSessionDir ? generateSessionRoot(personaId) : undefined; - const installRoot = sessionRoot ? sessionInstallRoot(sessionRoot) : undefined; - // --clean applies to interactive claude only. For non-claude harnesses, warn - // and drop the flag (same pattern as pluginDirs warnings in buildInteractiveSpec). - const cleanDecision = decideCleanMode(runtime.harness, options.clean === true); + // `installRoot` (out-of-repo skill staging via `--plugin-dir`) is currently + // claude-only; the workload-router SDK throws if it's set for other + // harnesses. For opencode, we instead keep installs out of the repo by + // running them inside a @relayfile/local-mount sandbox (see `useClean` + // below). The --install-in-repo flag forces legacy in-repo installs + // across the board. + const cleanDecision = decideCleanMode( + runtime.harness, + options.clean === true, + options.installInRepo === true + ); if (cleanDecision.warning) { process.stderr.write(`warning: ${cleanDecision.warning}\n`); } const useClean = cleanDecision.useClean; + // A session dir is needed whenever we either (a) stage skills out-of-repo + // via claude's installRoot, or (b) open a mount. Opencode reaches (b) by + // default; claude reaches both when --clean is set, and just (a) otherwise. + const useSessionDir = + !options.installInRepo && (runtime.harness === 'claude' || useClean); + const sessionRoot = useSessionDir ? generateSessionRoot(personaId) : undefined; + const installRoot = + sessionRoot && runtime.harness === 'claude' + ? sessionInstallRoot(sessionRoot) + : undefined; const ctx = useSelection( selection, installRoot !== undefined ? { installRoot } : {} @@ -334,14 +468,19 @@ async function runInteractive( // the plugin scaffold (mkdir + manifest + symlink) so `--plugin-dir` has a // valid target even for skill-less personas like posthog. Gate on the // command string rather than `installs.length` so we don't skip that. - if (install.commandString !== ':') { - const skillIds = install.plan.installs.map((i) => i.skillId).join(', '); - const targetLabel = installRoot ? ` → ${installRoot}` : ''; - const label = - install.plan.installs.length === 0 - ? `Staging session plugin dir${targetLabel}` - : `Installing skills: ${skillIds}${targetLabel}`; - runInstall(install.command, label); + const skillIds = install.plan.installs.map((i) => i.skillId).join(', '); + const installLabel = + install.plan.installs.length === 0 + ? `Staging session plugin dir${installRoot ? ` → ${installRoot}` : ''}` + : `Installing skills: ${skillIds}${installRoot ? ` → ${installRoot}` : ''}`; + // When useClean engages on a non-claude harness, the install must run + // INSIDE the mount so `.opencode/skills/`, `.agents/skills/`, prpm.lock, + // etc. land in the sandbox rather than the real repo. We defer it to + // `onBeforeLaunch` below instead of pre-running here. + const deferInstallToMount = + useClean && runtime.harness !== 'claude' && install.commandString !== ':'; + if (install.commandString !== ':' && !deferInstallToMount) { + runInstall(install.command, installLabel); } const spec = buildInteractiveSpec({ @@ -376,42 +515,132 @@ async function runInteractive( } } if (spec.initialPrompt) summary.push('initial-prompt='); - if (useClean) summary.push('clean=on'); + if (useClean) summary.push('mount=on'); process.stderr.write(`• spawning ${spec.bin} (${summary.join(', ')})\n`); - // --clean branch: delegate process lifecycle (spawn, signal forwarding, - // syncback, cleanup) to @relayfile/local-mount. Skill plugin dir lives - // outside the mount, so claude resolves --plugin-dir normally regardless - // of cwd. + // Mount branch: delegate process lifecycle (spawn, signal forwarding, + // syncback, cleanup) to @relayfile/local-mount. + // + // For claude: mount engages only when `--clean` is set, to hide the repo's + // claude config from the session. Skill plugin dir lives outside the mount + // at an absolute path, so claude resolves `--plugin-dir` normally. + // + // For opencode: mount engages by default (unless `--install-in-repo`), and + // the install itself runs inside the mount via `onBeforeLaunch` so that + // `npx prpm install` / `npx skills add` writes land in the sandbox. The + // skill-install paths are added to `ignoredPatterns` so they are neither + // copied in from the real repo nor synced back on exit. if (useClean && sessionRoot) { const mountDir = sessionMountDir(sessionRoot); - process.stderr.write(`• clean mount → ${mountDir}\n`); + const ignoredPatterns: string[] = + runtime.harness === 'claude' + ? [...CLEAN_IGNORED_PATTERNS] + : [...SKILL_INSTALL_IGNORED_PATTERNS]; + process.stderr.write(`• sandbox mount → ${mountDir}\n`); + // Three-stage SIGINT handler layered on top of launchOnMount's own signal + // forwarding. launchOnMount catches the first SIGINT to kill the child + // and run its finalize() (autoSync.stop + final syncBack), which walks + // both trees and can take several seconds on a large repo — during + // which the terminal otherwise looks frozen. + // + // 1st press → announce "syncing…" so the user knows the pause is real. + // 2nd press → abort `shutdownSignal`, which local-mount 0.5+ respects + // by skipping autosync's draining reconcile and returning + // the partial count from the final syncBack. Cleanup + // still runs, so no leaked mount dir. + // 3rd press → hard escape: synchronously rm the mount root and + // process.exit(130) in case the abort never resolves. + const shutdownController = new AbortController(); + let sigintCount = 0; + const forceExitHandler = () => { + sigintCount += 1; + if (sigintCount === 1) { + process.stderr.write( + '\n⏳ Syncing session changes back to the repo… (press Ctrl-C again to skip sync)\n' + ); + return; + } + if (sigintCount === 2) { + process.stderr.write( + '\n⚠ Aborting sync — partial changes will be propagated. (press Ctrl-C again to force quit)\n' + ); + shutdownController.abort(); + return; + } + process.stderr.write( + '\n✗ Force-quit: mount teardown skipped. Session dir may be left behind.\n' + ); + // Node-native removal rather than `rm -rf` so the emergency path + // works on Windows too. + try { + rmSync(sessionRoot, { recursive: true, force: true }); + } catch { + /* swallow — we're exiting anyway */ + } + process.exit(130); + }; + process.on('SIGINT', forceExitHandler); try { const result = await launchOnMount({ cli: spec.bin, projectDir: process.cwd(), mountDir, args: finalArgs, - ignoredPatterns: [...CLEAN_IGNORED_PATTERNS], + ignoredPatterns, // launchOnMount passes `env` straight to the child spawn, so without // merging process.env we'd strip PATH/HOME/etc. Match the non-clean // branch: persona env overlays the inherited environment. env: resolvedEnv ? { ...process.env, ...resolvedEnv } : process.env, - agentName: personaId + agentName: personaId, + // Second Ctrl-C aborts this signal → local-mount skips autosync's + // draining reconcile and returns the partial syncBack count. Cleanup + // still runs, so there's no leaked mount dir. + shutdownSignal: shutdownController.signal, + // Report sync stats so the user sees confirmation rather than a + // silent pause between the child exiting and the CLI returning. + onAfterSync: (count) => { + if (count > 0) { + const qualifier = shutdownController.signal.aborted ? ' (partial)' : ''; + process.stderr.write(`✓ Synced ${count} change(s) back to the repo${qualifier}.\n`); + } + }, + ...(deferInstallToMount + ? { + onBeforeLaunch: (dir: string) => { + runInstallOrThrow(install.command, installLabel, dir); + } + } + : {}) }); return result.exitCode; } catch (err) { + // InstallCommandError carries the real install exit code — surfacing + // it (rather than collapsing onto 127) lets callers distinguish a + // failed `npx prpm install` from a missing harness binary. + if (err instanceof InstallCommandError) { + process.stderr.write(`${err.message}. Aborting.\n`); + return err.exitCode; + } const e = err as NodeJS.ErrnoException; if (e.code === 'ENOENT') { process.stderr.write( - `Failed to spawn "${spec.bin}" inside clean mount: binary not found on PATH. Install the ${runtime.harness} CLI and retry.\n` + `Failed to spawn "${spec.bin}" inside sandbox mount: binary not found on PATH. Install the ${runtime.harness} CLI and retry.\n` ); - } else { - process.stderr.write(`Failed to launch clean mount: ${e.message}\n`); + return 127; } - return 127; + process.stderr.write(`Failed to launch sandbox mount: ${e.message}\n`); + return 1; } finally { - runCleanup(install.cleanupCommand, install.cleanupCommandString); + process.removeListener('SIGINT', forceExitHandler); + // When the install ran inside the mount, its cleanup paths are + // mount-relative (e.g. `.skills/`, `skills/`) and + // running cleanup here would resolve them against the real repo + // cwd — potentially `rm -rf`ing pre-existing user content. The + // mount dir is removed wholesale by `removeSessionRoot` below, so + // the install's cleanup is redundant anyway in that case. + if (!deferInstallToMount) { + runCleanup(install.cleanupCommand, install.cleanupCommandString); + } removeSessionRoot(sessionRoot); } } diff --git a/packages/harness-kit/src/harness.test.ts b/packages/harness-kit/src/harness.test.ts index c902b9d7..35a1d87c 100644 --- a/packages/harness-kit/src/harness.test.ts +++ b/packages/harness-kit/src/harness.test.ts @@ -107,15 +107,17 @@ test('codex warns when mcpServers / permissions are declared', () => { assert.match(result.warnings[1], /codex harness is not yet wired for runtime permission/); }); -test('opencode carries system prompt and strips provider prefix', () => { +test('opencode carries system prompt via --prompt flag (not as trailing positional cwd)', () => { const result = buildInteractiveSpec({ harness: 'opencode', model: 'opencode/minimax-m2.5', systemPrompt: 'x' }); assert.equal(result.bin, 'opencode'); - assert.deepEqual(result.args, ['--model', 'minimax-m2.5']); - assert.equal(result.initialPrompt, 'x'); + assert.deepEqual(result.args, ['--model', 'minimax-m2.5', '--prompt', 'x']); + // initialPrompt must be null so the caller does not append systemPrompt as + // a trailing positional — opencode would interpret it as a project dir. + assert.equal(result.initialPrompt, null); }); test('claude branch appends --plugin-dir per entry in pluginDirs', () => { diff --git a/packages/harness-kit/src/harness.ts b/packages/harness-kit/src/harness.ts index fdf7639a..4353fc10 100644 --- a/packages/harness-kit/src/harness.ts +++ b/packages/harness-kit/src/harness.ts @@ -13,8 +13,10 @@ export interface InteractiveSpec { /** * If set, the caller should append this as the final positional argument * — used by harnesses that don't support a separate system-prompt flag - * (codex, opencode) to carry the persona's system prompt as the initial - * user prompt. + * to carry the persona's system prompt as the initial user prompt. + * Currently only codex takes this path; opencode uses its own `--prompt` + * flag (wired directly into `args`) and claude uses `--append-system-prompt`, + * so both return `null` here. */ initialPrompt: string | null; /** @@ -60,10 +62,18 @@ function hasAnyPermission(p: PersonaPermissions | undefined): boolean { * so the spawned session only sees the persona's declared MCP servers, * never the user's or project's Claude Code config. * - * codex / opencode branches carry the system prompt as the initial - * positional `[PROMPT]` because neither CLI supports a separate - * system-prompt flag today. They emit a warning if the persona declares - * mcpServers or permissions — those features aren't wired for those + * The codex branch carries the system prompt via `initialPrompt` because + * codex has no dedicated system-prompt flag today — callers append it as + * the final positional `[PROMPT]`. + * + * The opencode branch uses opencode's own `--prompt` flag (wired directly + * into `args`) and returns `initialPrompt: null`. The earlier behavior of + * appending the prompt as a trailing positional was unsafe: `opencode`'s + * bare form treats a trailing positional as a project directory, which + * caused the TUI to fail with "Failed to change directory to …". + * + * Both codex and opencode emit a warning if the persona declares + * `mcpServers` or `permissions` — those features aren't wired for those * harnesses yet. */ export function buildInteractiveSpec(input: BuildInteractiveSpecInput): InteractiveSpec { @@ -138,10 +148,14 @@ export function buildInteractiveSpec(input: BuildInteractiveSpecInput): Interact 'pluginDirs is currently claude-only; ignoring under the opencode harness. Skills must be staged via opencode conventions.' ); } + // opencode's bare form is `opencode [project]` where the trailing + // positional is a project directory, NOT a prompt. Carry the persona's + // system prompt via `--prompt` (top-level TUI flag) so it isn't parsed + // as a cwd. return { bin: 'opencode', - args: ['--model', stripProviderPrefix(model)], - initialPrompt: systemPrompt, + args: ['--model', stripProviderPrefix(model), '--prompt', systemPrompt], + initialPrompt: null, warnings }; } diff --git a/packages/workload-router/src/index.test.ts b/packages/workload-router/src/index.test.ts index f5c26890..597b5a1b 100644 --- a/packages/workload-router/src/index.test.ts +++ b/packages/workload-router/src/index.test.ts @@ -139,37 +139,35 @@ test('legacy tier override remains available via resolvePersonaByTier', () => { assert.match(result.rationale, /legacy-tier-override/); }); -test('resolvePersona propagates env, mcpServers, and permissions to the selection', () => { - // posthog is the library's canonical carrier for all three optional fields: - // env.POSTHOG_API_KEY, mcpServers.posthog (http transport with bearer - // header), and permissions.allow auto-approving posthog MCP tools. +test('resolvePersona propagates mcpServers and permissions to the selection', () => { + // posthog is the library's canonical carrier for mcpServers + permissions. + // It used to also carry env.POSTHOG_API_KEY, but the persona switched to + // mcp-remote/OAuth (stdio, no env, no bearer header) in e3342c7. Env + // propagation through the loader is covered by the cli package's + // local-personas cascade tests. const selection = resolvePersona('posthog'); assert.equal(selection.personaId, 'posthog'); - // env is exposed on the selection and still holds the literal $VAR form — - // interpolation is the runner's job, not resolvePersona's. - assert.equal(selection.env?.POSTHOG_API_KEY, '$POSTHOG_API_KEY'); - - // mcpServers carry through including headers (runner interpolates later). + // mcpServers carry through with the full stdio shape (command + args). const posthogServer = selection.mcpServers?.posthog; assert.ok(posthogServer, 'expected mcpServers.posthog on the selection'); - assert.equal(posthogServer.type, 'http'); - if (posthogServer.type === 'http') { - assert.equal(posthogServer.url, 'https://mcp.posthog.com/mcp'); - assert.equal( - posthogServer.headers?.Authorization, - 'Bearer ${POSTHOG_API_KEY}' - ); + assert.equal(posthogServer.type, 'stdio'); + if (posthogServer.type === 'stdio') { + assert.equal(posthogServer.command, 'npx'); + assert.deepEqual(posthogServer.args, [ + '-y', + 'mcp-remote@latest', + 'https://mcp.posthog.com/mcp' + ]); } // permissions.allow is carried without modification. assert.deepEqual(selection.permissions?.allow, ['mcp__posthog']); }); -test('resolvePersonaByTier also propagates env / mcpServers / permissions', () => { +test('resolvePersonaByTier also propagates mcpServers / permissions', () => { const selection = resolvePersonaByTier('posthog', 'minimum'); assert.equal(selection.tier, 'minimum'); - assert.ok(selection.env, 'env should flow through tier override resolver'); assert.ok(selection.mcpServers, 'mcpServers should flow through tier override resolver'); assert.ok(selection.permissions, 'permissions should flow through tier override resolver'); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9b0c9ad2..d9600be7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -30,8 +30,8 @@ importers: specifier: workspace:* version: link:../workload-router '@relayfile/local-mount': - specifier: ^0.2.0 - version: 0.2.0 + specifier: ^0.5.0 + version: 0.5.0 packages/harness-kit: dependencies: @@ -88,14 +88,96 @@ packages: resolution: {integrity: sha512-wgm9Ehl2jpeqP3zw/7mo3kRHFp5MEDhqAdwy1fTGkHAwnkGOVsgpvQhL8B5n1qlb01jV3n/bI0ZfZp5lWA1k4w==} engines: {node: '>=18.0.0'} + '@parcel/watcher-android-arm64@2.5.6': + resolution: {integrity: sha512-YQxSS34tPF/6ZG7r/Ih9xy+kP/WwediEUsqmtf0cuCV5TPPKw/PQHRhueUo6JdeFJaqV3pyjm0GdYjZotbRt/A==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [android] + + '@parcel/watcher-darwin-arm64@2.5.6': + resolution: {integrity: sha512-Z2ZdrnwyXvvvdtRHLmM4knydIdU9adO3D4n/0cVipF3rRiwP+3/sfzpAwA/qKFL6i1ModaabkU7IbpeMBgiVEA==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [darwin] + + '@parcel/watcher-darwin-x64@2.5.6': + resolution: {integrity: sha512-HgvOf3W9dhithcwOWX9uDZyn1lW9R+7tPZ4sug+NGrGIo4Rk1hAXLEbcH1TQSqxts0NYXXlOWqVpvS1SFS4fRg==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [darwin] + + '@parcel/watcher-freebsd-x64@2.5.6': + resolution: {integrity: sha512-vJVi8yd/qzJxEKHkeemh7w3YAn6RJCtYlE4HPMoVnCpIXEzSrxErBW5SJBgKLbXU3WdIpkjBTeUNtyBVn8TRng==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [freebsd] + + '@parcel/watcher-linux-arm-glibc@2.5.6': + resolution: {integrity: sha512-9JiYfB6h6BgV50CCfasfLf/uvOcJskMSwcdH1PHH9rvS1IrNy8zad6IUVPVUfmXr+u+Km9IxcfMLzgdOudz9EQ==} + engines: {node: '>= 10.0.0'} + cpu: [arm] + os: [linux] + + '@parcel/watcher-linux-arm-musl@2.5.6': + resolution: {integrity: sha512-Ve3gUCG57nuUUSyjBq/MAM0CzArtuIOxsBdQ+ftz6ho8n7s1i9E1Nmk/xmP323r2YL0SONs1EuwqBp2u1k5fxg==} + engines: {node: '>= 10.0.0'} + cpu: [arm] + os: [linux] + + '@parcel/watcher-linux-arm64-glibc@2.5.6': + resolution: {integrity: sha512-f2g/DT3NhGPdBmMWYoxixqYr3v/UXcmLOYy16Bx0TM20Tchduwr4EaCbmxh1321TABqPGDpS8D/ggOTaljijOA==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [linux] + + '@parcel/watcher-linux-arm64-musl@2.5.6': + resolution: {integrity: sha512-qb6naMDGlbCwdhLj6hgoVKJl2odL34z2sqkC7Z6kzir8b5W65WYDpLB6R06KabvZdgoHI/zxke4b3zR0wAbDTA==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [linux] + + '@parcel/watcher-linux-x64-glibc@2.5.6': + resolution: {integrity: sha512-kbT5wvNQlx7NaGjzPFu8nVIW1rWqV780O7ZtkjuWaPUgpv2NMFpjYERVi0UYj1msZNyCzGlaCWEtzc+exjMGbQ==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [linux] + + '@parcel/watcher-linux-x64-musl@2.5.6': + resolution: {integrity: sha512-1JRFeC+h7RdXwldHzTsmdtYR/Ku8SylLgTU/reMuqdVD7CtLwf0VR1FqeprZ0eHQkO0vqsbvFLXUmYm/uNKJBg==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [linux] + + '@parcel/watcher-win32-arm64@2.5.6': + resolution: {integrity: sha512-3ukyebjc6eGlw9yRt678DxVF7rjXatWiHvTXqphZLvo7aC5NdEgFufVwjFfY51ijYEWpXbqF5jtrK275z52D4Q==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [win32] + + '@parcel/watcher-win32-ia32@2.5.6': + resolution: {integrity: sha512-k35yLp1ZMwwee3Ez/pxBi5cf4AoBKYXj00CZ80jUz5h8prpiaQsiRPKQMxoLstNuqe2vR4RNPEAEcjEFzhEz/g==} + engines: {node: '>= 10.0.0'} + cpu: [ia32] + os: [win32] + + '@parcel/watcher-win32-x64@2.5.6': + resolution: {integrity: sha512-hbQlYcCq5dlAX9Qx+kFb0FHue6vbjlf0FrNzSKdYK2APUf7tGfGxQCk2ihEREmbR6ZMc0MVAD5RIX/41gpUzTw==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [win32] + + '@parcel/watcher@2.5.6': + resolution: {integrity: sha512-tmmZ3lQxAe/k/+rNnXQRawJ4NjxO2hqiOLTHvWchtGZULp4RyFeh6aU4XdOYBFe2KE1oShQTv4AblOs2iOrNnQ==} + engines: {node: '>= 10.0.0'} + '@relaycast/sdk@1.1.0': resolution: {integrity: sha512-9mCpcinrwNxA5BJjWtv3mrI8onior88bHtUZaSCaEaSlXwhhY1Q4Rw2JggOFuDYpBM7MumVHzuc6ZajVSwTYHw==} '@relaycast/types@1.1.0': resolution: {integrity: sha512-d0zByxvWK2PeZRnrhzbmDkE8Yar84/XH1H+89qGJj3lA82kTf/7Z76a2cqOIoxFXLgIjK959w9hTctXLem+lhA==} - '@relayfile/local-mount@0.2.0': - resolution: {integrity: sha512-OF83jJdvgDtCPbJ6/U8n7lHuX2tcGcWvUGKblH0M+vnZv9OEWsDnt7WXnJRM8kjSsBWPjfzuZdxsJboFYg1TeQ==} + '@relayfile/local-mount@0.5.0': + resolution: {integrity: sha512-gz/onW56+Qg7WELNopS46+hnEl5BWzlOVZ0WJ4lleL01OlSfFvxNcvJkxh1GNJaQEDCA0W37OoKxKfOdjGZ+RQ==} engines: {node: '>=18'} '@relayfile/sdk@0.1.6': @@ -133,10 +215,6 @@ packages: resolution: {integrity: sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==} engines: {node: '>=10'} - chokidar@4.0.3: - resolution: {integrity: sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA==} - engines: {node: '>= 14.16.0'} - chownr@3.0.0: resolution: {integrity: sha512-+IxzY9BZOQd/XuYPRmrvEVjF/nqj5kgT4kEq7VofrDoM1MxoRjEWkrCC3EtLi59TVawxTAn+orJwFQcrqEN1+g==} engines: {node: '>=18'} @@ -160,6 +238,10 @@ packages: resolution: {integrity: sha512-Vw8qHK3bZM9y/P10u3Vib8o/DdkvA2OtPtZvD871QKjy74Wj1WSKFILMPRPSdUSx5RFK1arlJzEtA4PkFgnbuA==} engines: {node: '>=18'} + detect-libc@2.1.2: + resolution: {integrity: sha512-Btj2BOOO83o3WyH59e8MgXsxEQVcarkUOpEYrubB0urwnN10yQ364rsiByU11nZlqWYZm05i/of7io4mzihBtQ==} + engines: {node: '>=8'} + emoji-regex@10.6.0: resolution: {integrity: sha512-toUI84YS5YmxW219erniWD0CIVOo46xGKColeNQRgOzDorgBi1v4D71/OFzgD9GO2UGKIv1C3Sp8DAn0+j5w7A==} @@ -182,10 +264,18 @@ packages: resolution: {integrity: sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg==} engines: {node: '>= 4'} + is-extglob@2.1.1: + resolution: {integrity: sha512-SbKbANkN603Vi4jEZv49LeVJMn4yGwsbzZworEoyEiutsN3nJYdbO36zfhGJ6QEDpOZIFkDtnq5JRxmvl3jsoQ==} + engines: {node: '>=0.10.0'} + is-fullwidth-code-point@5.1.0: resolution: {integrity: sha512-5XHYaSyiqADb4RnZ1Bdad6cPp8Toise4TzEjcOYDHZkTCbKgiUl7WTUCpNWHuxmDt91wnsZBc9xinNzopv3JMQ==} engines: {node: '>=18'} + is-glob@4.0.3: + resolution: {integrity: sha512-xelSayHH36ZgE7ZWhli7pW34hNbNl8Ojv5KVmkJD4hBdD3th8Tfk9vYasLM+mXWOZhFkgZfxhLSnrwRr4elSSg==} + engines: {node: '>=0.10.0'} + listr2@10.2.1: resolution: {integrity: sha512-7I5knELsJKTUjXG+A6BkKAiGkW1i25fNa/xlUl9hFtk15WbE9jndA89xu5FzQKrY5llajE1hfZZFMILXkDHk/Q==} engines: {node: '>=22.13.0'} @@ -206,6 +296,9 @@ packages: resolution: {integrity: sha512-KZxYo1BUkWD2TVFLr0MQoM8vUUigWD3LlD83a/75BqC+4qE0Hb1Vo5v1FgcfaNXvfXzr+5EhQ6ing/CaBijTlw==} engines: {node: '>= 18'} + node-addon-api@7.1.1: + resolution: {integrity: sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==} + onetime@7.0.0: resolution: {integrity: sha512-VXJjc87FScF88uafS3JllDgvAm+c/Slfz06lorj2uAY34rlUu0Nt+v8wreiImcrgAjjIHp1rXpTDlLOGw29WwQ==} engines: {node: '>=18'} @@ -213,9 +306,9 @@ packages: picocolors@1.1.1: resolution: {integrity: sha512-xceH2snhtb5M9liqDsmEw56le376mTZkEX/jEb/RxNFyegNul7eNslCXP9FDj/Lcu0X8KEyMceP2ntpaHrDEVA==} - readdirp@4.1.2: - resolution: {integrity: sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg==} - engines: {node: '>= 14.18.0'} + picomatch@4.0.4: + resolution: {integrity: sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==} + engines: {node: '>=12'} restore-cursor@5.1.0: resolution: {integrity: sha512-oMA2dcrw6u0YfxJQXm342bFKX/E4sG9rbTzO9ptUcR/e8A33cHuvStiYOwH7fszkZlZ1z/ta9AAoPk2F4qIOHA==} @@ -345,6 +438,66 @@ snapshots: dependencies: minipass: 7.1.3 + '@parcel/watcher-android-arm64@2.5.6': + optional: true + + '@parcel/watcher-darwin-arm64@2.5.6': + optional: true + + '@parcel/watcher-darwin-x64@2.5.6': + optional: true + + '@parcel/watcher-freebsd-x64@2.5.6': + optional: true + + '@parcel/watcher-linux-arm-glibc@2.5.6': + optional: true + + '@parcel/watcher-linux-arm-musl@2.5.6': + optional: true + + '@parcel/watcher-linux-arm64-glibc@2.5.6': + optional: true + + '@parcel/watcher-linux-arm64-musl@2.5.6': + optional: true + + '@parcel/watcher-linux-x64-glibc@2.5.6': + optional: true + + '@parcel/watcher-linux-x64-musl@2.5.6': + optional: true + + '@parcel/watcher-win32-arm64@2.5.6': + optional: true + + '@parcel/watcher-win32-ia32@2.5.6': + optional: true + + '@parcel/watcher-win32-x64@2.5.6': + optional: true + + '@parcel/watcher@2.5.6': + dependencies: + detect-libc: 2.1.2 + is-glob: 4.0.3 + node-addon-api: 7.1.1 + picomatch: 4.0.4 + optionalDependencies: + '@parcel/watcher-android-arm64': 2.5.6 + '@parcel/watcher-darwin-arm64': 2.5.6 + '@parcel/watcher-darwin-x64': 2.5.6 + '@parcel/watcher-freebsd-x64': 2.5.6 + '@parcel/watcher-linux-arm-glibc': 2.5.6 + '@parcel/watcher-linux-arm-musl': 2.5.6 + '@parcel/watcher-linux-arm64-glibc': 2.5.6 + '@parcel/watcher-linux-arm64-musl': 2.5.6 + '@parcel/watcher-linux-x64-glibc': 2.5.6 + '@parcel/watcher-linux-x64-musl': 2.5.6 + '@parcel/watcher-win32-arm64': 2.5.6 + '@parcel/watcher-win32-ia32': 2.5.6 + '@parcel/watcher-win32-x64': 2.5.6 + '@relaycast/sdk@1.1.0': dependencies: '@relaycast/types': 1.1.0 @@ -354,9 +507,9 @@ snapshots: dependencies: zod: 4.3.6 - '@relayfile/local-mount@0.2.0': + '@relayfile/local-mount@0.5.0': dependencies: - chokidar: 4.0.3 + '@parcel/watcher': 2.5.6 ignore: 7.0.5 '@relayfile/sdk@0.1.6': {} @@ -390,10 +543,6 @@ snapshots: ansi-styles: 4.3.0 supports-color: 7.2.0 - chokidar@4.0.3: - dependencies: - readdirp: 4.1.2 - chownr@3.0.0: {} cli-cursor@5.0.0: @@ -413,6 +562,8 @@ snapshots: commander@12.1.0: {} + detect-libc@2.1.2: {} + emoji-regex@10.6.0: {} environment@1.1.0: {} @@ -425,10 +576,16 @@ snapshots: ignore@7.0.5: {} + is-extglob@2.1.1: {} + is-fullwidth-code-point@5.1.0: dependencies: get-east-asian-width: 1.5.0 + is-glob@4.0.3: + dependencies: + is-extglob: 2.1.1 + listr2@10.2.1: dependencies: cli-truncate: 5.2.0 @@ -453,13 +610,15 @@ snapshots: dependencies: minipass: 7.1.3 + node-addon-api@7.1.1: {} + onetime@7.0.0: dependencies: mimic-function: 5.0.1 picocolors@1.1.1: {} - readdirp@4.1.2: {} + picomatch@4.0.4: {} restore-cursor@5.1.0: dependencies: