From 21670590898504205eef98d1295fa2730707a44a Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Mon, 20 Apr 2026 15:26:28 -0400 Subject: [PATCH 1/7] Fix mounts, installs, and opencode prompt handling Refactor interactive session behavior across harnesses: change decideCleanMode to account for opencode defaults and an --install-in-repo override, and add SKILL_INSTALL_IGNORED_PATTERNS to keep skill-install artifacts out of the real repo. Update CLI help text to clarify mount/install semantics. Allow runInstall to accept cwd, add runInstallOrThrow for error-safe installs inside mounts, and defer non-claude installs into the mount via onBeforeLaunch so writes land in the sandbox. Also adjust buildInteractiveSpec for opencode to pass system prompts via --prompt (and set initialPrompt to null). Tests updated to cover the new decisions, ignored patterns, and prompt behavior. --- packages/cli/src/cli.test.ts | 46 ++++-- packages/cli/src/cli.ts | 173 +++++++++++++++++------ packages/harness-kit/src/harness.test.ts | 8 +- packages/harness-kit/src/harness.ts | 8 +- 4 files changed, 182 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index 66724238..a24d9457 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -1,7 +1,12 @@ 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 +} 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 +122,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 +165,20 @@ test('CLEAN_IGNORED_PATTERNS: covers the declared repo-level claude config files ]); }); +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/` + // etc. 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', + '.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 diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 212724dc..eebfb135 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -42,19 +42,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 @@ -209,11 +218,11 @@ function signalExitCode(signal: NodeJS.Signals | null): number { return 128 + (num ?? 1); } -function runInstall(command: readonly string[], label: string): void { +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 }); + const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, ...(cwd ? { cwd } : {}) }); if (res.status !== 0) { const code = res.status ?? 1; process.stderr.write(`${label} failed (exit ${code}). Aborting.\n`); @@ -221,6 +230,22 @@ function runInstall(command: readonly string[], label: string): void { } } +/** + * 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 }); + if (res.status !== 0) { + const code = res.status ?? 1; + throw new Error(`${label} failed (exit ${code})`); + } +} + function runCleanup(command: readonly string[], commandString: string): void { if (commandString === ':') return; const [bin, ...args] = command; @@ -277,23 +302,56 @@ 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. Claude sessions use `installRoot` + * for out-of-repo staging instead, so these patterns don't apply there. + */ +export const SKILL_INSTALL_IGNORED_PATTERNS = [ + '.agents', + '.opencode', + '.skills', + '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 +359,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 +403,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({ @@ -379,12 +453,24 @@ async function runInteractive( if (useClean) summary.push('clean=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); + const ignoredPatterns: string[] = + runtime.harness === 'claude' + ? [...CLEAN_IGNORED_PATTERNS] + : [...SKILL_INSTALL_IGNORED_PATTERNS]; process.stderr.write(`• clean mount → ${mountDir}\n`); try { const result = await launchOnMount({ @@ -392,12 +478,19 @@ async function runInteractive( 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, + ...(deferInstallToMount + ? { + onBeforeLaunch: (dir: string) => { + runInstallOrThrow(install.command, installLabel, dir); + } + } + : {}) }); return result.exitCode; } catch (err) { 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..fa635807 100644 --- a/packages/harness-kit/src/harness.ts +++ b/packages/harness-kit/src/harness.ts @@ -138,10 +138,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 }; } From bccd02bfb97e6b1f6c0dc1eb1cd7ee604647468c Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Tue, 21 Apr 2026 15:58:16 -0400 Subject: [PATCH 2/7] Resolve prompts and add SIGINT handling Add resolveSystemPromptPlaceholders(prompt, harness) and tests to substitute the placeholder in persona systemPrompts while leaving other angle-bracket tokens intact. Use this resolver in buildSelection so runtime.systemPrompt contains the active harness. In runInteractive, add a two-stage SIGINT handler that notifies the user on first Ctrl-C (syncing message) and force-quits on second (removes session root and exits 130). Also add an onAfterSync callback to report synced change counts and ensure the SIGINT handler is removed in the finally block. These changes improve prompt correctness for LLM-facing commands and give users a clear escape hatch and feedback during session syncs. --- packages/cli/src/cli.test.ts | 24 ++++++++++++++- packages/cli/src/cli.ts | 59 +++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index a24d9457..9193eea8 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -5,7 +5,8 @@ import { CLEAN_IGNORED_PATTERNS, SKILL_INSTALL_IGNORED_PATTERNS, decideCleanMode, - parseAgentArgs + parseAgentArgs, + resolveSystemPromptPlaceholders } from './cli.js'; // The conflict-detection path inside parseAgentArgs uses the module-local @@ -165,6 +166,27 @@ 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/` // etc. from being copied into the mount or synced back on exit. Shrinking diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index eebfb135..5ea377d7 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -153,11 +153,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 } : {}), @@ -472,6 +492,35 @@ async function runInteractive( ? [...CLEAN_IGNORED_PATTERNS] : [...SKILL_INSTALL_IGNORED_PATTERNS]; process.stderr.write(`• clean mount → ${mountDir}\n`); + // Two-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. We announce "syncing…" on + // that first press, and on a second press we tear down the mount + // synchronously and exit 130 so the user always has an escape hatch. + 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 force quit)\n' + ); + return; + } + process.stderr.write( + '\n✗ Force-quit: session sync aborted. Any in-flight writes may be incomplete.\n' + ); + // Best-effort teardown — the mount dir lives under + // ~/.agent-workforce/sessions/ and would otherwise accumulate. + try { + spawnSync('rm', ['-rf', sessionRoot], { stdio: 'ignore', shell: false }); + } catch { + /* swallow — we're exiting anyway */ + } + process.exit(130); + }; + process.on('SIGINT', forceExitHandler); try { const result = await launchOnMount({ cli: spec.bin, @@ -484,6 +533,13 @@ async function runInteractive( // branch: persona env overlays the inherited environment. env: resolvedEnv ? { ...process.env, ...resolvedEnv } : process.env, agentName: personaId, + // 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) { + process.stderr.write(`✓ Synced ${count} change(s) back to the repo.\n`); + } + }, ...(deferInstallToMount ? { onBeforeLaunch: (dir: string) => { @@ -504,6 +560,7 @@ async function runInteractive( } return 127; } finally { + process.removeListener('SIGINT', forceExitHandler); runCleanup(install.cleanupCommand, install.cleanupCommandString); removeSessionRoot(sessionRoot); } From a406e1ba3baf8caf358551364b7d344496b827bd Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Tue, 21 Apr 2026 16:02:39 -0400 Subject: [PATCH 3/7] Bump local-mount to 0.5 and add 3-stage SIGINT Bump @relayfile/local-mount to ^0.5.0 and enhance CLI shutdown handling in runInteractive. Replace the previous two-stage SIGINT flow with a three-stage handler: 1) announce syncing on first Ctrl-C, 2) abort a new AbortController to skip autosync's draining reconcile (passed as shutdownSignal to the mount) and allow a partial syncBack, and 3) hard-quit by removing the mount root and exiting on a third Ctrl-C. Update user-facing messages and qualify the sync confirmation when an abort produces a partial sync. --- packages/cli/package.json | 2 +- packages/cli/src/cli.ts | 35 +++++-- pnpm-lock.yaml | 195 ++++++++++++++++++++++++++++++++++---- 3 files changed, 204 insertions(+), 28 deletions(-) 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.ts b/packages/cli/src/cli.ts index 5ea377d7..588189b3 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -492,27 +492,39 @@ async function runInteractive( ? [...CLEAN_IGNORED_PATTERNS] : [...SKILL_INSTALL_IGNORED_PATTERNS]; process.stderr.write(`• clean mount → ${mountDir}\n`); - // Two-stage SIGINT handler layered on top of launchOnMount's own signal + // 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. We announce "syncing…" on - // that first press, and on a second press we tear down the mount - // synchronously and exit 130 so the user always has an escape hatch. + // 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 force quit)\n' + '\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: session sync aborted. Any in-flight writes may be incomplete.\n' + '\n✗ Force-quit: mount teardown skipped. Session dir may be left behind.\n' ); - // Best-effort teardown — the mount dir lives under - // ~/.agent-workforce/sessions/ and would otherwise accumulate. try { spawnSync('rm', ['-rf', sessionRoot], { stdio: 'ignore', shell: false }); } catch { @@ -533,11 +545,16 @@ async function runInteractive( // branch: persona env overlays the inherited environment. env: resolvedEnv ? { ...process.env, ...resolvedEnv } : process.env, 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) { - process.stderr.write(`✓ Synced ${count} change(s) back to the repo.\n`); + const qualifier = shutdownController.signal.aborted ? ' (partial)' : ''; + process.stderr.write(`✓ Synced ${count} change(s) back to the repo${qualifier}.\n`); } }, ...(deferInstallToMount 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: From 2f41f6f5f7aeb60a71391616a73c504bd1186374 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Tue, 21 Apr 2026 16:21:06 -0400 Subject: [PATCH 4/7] Update posthog propagation tests for mcp-remote OAuth shape posthog.json switched to stdio + mcp-remote in e3342c7 (removing env.POSTHOG_API_KEY and the http transport with Bearer header), but the resolvePersona / resolvePersonaByTier tests still asserted the old shape and broke on main. Update them to check the new stdio shape (command + args) and drop the env assertion. Env loader coverage already lives in the cli local-personas cascade tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/workload-router/src/index.test.ts | 34 ++++++++++------------ 1 file changed, 16 insertions(+), 18 deletions(-) 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'); }); From 1790d7117c60107763926f835999b9dec1b18317 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Tue, 21 Apr 2026 16:46:48 -0400 Subject: [PATCH 5/7] Address PR #20 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Expand SKILL_INSTALL_IGNORED_PATTERNS to cover every skill.sh per-provider output root (.claude/skills, .factory/skills, .kiro/skills, skills) in addition to the prpm/opencode roots. Missing entries would let skill.sh copy pre-existing repo content into the mount and sync new installs back to the real repo — reintroducing the pollution this set is meant to prevent. 2. Skip the install.cleanupCommand in the mount branch when the install was deferred into the mount. Its paths are mount-relative (.skills/, skills/) and running it against the real repo cwd could rm -rf pre-existing user content with the same name. removeSessionRoot already tears down the whole mount, so per-skill cleanup is redundant in that case. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/cli.test.ts | 13 +++++++++---- packages/cli/src/cli.ts | 24 +++++++++++++++++++++--- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index 9193eea8..3c8b3527 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -188,12 +188,17 @@ test('resolveSystemPromptPlaceholders: leaves prompts without the placeholder un }); 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/` - // etc. 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. + // 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', diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 588189b3..60018e0e 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -325,13 +325,23 @@ export const CLEAN_IGNORED_PATTERNS = [ * 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. Claude sessions use `installRoot` - * for out-of-repo staging instead, so these patterns don't apply there. + * 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; @@ -578,7 +588,15 @@ async function runInteractive( return 127; } finally { process.removeListener('SIGINT', forceExitHandler); - runCleanup(install.cleanupCommand, install.cleanupCommandString); + // 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); } } From fbd756dc375335bfcefae544cc5b222c43157be0 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Tue, 21 Apr 2026 17:32:53 -0400 Subject: [PATCH 6/7] Address second-round PR #20 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Rename "clean mount" → "sandbox mount" in user-facing output. The log line fired whenever the mount engaged, including when opencode auto-enabled it without --clean, so the wording was misleading relative to the --clean flag's semantics. Also renamed the summary flag from clean=on → mount=on. 2. Propagate the real install exit code from the mount branch. Introduce InstallCommandError so runInstallOrThrow can carry the child's status up through launchOnMount's onBeforeLaunch and into the catch, instead of collapsing every failure onto 127 (command-not-found). Split the remaining catch branches so a generic launch failure returns 1 and a missing binary still returns 127. 3. Use fs.rmSync(..., { recursive: true, force: true }) instead of spawnSync('rm', '-rf', ...) in removeSessionRoot and the 3rd-press force-quit teardown so the emergency path works on Windows. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/README.md | 2 +- packages/cli/src/cli.test.ts | 6 ++--- packages/cli/src/cli.ts | 50 ++++++++++++++++++++++++++++-------- 3 files changed, 44 insertions(+), 14 deletions(-) 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/src/cli.test.ts b/packages/cli/src/cli.test.ts index 3c8b3527..8d5e3576 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -273,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', @@ -286,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 60018e0e..9236c153 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'; @@ -250,6 +251,20 @@ function runInstall(command: readonly string[], label: string, cwd?: string): vo } } +/** + * 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 @@ -261,8 +276,7 @@ function runInstallOrThrow(command: readonly string[], label: string, cwd: strin process.stderr.write(`• ${label}\n`); const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, cwd }); if (res.status !== 0) { - const code = res.status ?? 1; - throw new Error(`${label} failed (exit ${code})`); + throw new InstallCommandError(label, res.status ?? 1); } } @@ -279,10 +293,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 */ + } } /** @@ -480,7 +501,7 @@ 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`); // Mount branch: delegate process lifecycle (spawn, signal forwarding, @@ -501,7 +522,7 @@ async function runInteractive( runtime.harness === 'claude' ? [...CLEAN_IGNORED_PATTERNS] : [...SKILL_INSTALL_IGNORED_PATTERNS]; - process.stderr.write(`• clean mount → ${mountDir}\n`); + 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 @@ -535,8 +556,10 @@ async function runInteractive( 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 { - spawnSync('rm', ['-rf', sessionRoot], { stdio: 'ignore', shell: false }); + rmSync(sessionRoot, { recursive: true, force: true }); } catch { /* swallow — we're exiting anyway */ } @@ -577,15 +600,22 @@ async function runInteractive( }); 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 { process.removeListener('SIGINT', forceExitHandler); // When the install ran inside the mount, its cleanup paths are From de18a6b425f8b7b00ae85cc69ab5a25d1b369dda Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Tue, 21 Apr 2026 17:58:01 -0400 Subject: [PATCH 7/7] Address third-round PR #20 review comments 1. Map subprocess signal termination to 128+N in install helpers. spawnSync sets status=null + signal=SIGINT when a child is killed before it can set an exit code, so runInstall / runInstallOrThrow were collapsing Ctrl-C / SIGTERM during install onto a generic exit 1. Extract subprocessExitCode() that defers to signalExitCode when status is null, matching the async spawn path's conventions. InstallCommandError now carries the signal-derived code too. 2. Refresh the opencode-related docstrings in harness.ts. The opencode branch moved to --prompt + initialPrompt: null, but the earlier comments on InteractiveSpec.initialPrompt and buildInteractiveSpec's header still claimed opencode relied on the trailing-positional initial-prompt path. Updated them to reflect current behavior and note why the old path was unsafe (opencode parses the trailing positional as a project directory). Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/cli.ts | 22 ++++++++++++++++++---- packages/harness-kit/src/harness.ts | 22 ++++++++++++++++------ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 9236c153..7b600bb3 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -239,13 +239,26 @@ function signalExitCode(signal: NodeJS.Signals | null): number { return 128 + (num ?? 1); } +/** + * 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, ...(cwd ? { cwd } : {}) }); - if (res.status !== 0) { - const code = res.status ?? 1; + const code = subprocessExitCode(res); + if (code !== 0) { process.stderr.write(`${label} failed (exit ${code}). Aborting.\n`); process.exit(code); } @@ -275,8 +288,9 @@ function runInstallOrThrow(command: readonly string[], label: string, cwd: strin if (!bin) return; process.stderr.write(`• ${label}\n`); const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, cwd }); - if (res.status !== 0) { - throw new InstallCommandError(label, res.status ?? 1); + const code = subprocessExitCode(res); + if (code !== 0) { + throw new InstallCommandError(label, code); } } diff --git a/packages/harness-kit/src/harness.ts b/packages/harness-kit/src/harness.ts index fa635807..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 {