From 08b8c98f87cf020b353418d3500452c2ea725853 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 22 Apr 2026 19:40:45 -0400 Subject: [PATCH 1/3] Wire opencode interactive through its agent abstraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opencode interactive was launching with `--model ` + `--prompt `, which failed on two counts: 1. `--prompt` pre-fills the TUI input buffer with the string as a *user* message, not the agent's instructions. The persona's system prompt ended up sitting unsent in the chat field. 2. `-m`/`--model` expects `provider/model` per opencode's own docs. Stripping the `opencode/` prefix left a bare model name that opencode could not resolve; it silently fell back to its default provider + model regardless of what the persona declared. Opencode's actual system-prompt + model pathway is its agent abstraction (https://opencode.ai/config.json): `agent..{ model, prompt, mode }`, selected at launch with `--agent `. Switching to that shape: - harness-kit: extend InteractiveSpec with `configFiles` so the opencode branch can emit a per-session `opencode.json` carrying the persona's agent definition (full provider/model + prompt). Builder now takes `personaId` as the agent name. Claude and codex return an empty configFiles array — only opencode needs it today. - cli: materialize spec.configFiles into the mount dir via `onBeforeLaunch` so opencode finds `opencode.json` at its cwd. The non-mount opencode path (only reachable under --install-in-repo) would pollute the user's repo root, so it degrades: strip --agent, warn, and launch with opencode's default agent. Documented trade-off is that --install-in-repo cannot apply the persona's prompt; mount mode (the default) handles this cleanly. Tests: opencode test replaced + two new tests for the opencode.json shape and claude/codex configFiles=[]; new cli tests for stripAgentFlag covering the happy path and a defensive case. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/cli.test.ts | 27 ++++++++- packages/cli/src/cli.ts | 54 +++++++++++++++-- packages/harness-kit/src/harness.test.ts | 69 ++++++++++++++++++++-- packages/harness-kit/src/harness.ts | 74 ++++++++++++++++++++---- packages/harness-kit/src/index.ts | 1 + 5 files changed, 204 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index 8d5e357..a5b8cf1 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -6,7 +6,8 @@ import { SKILL_INSTALL_IGNORED_PATTERNS, decideCleanMode, parseAgentArgs, - resolveSystemPromptPlaceholders + resolveSystemPromptPlaceholders, + stripAgentFlag } from './cli.js'; // The conflict-detection path inside parseAgentArgs uses the module-local @@ -144,6 +145,30 @@ test('decideCleanMode: opencode + --install-in-repo → no mount', () => { assert.deepEqual(decideCleanMode('opencode', true, true), { useClean: false }); }); +test('stripAgentFlag: removes --agent pair preserving surrounding args', () => { + // Degrade path: when the CLI cannot materialize opencode.json (non-mount + // --install-in-repo), it strips the --agent selector so opencode launches + // with its default agent rather than failing to resolve the unknown one. + assert.deepEqual( + stripAgentFlag(['--agent', 'persona-maker']), + [] + ); + assert.deepEqual( + stripAgentFlag(['--foo', '--agent', 'persona-maker', '--bar']), + ['--foo', '--bar'] + ); + assert.deepEqual( + stripAgentFlag(['--keep-me']), + ['--keep-me'] + ); +}); + +test('stripAgentFlag: trailing --agent without a value is preserved (caller decides)', () => { + // Defensive: don't swallow an argv that looks malformed — let the harness + // reject it so the bug surfaces instead of getting silently stripped. + assert.deepEqual(stripAgentFlag(['--agent']), ['--agent']); +}); + test('decideCleanMode: claude + clean → engaged', () => { assert.deepEqual(decideCleanMode('claude', true), { useClean: true }); }); diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 7b600bb..a9ad144 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,7 +1,7 @@ #!/usr/bin/env node import { spawn, spawnSync } from 'node:child_process'; import { randomBytes } from 'node:crypto'; -import { rmSync } from 'node:fs'; +import { rmSync, writeFileSync } from 'node:fs'; import { constants, homedir } from 'node:os'; import { join, resolve as resolvePath } from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -345,6 +345,24 @@ function sessionMountDir(sessionRoot: string): string { return join(sessionRoot, 'mount'); } +/** + * Remove a single `--agent ` pair from a harness argv. Used on the + * non-mount opencode path where we cannot safely materialize the persona's + * opencode.json (it would land in the user's real repo), so we fall back to + * launching opencode without a persona-specific agent selection. + */ +export function stripAgentFlag(args: readonly string[]): string[] { + const out: string[] = []; + for (let i = 0; i < args.length; i += 1) { + if (args[i] === '--agent' && i + 1 < args.length) { + i += 1; + continue; + } + out.push(args[i]); + } + return out; +} + /** Patterns hidden from an interactive claude session when `--clean` is set. * Applied by `@relayfile/local-mount` with gitignore semantics, so bare names * match at any depth in the project tree (e.g. `.claude` hides both @@ -485,6 +503,7 @@ async function runInteractive( const spec = buildInteractiveSpec({ harness: runtime.harness, + personaId, model: runtime.model, systemPrompt: runtime.systemPrompt, mcpServers: resolvedMcp, @@ -492,7 +511,29 @@ async function runInteractive( ...(installRoot !== undefined ? { pluginDirs: [installRoot] } : {}) }); for (const w of spec.warnings) process.stderr.write(`warning: ${w}\n`); - const finalArgs = spec.initialPrompt ? [...spec.args, spec.initialPrompt] : [...spec.args]; + + // Config-file materialization strategy: + // - Mount path (opencode default, claude --clean): write each configFile + // into the mount dir via onBeforeLaunch, so it lives only in the + // sandbox and is torn down with the session. + // - Non-mount path: today the only configFile producer is opencode + // (opencode.json for the --agent wiring), and the non-mount opencode + // path only engages under --install-in-repo. Writing opencode.json + // into the user's real repo would pollute the working tree, so we + // degrade: drop --agent from the argv, warn, and launch opencode with + // its default agent. The persona's prompt will not be applied in that + // mode; users who want it should drop --install-in-repo (the mount + // default handles this cleanly). + const hasConfigFiles = spec.configFiles.length > 0; + const degradeConfigFiles = hasConfigFiles && !useClean; + let effectiveArgs: readonly string[] = spec.args; + if (degradeConfigFiles) { + process.stderr.write( + 'warning: --install-in-repo cannot safely materialize the persona agent config (would write opencode.json into your repo); launching without --agent. Drop --install-in-repo to apply the persona prompt.\n' + ); + effectiveArgs = stripAgentFlag(spec.args); + } + const finalArgs = spec.initialPrompt ? [...effectiveArgs, spec.initialPrompt] : [...effectiveArgs]; // Print a sanitized summary rather than raw argv: spec.args for the claude // harness contains the resolved --mcp-config JSON and the full system @@ -604,10 +645,15 @@ async function runInteractive( process.stderr.write(`✓ Synced ${count} change(s) back to the repo${qualifier}.\n`); } }, - ...(deferInstallToMount + ...(deferInstallToMount || hasConfigFiles ? { onBeforeLaunch: (dir: string) => { - runInstallOrThrow(install.command, installLabel, dir); + if (deferInstallToMount) { + runInstallOrThrow(install.command, installLabel, dir); + } + for (const file of spec.configFiles) { + writeFileSync(join(dir, file.path), file.contents, 'utf8'); + } } } : {}) diff --git a/packages/harness-kit/src/harness.test.ts b/packages/harness-kit/src/harness.test.ts index 35a1d87..d396b8d 100644 --- a/packages/harness-kit/src/harness.test.ts +++ b/packages/harness-kit/src/harness.test.ts @@ -6,6 +6,7 @@ import { buildInteractiveSpec } from './harness.js'; test('claude branch always emits --mcp-config + --strict-mcp-config', () => { const result = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-opus-4-6', systemPrompt: 'you are a test' }); @@ -26,6 +27,7 @@ test('claude branch always emits --mcp-config + --strict-mcp-config', () => { test('claude branch serializes resolved mcpServers into the --mcp-config payload', () => { const result = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-sonnet-4-6', systemPrompt: 'x', mcpServers: { @@ -52,6 +54,7 @@ test('claude branch serializes resolved mcpServers into the --mcp-config payload test('claude branch translates permissions to flags', () => { const result = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-opus-4-6', systemPrompt: 'x', permissions: { @@ -74,6 +77,7 @@ test('claude branch translates permissions to flags', () => { test('claude branch omits permission flags when unset or empty', () => { const result = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-opus-4-6', systemPrompt: 'x', permissions: { allow: [], deny: [] } @@ -86,6 +90,7 @@ test('claude branch omits permission flags when unset or empty', () => { test('codex carries system prompt as initial positional; strips provider prefix from model', () => { const result = buildInteractiveSpec({ harness: 'codex', + personaId: 'test-persona', model: 'openai-codex/gpt-5.3-codex', systemPrompt: 'system-directive' }); @@ -97,6 +102,7 @@ test('codex carries system prompt as initial positional; strips provider prefix test('codex warns when mcpServers / permissions are declared', () => { const result = buildInteractiveSpec({ harness: 'codex', + personaId: 'test-persona', model: 'openai-codex/gpt-5.3-codex', systemPrompt: 'x', mcpServers: { foo: { type: 'http', url: 'https://example.com' } }, @@ -107,22 +113,70 @@ 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 via --prompt flag (not as trailing positional cwd)', () => { +test('opencode defines a per-persona agent in opencode.json and selects it with --agent', () => { const result = buildInteractiveSpec({ harness: 'opencode', + personaId: 'test-persona', model: 'opencode/minimax-m2.5', - systemPrompt: 'x' + systemPrompt: 'you are a test' }); assert.equal(result.bin, 'opencode'); - 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. + // No --prompt (that flag pre-fills the TUI input with the system prompt as + // a *user* message) and no bare -m (opencode's -m expects provider/model; + // earlier code stripped the provider and silently fell back to the default + // model). Model + system prompt now live in the emitted opencode.json, + // selected by persona id via --agent. + assert.deepEqual(result.args, ['--agent', 'test-persona']); + assert.ok(!result.args.includes('--prompt')); + assert.ok(!result.args.includes('--model')); + assert.ok(!result.args.includes('-m')); assert.equal(result.initialPrompt, null); }); +test('opencode configFiles carries a well-formed opencode.json with the agent definition', () => { + const result = buildInteractiveSpec({ + harness: 'opencode', + personaId: 'test-persona', + model: 'opencode/minimax-m2.5', + systemPrompt: 'you are a test' + }); + assert.equal(result.configFiles.length, 1); + const [file] = result.configFiles; + assert.equal(file.path, 'opencode.json'); + const parsed = JSON.parse(file.contents); + assert.deepEqual(parsed, { + agent: { + 'test-persona': { + model: 'opencode/minimax-m2.5', + prompt: 'you are a test', + mode: 'primary' + } + } + }); +}); + +test('claude and codex emit an empty configFiles array', () => { + const claude = buildInteractiveSpec({ + harness: 'claude', + personaId: 'test-persona', + model: 'claude-opus-4-6', + systemPrompt: 'x' + }); + assert.deepEqual(claude.configFiles, []); + + const codex = buildInteractiveSpec({ + harness: 'codex', + personaId: 'test-persona', + model: 'openai-codex/gpt-5.3-codex', + systemPrompt: 'x' + }); + assert.deepEqual(codex.configFiles, []); +}); + test('claude branch appends --plugin-dir per entry in pluginDirs', () => { const result = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-opus-4-6', systemPrompt: 'x', pluginDirs: ['/tmp/session-a/claude/plugin', '/tmp/session-b/claude/plugin'] @@ -141,12 +195,14 @@ test('claude branch appends --plugin-dir per entry in pluginDirs', () => { test('claude branch omits --plugin-dir when pluginDirs is empty or absent', () => { const withEmpty = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-opus-4-6', systemPrompt: 'x', pluginDirs: [] }); const without = buildInteractiveSpec({ harness: 'claude', + personaId: 'test-persona', model: 'claude-opus-4-6', systemPrompt: 'x' }); @@ -157,6 +213,7 @@ test('claude branch omits --plugin-dir when pluginDirs is empty or absent', () = test('non-claude harnesses warn and ignore pluginDirs', () => { const codex = buildInteractiveSpec({ harness: 'codex', + personaId: 'test-persona', model: 'x', systemPrompt: 'x', pluginDirs: ['/tmp/session/plugin'] @@ -166,6 +223,7 @@ test('non-claude harnesses warn and ignore pluginDirs', () => { const opencode = buildInteractiveSpec({ harness: 'opencode', + personaId: 'test-persona', model: 'x', systemPrompt: 'x', pluginDirs: ['/tmp/session/plugin'] @@ -179,6 +237,7 @@ test('warnings are returned, not printed — library consumers route I/O themsel // test would leak output into the test runner. We just assert the shape. const result = buildInteractiveSpec({ harness: 'codex', + personaId: 'test-persona', model: 'x', systemPrompt: 'x', mcpServers: { a: { type: 'http', url: 'https://x' } } diff --git a/packages/harness-kit/src/harness.ts b/packages/harness-kit/src/harness.ts index 4353fc1..c8490ee 100644 --- a/packages/harness-kit/src/harness.ts +++ b/packages/harness-kit/src/harness.ts @@ -4,6 +4,18 @@ import type { PersonaPermissions } from '@agentworkforce/workload-router'; +/** + * A config file the caller should materialize before launching the harness. + * Paths are relative to the harness's cwd — typically the mount dir when a + * sandbox is in use, otherwise process.cwd(). Pure data, no I/O here. + */ +export interface InteractiveConfigFile { + /** Relative path (from cwd) where the file should be written. */ + path: string; + /** Exact file contents, already serialized. */ + contents: string; +} + /** Result of translating a persona's runtime into a spawnable command. */ export interface InteractiveSpec { /** Binary to exec (e.g. `claude`, `codex`, `opencode`). */ @@ -14,8 +26,8 @@ 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 * 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`, + * Currently only codex takes this path; claude uses `--append-system-prompt` + * and opencode writes the prompt into `opencode.json` (see `configFiles`), * so both return `null` here. */ initialPrompt: string | null; @@ -24,10 +36,23 @@ export interface InteractiveSpec { * support MCP yet, ignoring". Callers decide whether to print them. */ warnings: string[]; + /** + * Config files the caller must write (relative to the harness cwd) before + * launch. Opencode uses this to materialize an `opencode.json` carrying + * the persona's agent definition (model + system prompt) so `--agent` can + * resolve it; claude and codex return an empty array. + */ + configFiles: InteractiveConfigFile[]; } export interface BuildInteractiveSpecInput { harness: Harness; + /** + * Persona id — used as the opencode agent name. Claude and codex ignore + * this field today; keeping it required here keeps call sites honest and + * lets future harnesses consume it without another type change. + */ + personaId: string; model: string; systemPrompt: string; /** Env-resolved MCP servers (pass the output of `resolveMcpServersLenient().servers`). */ @@ -77,7 +102,7 @@ function hasAnyPermission(p: PersonaPermissions | undefined): boolean { * harnesses yet. */ export function buildInteractiveSpec(input: BuildInteractiveSpecInput): InteractiveSpec { - const { harness, model, systemPrompt, mcpServers, permissions, pluginDirs } = input; + const { harness, personaId, model, systemPrompt, mcpServers, permissions, pluginDirs } = input; const warnings: string[] = []; const hasPluginDirs = pluginDirs !== undefined && pluginDirs.length > 0; @@ -107,7 +132,7 @@ export function buildInteractiveSpec(input: BuildInteractiveSpecInput): Interact if (permissions?.mode) { args.push('--permission-mode', permissions.mode); } - return { bin: 'claude', args, initialPrompt: null, warnings }; + return { bin: 'claude', args, initialPrompt: null, warnings, configFiles: [] }; } case 'codex': { if (mcpServers && Object.keys(mcpServers).length > 0) { @@ -129,7 +154,8 @@ export function buildInteractiveSpec(input: BuildInteractiveSpecInput): Interact bin: 'codex', args: ['-m', stripProviderPrefix(model)], initialPrompt: systemPrompt, - warnings + warnings, + configFiles: [] }; } case 'opencode': { @@ -148,15 +174,41 @@ 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. + // opencode resolves a persona's system prompt + model through its own + // "agent" abstraction (see https://opencode.ai/config.json: `agent..{ + // model, prompt, mode }`). Earlier revisions tried to use `--prompt` for + // the system prompt and `-m` for the model directly, but that was wrong + // on two counts: + // (a) `--prompt` pre-fills the TUI input buffer with a *user* message, + // not the agent's instructions — the persona's systemPrompt ended + // up sitting unsent in the chat field. + // (b) `-m` expects `provider/model` (opencode's own docs); stripping + // the `opencode/` prefix left just `gpt-5-nano`, which opencode + // could not resolve and silently fell back to its default model. + // The correct shape is an `opencode.json` under cwd defining an agent + // with the persona's prompt + full-provider-form model, selected via + // `--agent ` at launch. We emit that file via configFiles + // so the CLI can drop it into the mount dir before exec. + const agentConfig = { + agent: { + [personaId]: { + model, + prompt: systemPrompt, + mode: 'primary' + } + } + }; return { bin: 'opencode', - args: ['--model', stripProviderPrefix(model), '--prompt', systemPrompt], + args: ['--agent', personaId], initialPrompt: null, - warnings + warnings, + configFiles: [ + { + path: 'opencode.json', + contents: JSON.stringify(agentConfig, null, 2) + '\n' + } + ] }; } } diff --git a/packages/harness-kit/src/index.ts b/packages/harness-kit/src/index.ts index 46e7033..eb87034 100644 --- a/packages/harness-kit/src/index.ts +++ b/packages/harness-kit/src/index.ts @@ -19,6 +19,7 @@ export { export { buildInteractiveSpec, type BuildInteractiveSpecInput, + type InteractiveConfigFile, type InteractiveSpec } from './harness.js'; From 3096bb014b9211f4480b5ff06ce8d14949cd62da Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 22 Apr 2026 19:56:26 -0400 Subject: [PATCH 2/3] Hide configFiles from the mount-mirror in both directions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @relayfile/local-mount mirrors the real repo into the mount and syncs changes back on exit. Without hiding them, the opencode.json written by onBeforeLaunch would (a) get masked on the way in by any pre-existing opencode.json in the user's repo, and (b) sync back out on exit and pollute the user's working tree. Add spec.configFiles[].path to ignoredPatterns dynamically after the initial assignment — keeps the fix generic for any future configFile producer rather than hardcoding opencode.json into the pinned static SKILL_INSTALL_IGNORED_PATTERNS set. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/cli.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index a9ad144..9e78756 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -577,6 +577,16 @@ async function runInteractive( runtime.harness === 'claude' ? [...CLEAN_IGNORED_PATTERNS] : [...SKILL_INSTALL_IGNORED_PATTERNS]; + // Anything we materialize into the mount via onBeforeLaunch must be + // hidden from the mount-mirror in both directions: without this, any + // opencode.json already present in the real repo would be copied into + // the mount (masking the per-session agent config we write), and the + // fresh write from onBeforeLaunch would sync back out on exit and + // pollute the user's working tree. Added dynamically so this stays + // generic for any future configFile producer. + for (const file of spec.configFiles) { + ignoredPatterns.push(file.path); + } 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 From 617fbf798a4c9e485a85a8f6382f8fbcd4029173 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 22 Apr 2026 21:40:11 -0400 Subject: [PATCH 3/3] Address PR #23 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit barryollama (APPROVED with requested changes): - Add exhaustiveness guard to buildInteractiveSpec's switch so future Harness union additions fail compile-time instead of silently falling through at runtime. - Rewrite the stale JSDoc paragraph that still described the old opencode `--prompt` flag behavior; document the agent-abstraction shape and why --prompt / bare -m are deliberately avoided. copilot-pull-request-reviewer inline threads: - stripAgentFlag: JSDoc now accurately describes "removes every --agent pair", matching the implementation. Note why strip-all is the safer idempotent behavior even though the current producer emits one pair. - configFiles materialization: mkdir -p parent directories before writeFileSync so nested relative paths don't throw ENOENT. Gate every write on assertSafeRelativePath, which rejects empty / absolute paths and any segment equal to `..` — prevents a malformed persona from escaping the mount via `join()` and overwriting files outside the sandbox. Tests: new coverage for multi-agent stripping, safe-path acceptance, and safe-path rejection (empty, absolute, traversal). Exhaustiveness branch is covered by TypeScript itself; no runtime test added since reaching the throw requires a cast through `never`. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/cli.test.ts | 34 +++++++++++++++++++++ packages/cli/src/cli.ts | 47 ++++++++++++++++++++++++++--- packages/harness-kit/src/harness.ts | 23 +++++++++++--- 3 files changed, 94 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/cli.test.ts b/packages/cli/src/cli.test.ts index a5b8cf1..d46f66c 100644 --- a/packages/cli/src/cli.test.ts +++ b/packages/cli/src/cli.test.ts @@ -4,6 +4,7 @@ import assert from 'node:assert/strict'; import { CLEAN_IGNORED_PATTERNS, SKILL_INSTALL_IGNORED_PATTERNS, + assertSafeRelativePath, decideCleanMode, parseAgentArgs, resolveSystemPromptPlaceholders, @@ -169,6 +170,39 @@ test('stripAgentFlag: trailing --agent without a value is preserved (caller deci assert.deepEqual(stripAgentFlag(['--agent']), ['--agent']); }); +test('stripAgentFlag: removes every --agent pair, not just the first', () => { + // Current producer emits exactly one pair, so behavior is equivalent + // today, but "strip all" is idempotent and safer if a future caller ever + // appends a second --agent for any reason. + assert.deepEqual( + stripAgentFlag(['--agent', 'a', '--agent', 'b']), + [] + ); + assert.deepEqual( + stripAgentFlag(['--before', '--agent', 'a', '--mid', '--agent', 'b', '--after']), + ['--before', '--mid', '--after'] + ); +}); + +test('assertSafeRelativePath: accepts typical relative paths', () => { + // Sanity: representative safe paths the opencode + future harnesses emit. + assert.doesNotThrow(() => assertSafeRelativePath('opencode.json')); + assert.doesNotThrow(() => assertSafeRelativePath('.opencode/config.json')); + assert.doesNotThrow(() => assertSafeRelativePath('nested/dir/file.json')); +}); + +test('assertSafeRelativePath: rejects empty, absolute, and path-traversal inputs', () => { + // Guards materialization against a malformed or adversarial persona trying + // to escape the mount via `join(dir, path)` and overwrite files outside + // the sandbox. Failure must surface with a clear message BEFORE any + // writeFileSync runs. + assert.throws(() => assertSafeRelativePath(''), /non-empty/); + assert.throws(() => assertSafeRelativePath('/etc/passwd'), /absolute/); + assert.throws(() => assertSafeRelativePath('../escape.json'), /\.\./); + assert.throws(() => assertSafeRelativePath('ok/../escape.json'), /\.\./); + assert.throws(() => assertSafeRelativePath('a/../../b.json'), /\.\./); +}); + test('decideCleanMode: claude + clean → engaged', () => { assert.deepEqual(decideCleanMode('claude', true), { useClean: true }); }); diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 9e78756..e3bd498 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,9 +1,9 @@ #!/usr/bin/env node import { spawn, spawnSync } from 'node:child_process'; import { randomBytes } from 'node:crypto'; -import { rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { constants, homedir } from 'node:os'; -import { join, resolve as resolvePath } from 'node:path'; +import { dirname, isAbsolute, join, resolve as resolvePath } from 'node:path'; import { pathToFileURL } from 'node:url'; import { @@ -346,10 +346,17 @@ function sessionMountDir(sessionRoot: string): string { } /** - * Remove a single `--agent ` pair from a harness argv. Used on the - * non-mount opencode path where we cannot safely materialize the persona's + * Remove every `--agent ` pair from a harness argv. Used on the non-mount + * opencode path where we cannot safely materialize the persona's * opencode.json (it would land in the user's real repo), so we fall back to * launching opencode without a persona-specific agent selection. + * + * Strips all occurrences rather than just the first — the current producer + * (harness-kit's opencode branch) emits exactly one pair, so both behaviors + * are equivalent today, but "remove all" is idempotent and safer if a future + * caller ever appends a second `--agent` for any reason. A trailing `--agent` + * with no following value is preserved so the malformed argv surfaces at the + * harness rather than getting silently swallowed here. */ export function stripAgentFlag(args: readonly string[]): string[] { const out: string[] = []; @@ -363,6 +370,30 @@ export function stripAgentFlag(args: readonly string[]): string[] { return out; } +/** + * Validate that a configFile's relative path is safe to resolve under a + * sandbox/session directory. Rejects absolute paths and any segment equal to + * `..` so a malformed or adversarial persona cannot escape the mount via + * `join()` and overwrite files elsewhere. Called at materialization time so + * the failure surfaces with a clear path before any disk write happens. + */ +export function assertSafeRelativePath(relPath: string): void { + if (!relPath) { + throw new Error('configFile path must be a non-empty relative path'); + } + if (isAbsolute(relPath)) { + throw new Error( + `configFile path must be relative; got absolute path ${JSON.stringify(relPath)}` + ); + } + const segments = relPath.split(/[\\/]+/); + if (segments.some((s) => s === '..')) { + throw new Error( + `configFile path must not contain ".." segments; got ${JSON.stringify(relPath)}` + ); + } +} + /** Patterns hidden from an interactive claude session when `--clean` is set. * Applied by `@relayfile/local-mount` with gitignore semantics, so bare names * match at any depth in the project tree (e.g. `.claude` hides both @@ -662,7 +693,13 @@ async function runInteractive( runInstallOrThrow(install.command, installLabel, dir); } for (const file of spec.configFiles) { - writeFileSync(join(dir, file.path), file.contents, 'utf8'); + assertSafeRelativePath(file.path); + const target = join(dir, file.path); + // mkdir -p for any subdirs in file.path — the + // InteractiveConfigFile contract allows nested relative + // paths, and writeFileSync would otherwise throw ENOENT. + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, file.contents, 'utf8'); } } } diff --git a/packages/harness-kit/src/harness.ts b/packages/harness-kit/src/harness.ts index c8490ee..1dda75c 100644 --- a/packages/harness-kit/src/harness.ts +++ b/packages/harness-kit/src/harness.ts @@ -91,11 +91,16 @@ function hasAnyPermission(p: PersonaPermissions | undefined): boolean { * 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 …". + * The opencode branch routes model + system prompt through opencode's + * agent abstraction (see https://opencode.ai/config.json: `agent..{ + * model, prompt, mode }`). It emits an `opencode.json` via `configFiles` + * for the caller to materialize at cwd, and selects it with `--agent + * `. It deliberately does NOT use `--prompt` (that flag + * pre-fills the TUI input with a user message) or bare `-m` (opencode + * expects full `provider/model` form and silently falls back to its + * default when given a stripped model name). `initialPrompt` is `null` + * so callers do not append a trailing positional, which opencode would + * otherwise interpret as a project directory. * * Both codex and opencode emit a warning if the persona declares * `mcpServers` or `permissions` — those features aren't wired for those @@ -211,5 +216,13 @@ export function buildInteractiveSpec(input: BuildInteractiveSpecInput): Interact ] }; } + default: { + // Exhaustiveness guard: if `Harness` gains a new variant, this + // assertion will fail to compile and force the maintainer to handle + // the new case above rather than silently dropping into an untyped + // fallthrough at runtime. + const _exhaustive: never = harness; + throw new Error(`Unhandled harness: ${String(_exhaustive)}`); + } } }