diff --git a/server/lib/opencodeConfig.js b/server/lib/opencodeConfig.js index 2c068c9333..b5be495257 100644 --- a/server/lib/opencodeConfig.js +++ b/server/lib/opencodeConfig.js @@ -38,6 +38,7 @@ import { isOpencodeCommand, prefixOpencodeModel, parseOpencodeConfigContent, + OPENCODE_BUILD_AGENT, OPENCODE_PUBLIC_REVIEW_AGENT, } from './providerModels.js'; import { PROVIDER_GATEWAYS, PROVIDER_GATEWAY_IDS, gatewayById, isGatewayNamespace } from './providerGateways.js'; @@ -408,14 +409,14 @@ function hardenOpencodeConfigForNoTool(config) { config.permission = DENY_ALL_PERMISSIONS; config.tools = { ...DENY_ALL_TOOLS }; const agents = asObject(config.agent); - const agentNames = new Set([...Object.keys(agents), 'build', OPENCODE_PUBLIC_REVIEW_AGENT]); + const agentNames = new Set([...Object.keys(agents), OPENCODE_BUILD_AGENT, OPENCODE_PUBLIC_REVIEW_AGENT]); // `buildAgentGeneration` writes the stage's temperature / topP / thinking / // reasoningEffort onto `agent.build` — OpenCode's default agent — but this // profile runs `--agent plan`. Seed the review agent from `build` so the // stage's configured effort actually reaches the model that runs, instead of // silently falling back to the backend default. An explicit `agent.plan` in // the user's own config still wins (it is spread after). - const generationSource = asObject(agents.build); + const generationSource = asObject(agents[OPENCODE_BUILD_AGENT]); config.agent = Object.fromEntries([...agentNames].map((name) => [name, { ...(name === OPENCODE_PUBLIC_REVIEW_AGENT ? generationSource : {}), ...asObject(agents[name]), diff --git a/server/lib/providerModels.js b/server/lib/providerModels.js index 59ef839a01..eea099dc27 100644 --- a/server/lib/providerModels.js +++ b/server/lib/providerModels.js @@ -618,6 +618,13 @@ export function isOpencodeCommand(command) { */ export const OPENCODE_PUBLIC_REVIEW_AGENT = 'plan'; +/** + * OpenCode's built-in tool-enabled agent — the one an attachable Stage 3 + * session runs as (`providerVendors.js`) and the one `hardenOpencodeConfigForNoTool` + * must still empty. Homed here for the same import-graph reason as above. + */ +export const OPENCODE_BUILD_AGENT = 'build'; + /** * OpenCode addresses models as `provider/model` (e.g. `ollama/qwen2.5:7b`). The * OpenCode Ollama provider declares its local daemon under the config-provider diff --git a/server/lib/providerVendors.js b/server/lib/providerVendors.js index ce93d1a46d..f614064eea 100644 --- a/server/lib/providerVendors.js +++ b/server/lib/providerVendors.js @@ -74,6 +74,7 @@ import { localRuntimeNamespace, opencodeProviderIsLocalOnly, OPENCODE_PUBLIC_REVIEW_AGENT, + OPENCODE_BUILD_AGENT, applyLeanClaudeArgs, } from './providerModels.js'; import { @@ -147,7 +148,7 @@ function defaultSpawnArgs(cliArgsFn, fallbackCommand) { * a vendor (`codex-tui`, `grok-tui`, …) is spawned through that vendor's * headless public-review recipe exactly like its CLI sibling, so the user's * enabled TUI providers are legal stage choices. The one exception is a recipe - * marked `tui: true` (see `supportsTuiPublicReviewPosture`), which the + * supplying `tuiSpawnArgs` (see `supportsTuiPublicReviewPosture`), which the * sandboxed-actions stage may run as an attachable session so an operator can * watch and steer it. API/custom providers have no binary and no recipe. */ @@ -332,15 +333,20 @@ const ANTIGRAVITY = { // ─── opencode ─────────────────────────────────────────────────────────────── -function opencodeCliArgs(baseArgs, { model, provider }) { - const args = baseArgs.includes('run') ? [...baseArgs] : ['run', ...baseArgs]; +/** Append the namespaced `-m` unless the argv already pins a model. */ +function appendOpencodeModel(args, provider, model) { const resolvedModel = prefixOpencodeModel(provider, model); - if (resolvedModel && !hasModelFlag(baseArgs)) { - args.push('-m', resolvedModel); - } + if (resolvedModel && !hasModelFlag(args)) args.push('-m', resolvedModel); return args; } +const opencodeSpawnConfig = (provider, args) => ({ command: provider?.command || 'opencode', args, stdinMode: 'prompt' }); + +function opencodeCliArgs(baseArgs, { model, provider }) { + const args = baseArgs.includes('run') ? [...baseArgs] : ['run', ...baseArgs]; + return appendOpencodeModel(args, provider, model); +} + /** * An OpenCode wrapper this install can actually run the tool-free gate on. * Three conditions, each closing a different way the stage would otherwise be @@ -363,8 +369,9 @@ function opencodeCliArgs(baseArgs, { model, provider }) { * unrepresentable. * - **a spawnable binary**, as for every other vendor. */ -const isLocalOpencodeProvider = (provider) => isDirectBinaryProvider(provider) - && isOpencodeCommand(provider?.command) +const matchOpencodeBinary = (provider) => isDirectBinaryProvider(provider) && isOpencodeCommand(provider?.command); + +const isLocalOpencodeProvider = (provider) => matchOpencodeBinary(provider) && localRuntimeNamespace(provider) === 'ollama' && opencodeProviderIsLocalOnly(provider); @@ -385,11 +392,32 @@ const isLocalOpencodeProvider = (provider) => isDirectBinaryProvider(provider) * onto this agent (see `hardenOpencodeConfigForNoTool`). */ function opencodePublicReviewSpawnArgs(provider, { effectiveModel } = {}) { - return { - command: provider?.command || 'opencode', - args: opencodeCliArgs(['--agent', OPENCODE_PUBLIC_REVIEW_AGENT], { model: effectiveModel, provider }), - stdinMode: 'prompt', - }; + return opencodeSpawnConfig(provider, opencodeCliArgs(['--agent', OPENCODE_PUBLIC_REVIEW_AGENT], { model: effectiveModel, provider })); +} + +/** + * The ATTACHABLE `sandboxed-actions` invocation (#6238). OpenCode's headless + * argv is a one-shot `opencode run …`, which cannot become an interactive + * session by dropping flags the way Claude's recipe does — in a PTY it neither + * accepts a pasted prompt nor renders. Its real interactive entry point is the + * BARE binary, which takes the same `--agent`/`-m` flags and reads the prompt + * the spawner pastes into every TUI (`agentTuiSpawning.js`). The tool-enabled + * agent is pinned on the argv and provider args are not forwarded (same rule as + * `buildTuiSpawnConfig`): a saved `--agent plan` would hand the session to a + * human nobody has to be. Permissions are the config's, on both paths — every + * tool allowed, the interactive gates denied (`buildOpencodeEnvVars`) — for as + * long as `OPENCODE_CONFIG_CONTENT` survives the actions env allowlist, i.e. a + * local-only endpoint (`cliChildEnv.js`); a gateway-backed wrapper runs on the + * operator's own `~/.config/opencode` instead, exactly as its headless run + * already did. + * + * The row supplies ONLY this builder: a headless actions run keeps falling + * through to the ordinary `run` argv, and with no headless `spawnArgs` the row + * is not an enforcement (OpenCode ships no sandbox; the disposable worktree is + * the isolation), so the schedule UI keeps reporting it as worktree-only. + */ +function opencodePublicReviewActionsTuiSpawnArgs(provider, { effectiveModel } = {}) { + return opencodeSpawnConfig(provider, appendOpencodeModel(['--agent', OPENCODE_BUILD_AGENT], provider, effectiveModel)); } const OPENCODE = { @@ -407,9 +435,12 @@ const OPENCODE = { spawnArgs: opencodePublicReviewSpawnArgs, matchProvider: isLocalOpencodeProvider, }, - // No `sandboxed-actions` recipe: OpenCode ships no OS sandbox of its own, - // so it stays in the open-to-every-binary tier where the disposable - // worktree is the isolation — see `supportsPublicReviewPosture`. + [PUBLIC_REVIEW_ACTIONS_POSTURE]: { + // Attachable on every backend (MTPLX and gateways included): unlike the + // no-tool gate there is no model-capability probe involved. + tuiSpawnArgs: opencodePublicReviewActionsTuiSpawnArgs, + matchProvider: matchOpencodeBinary, + }, }, }; @@ -592,7 +623,7 @@ const CLAUDE_PUBLIC_REVIEW_ACTIONS_ARGS = [ // Flags Claude Code accepts ONLY alongside `--print`, mapped to whether they // consume the following argv entry as their value. The posture arrays above are -// written for the headless launch, so an attachable (`tui: true`) recipe has to +// written for the headless launch, so the attachable `tuiSpawnArgs` recipe has to // drop them — the CLI refuses to start at all otherwise: // // Error: --no-session-persistence can only be used with --print mode. @@ -676,7 +707,8 @@ const CLAUDE = { [PUBLIC_REVIEW_ACTIONS_POSTURE]: { spawnArgs: claudePublicReviewSpawnArgsFor(CLAUDE_PUBLIC_REVIEW_ACTIONS_ARGS), matchProvider: matchClaudeBinary, - // The only posture/vendor pairing that may run as an ATTACHABLE session. + // One of two posture/vendor pairings that may run as an ATTACHABLE session + // (OpenCode is the other — see its row). // `claudePublicReviewArgs` drops only the flags that REQUIRE `--print` // (the headless output set plus CLAUDE_PRINT_ONLY_ARGS) for // `tui: true`; every enforcement flag above (`--permission-mode @@ -687,7 +719,7 @@ const CLAUDE = { // lever that lifts Claude Code's filesystem protection is // `sandbox.filesystem.disabled`, which this recipe never emits, and // `--disable-slash-commands` removes the in-session settings surface. - tui: true, + tuiSpawnArgs: claudePublicReviewSpawnArgsFor(CLAUDE_PUBLIC_REVIEW_ACTIONS_ARGS), }, }, }; @@ -731,7 +763,7 @@ export function publicReviewRecipe(provider, posture) { if (!PUBLIC_REVIEW_POSTURES.includes(posture)) return null; for (const vendor of PROVIDER_VENDORS) { const recipe = vendor.publicReview?.[posture]; - if (recipe?.spawnArgs && recipe.matchProvider(provider)) return recipe; + if ((recipe?.spawnArgs || recipe?.tuiSpawnArgs) && recipe.matchProvider(provider)) return recipe; } return null; } @@ -808,12 +840,12 @@ export function buildVendorSpawnConfig(provider, ctx) { // session whose posture is decorative. Callers decide TUI-vs-headless from // `supportsTuiPublicReviewPosture`, so reaching this is a routing bug. if (ctx?.tui) { - if (!recipe?.tui) { + if (!recipe?.tuiSpawnArgs) { throw new Error(`Provider '${providerLabel(provider)}' has no attachable ${posture} public-review recipe`); } - return recipe.spawnArgs(provider, ctx); + return recipe.tuiSpawnArgs(provider, ctx); } - if (recipe) return recipe.spawnArgs(provider, ctx); + if (recipe?.spawnArgs) return recipe.spawnArgs(provider, ctx); // See supportsPublicReviewPosture for why the actions stage may fall // through to the vendor's ordinary headless recipe and the gate may not. if (!supportsPublicReviewPosture(provider, posture)) { @@ -833,7 +865,7 @@ export function buildVendorSpawnConfig(provider, ctx) { * API/custom providers have no maintained recipe: a generic read-only prompt * is not enforcement, so they fail closed. A TUI record IS eligible — the * stage spawns its binary through the vendor's enforced recipe, headless unless - * that recipe is also marked `tui: true` (see `isDirectBinaryProvider` and + * that recipe also supplies `tuiSpawnArgs` (see `isDirectBinaryProvider` and * `supportsTuiPublicReviewPosture`). */ export function publicReviewPosturesForProvider(provider) { @@ -849,8 +881,12 @@ export function enforcedPublicReviewPosturesForProvider(provider) { return PUBLIC_REVIEW_POSTURES.filter((posture) => enforcesPublicReviewPosture(provider, posture)); } +// A row that supplies the HEADLESS argv is an enforcement; one that supplies +// only an attachable invocation (OpenCode's actions row) is not — the stage +// still falls through to the vendor's ordinary argv and the schedule UI keeps +// reporting that choice as worktree-only rather than OS-sandboxed. const enforcesPublicReviewPosture = (provider, posture) => ( - isDirectBinaryProvider(provider) && Boolean(publicReviewRecipe(provider, posture)) + isDirectBinaryProvider(provider) && Boolean(publicReviewRecipe(provider, posture)?.spawnArgs) ); /** @@ -919,17 +955,17 @@ export function supportsPublicReviewActionsProvider(provider) { * Deliberately much narrower than `supportsPublicReviewPosture`: that one lets * the actions stage fall through to a vendor's ordinary headless recipe when it * declares none, which is fine for a `--print` child and useless in a PTY. An - * interactive session requires a recipe that has been reviewed for it and says - * so with `tui: true` — the recipe still owns the argv (`spawnArgs(provider, - * { ...ctx, tui: true })`), it just drops the flags that only work under - * `--print`. + * interactive session requires a recipe that has been reviewed for it and + * supplies `tuiSpawnArgs` — the argv a PTY can drive (for Claude the headless + * argv minus the flags that only work under `--print`; for OpenCode a + * different entry point entirely). * * `no-tool` is structurally excluded: an interactive session for a reasoner * with no tools buys nothing and widens the boundary for free, so no row * declares it and this returns false for that posture by construction. */ export function supportsTuiPublicReviewPosture(provider, posture) { - return isDirectBinaryProvider(provider) && Boolean(publicReviewRecipe(provider, posture)?.tui); + return isDirectBinaryProvider(provider) && Boolean(publicReviewRecipe(provider, posture)?.tuiSpawnArgs); } /** Whether the sandboxed final public-review stage can attach a PTY here. */ diff --git a/server/lib/providerVendors.publicReview.test.js b/server/lib/providerVendors.publicReview.test.js index 5e95eaa4ba..94c7389b3b 100644 --- a/server/lib/providerVendors.publicReview.test.js +++ b/server/lib/providerVendors.publicReview.test.js @@ -366,9 +366,52 @@ describe('public-review provider postures', () => { expect(supportsTuiPublicReviewActionsProvider({ id: 'codex-tui', type: 'tui', command: 'codex' })).toBe(false); expect(supportsTuiPublicReviewActionsProvider({ id: 'grok-tui', type: 'tui', command: 'grok' })).toBe(false); expect(supportsTuiPublicReviewActionsProvider(antigravity)).toBe(false); - // …as do vendors with no actions recipe at all, and non-binary records. - expect(supportsTuiPublicReviewActionsProvider({ id: 'opencode-tui', type: 'tui', command: 'opencode' })).toBe(false); + // …as do non-binary records, whatever their vendor. expect(supportsTuiPublicReviewActionsProvider({ id: 'claude-api', type: 'api', command: 'claude' })).toBe(false); + expect(supportsTuiPublicReviewActionsProvider({ id: 'opencode-api', type: 'api', command: 'opencode' })).toBe(false); + // #6238 — OpenCode is attachable on EVERY backend: unlike the no-tool gate + // (Ollama-only — see the `mtplxBacked` cases above) there is no + // model-capability probe involved, so an MTPLX or gateway wrapper qualifies. + expect(supportsTuiPublicReviewActionsProvider({ id: 'opencode-tui', type: 'tui', command: 'opencode', mtplxBacked: true })).toBe(true); + expect(supportsTuiPublicReviewActionsProvider({ id: 'opencode-cli', type: 'cli', command: 'opencode' })).toBe(true); + expect(supportsTuiPublicReviewPosture({ id: 'opencode-tui', type: 'tui', command: 'opencode', ollamaBacked: true }, PUBLIC_REVIEW_NO_TOOL_POSTURE)).toBe(false); + }); + + // #6238 — OpenCode's headless actions run is a one-shot `opencode run …`, + // which cannot become an interactive session by dropping flags; the + // attachable recipe is the BARE binary (OpenCode's TUI entry point) with the + // same agent/model flags, and the spawner pastes the prompt as for any TUI. + it('builds the attachable OpenCode actions argv as the bare binary while the headless argv is unchanged', () => { + const opencodeTui = { id: 'opencode-tui', type: 'tui', command: 'opencode', args: ['--agent', 'plan', '--auto'], ollamaBacked: true }; + const headless = buildVendorSpawnConfig(opencodeTui, { + effectiveModel: 'qwen3-coder:30b', + safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE, + }); + // The headless shape is UNCHANGED by the row: still the ordinary + // `run`-prefixed argv with the provider's own args forwarded. + expect(headless).toEqual({ + command: 'opencode', + args: ['run', '--agent', 'plan', '--auto', '-m', 'ollama/qwen3-coder:30b'], + stdinMode: 'prompt', + }); + + const tui = buildVendorSpawnConfig(opencodeTui, { + effectiveModel: 'qwen3-coder:30b', + safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE, + tui: true, + }); + expect(tui).toEqual({ + command: 'opencode', + // No `run` subcommand: that is print mode and never renders in a PTY. The + // tool-enabled agent is pinned on the argv, and the provider's saved args + // (`--agent plan`, `--auto`) are NOT forwarded on the attachable path. + args: ['--agent', 'build', '-m', 'ollama/qwen3-coder:30b'], + stdinMode: 'prompt', + }); + expect(buildVendorSpawnConfig(opencodeTui, { + safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE, + tui: true, + }).args).toEqual(['--agent', 'build']); }); it('refuses to build an attachable argv for a vendor with no attachable recipe', () => { @@ -378,7 +421,6 @@ describe('public-review provider postures', () => { for (const provider of [ { id: 'codex-tui', type: 'tui', command: 'codex' }, { id: 'grok-tui', type: 'tui', command: 'grok' }, - { id: 'opencode-tui', type: 'tui', command: 'opencode' }, { id: 'antigravity-tui', type: 'tui', command: 'agy' }, ]) { expect(() => buildVendorSpawnConfig(provider, { diff --git a/server/services/agentErrorAnalysis.js b/server/services/agentErrorAnalysis.js index a8954bb9c8..dc1159a464 100644 --- a/server/services/agentErrorAnalysis.js +++ b/server/services/agentErrorAnalysis.js @@ -221,7 +221,7 @@ export const ERROR_PATTERNS = [ const requires = redactFailureSnippet(match[2] || '').replace(/[.,;]+$/, '').slice(0, CONFIG_EXPECTED_MAX_CHARS); return { message: `Provider CLI rejected the flag ${flag}`, - suggestedFix: `The CLI exited while parsing its arguments, before the prompt was delivered, so every retry fails identically.${requires ? ` It reports that \`${flag}\` only works with ${requires}.` : ''} PortOS builds this argv itself — fix the posture/vendor recipe in server/lib/providerVendors.js (an attachable \`tui: true\` recipe must drop every flag that requires \`--print\`), not the provider record.`, + suggestedFix: `The CLI exited while parsing its arguments, before the prompt was delivered, so every retry fails identically.${requires ? ` It reports that \`${flag}\` only works with ${requires}.` : ''} PortOS builds this argv itself — fix the posture/vendor recipe in server/lib/providerVendors.js (an attachable \`tuiSpawnArgs\` recipe must drop every flag that requires \`--print\`), not the provider record.`, rejectedCliFlag: flag }; } diff --git a/server/services/agentLifecycle.js b/server/services/agentLifecycle.js index 789415fd61..097947bb12 100644 --- a/server/services/agentLifecycle.js +++ b/server/services/agentLifecycle.js @@ -444,7 +444,7 @@ async function runAgentSpawn(task) { // patch and runs the repo's tests — so it is the one an operator actually // wants to attach to and steer. It may run as an interactive session when // its configured provider is a TUI record AND that vendor declares an - // attachable recipe (`tui: true`), which keeps every enforcement flag and + // attachable recipe (`tuiSpawnArgs`), which keeps every enforcement flag and // drops only the headless output flags. // // Everything else stays headless. The `no-tool` postures (Stage 1's diff --git a/server/services/agentLifecycle.postureGate.test.js b/server/services/agentLifecycle.postureGate.test.js index 136e7b4b7a..93da61c806 100644 --- a/server/services/agentLifecycle.postureGate.test.js +++ b/server/services/agentLifecycle.postureGate.test.js @@ -345,6 +345,39 @@ describe('public-review posture gate — spawn behavior (#5866)', () => { ); }); + // #6238 — OpenCode's actions row exists for exactly this: a Stage 3 run on an + // OpenCode-backed TUI record gets a PTY (and so an attachable Shell session) + // instead of being forced headless. Its no-tool gate stays blocked above. + it('spawns the sandboxed-actions stage as a PTY on an OpenCode TUI provider', async () => { + const { buildTuiSpawnConfig, spawnTuiAgent } = await import('./agentTuiSpawning.js'); + vi.mocked(resolveAgentProviderAndModel).mockResolvedValue({ + ok: true, provider: { ...OPENCODE_TUI, mtplxBacked: true }, selectedModel: 'example-model', modelSelection: {}, + }); + vi.mocked(buildTuiSpawnConfig).mockReturnValue({ command: 'opencode', args: [], commandLine: 'opencode' }); + reachDispatch(); + + await spawnAgentForTask({ + id: 'task-public-review-actions-opencode', + metadata: { + executionProfile: 'public-review-actions', + issueWatcher: { pullRequests: [{ number: 42 }] }, + pipeline: { + securityScan: { completed: true, status: 'passed', safePrCount: 1 }, + eligibility: { complete: true, eligibleNumbers: [42] }, + }, + }, + }); + + expect(postureBlockWrites()).toEqual([]); + expect(spawnDirectly).not.toHaveBeenCalled(); + expect(spawnTuiAgent).toHaveBeenCalledTimes(1); + expect(buildTuiSpawnConfig).toHaveBeenCalledWith( + expect.objectContaining({ id: 'opencode-tui' }), + 'example-model', + expect.objectContaining({ safetyProfile: 'public-review-actions' }), + ); + }); + // The narrow half of the same rule. A vendor whose actions recipe has not been // reviewed for a PTY emits headless argv (`exec`, `--print`, `run`) that a PTY // can neither prompt nor enforce, so it stays headless rather than opening a