From 4660562fdee6eb6d1dc4649abe338c7222e97da6 Mon Sep 17 00:00:00 2001 From: Philip D'Souza Date: Thu, 13 Aug 2026 00:44:03 +0100 Subject: [PATCH] fix(opencode-local): make the model-availability probe non-fatal (#10294) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents run through adapters; the `opencode-local` adapter shells out to the OpenCode CLI and, before each run, does a pre-flight `opencode models` **availability probe** to fail fast on a misconfigured `provider/model`. > - That probe was written to **throw on any probe failure** — a timeout, a non-zero exit, or a transient `Unexpected error` from the CLI — which aborts the whole heartbeat run. > - In practice the CLI probe fails transiently (provider hiccup, cold cache, momentary CLI error). When that happens *after* the agent has already done its work, the run dies before its terminal disposition is written, so the platform reopens the issue and re-runs it — a spurious crash/re-run loop that affects every agent on the OpenCode adapter. > - This PR makes the probe **non-fatal when it cannot run**: it warns and proceeds with the configured model, letting the real invocation be authoritative. > - It deliberately **keeps** the genuine guard: when the probe *succeeds* and the configured model is absent from a non-empty list, it still throws (this is what catches misconfigured slugs). > - The benefit is that a best-effort pre-flight check can no longer take down an otherwise-healthy run, while the useful misconfiguration guard is retained. ## Linked Issues or Issue Description No public GitHub issue exists; describing inline (bug). **What happened:** an OpenCode-adapter agent run terminated at the adapter level with `` `opencode models` failed: Unexpected error ``. The failure landed after the agent had produced its work, so the terminal-status update never applied and the run was reopened and re-executed. **Expected:** a transient failure of the `opencode models` availability *probe* should not abort the run — the probe is a best-effort pre-flight guard, not a gate. **Actual:** the probe threw on timeout / non-zero exit / empty output, aborting the run and discarding the completed work + disposition. **Scope:** both the local (`models.ts`) and remote/SSH (`execute.ts`) probe paths; affects any agent on the `opencode_local` adapter. Related PRs (context / prior art): - Refs #5119 — added the remote execution-target model-probe validation this PR softens. - Refs #3291 — closed prior attempt to make the `opencode_local` model probe non-blocking (at agent-create time; different entry point). - Refs #8014 — related open work raising the probe timeout (20s → 60s); complementary, not overlapping. ## What Changed - `models.ts` (`ensureOpenCodeModelConfiguredAndAvailable`): if discovery throws (probe can't run) or returns an empty list, **warn and proceed** with the configured model instead of throwing. The "model present in a non-empty list" check is unchanged and still throws when the configured model is genuinely absent. - `execute.ts` (`ensureRemoteOpenCodeModelConfiguredAndAvailable`): remote probe **timeout / non-zero exit / empty output** now warn and return (proceed) instead of throwing. The remote model-absent guard still throws. - `models.test.ts`: the local "discovery cannot run" case now asserts the probe **proceeds** with the configured model (was: asserts it rejects). - `execute.test.ts`: added remote regression tests — non-zero exit, timeout, and empty output all proceed; a successful probe missing the configured model still rejects. ## Verification ```bash pnpm --filter @paperclipai/adapter-opencode-local typecheck # clean # opencode-local server suite (default 5s per-test timeout is too tight for the # heavy SSH tests on some machines; use a realistic timeout): node node_modules/.pnpm/vitest@*/node_modules/vitest/vitest.mjs run \ packages/adapters/opencode-local/src/server/models.test.ts \ packages/adapters/opencode-local/src/server/execute.test.ts \ packages/adapters/opencode-local/src/server/execute.remote.test.ts \ --testTimeout=45000 ``` Result: typecheck clean; all opencode-local server tests pass, including the new remote fail-open tests and the retained "model unavailable on the remote target" guard test. ## Risks - **Fail-open behavior (intentional).** When the probe can't run, a genuinely misconfigured model is no longer caught at pre-flight — it surfaces at the real invocation instead. This is the accepted tradeoff: the probe is best-effort, and the real invocation is authoritative. The high-value guard (probe succeeds + model absent from a non-empty list) is retained, so the common misconfiguration — a bad `provider/model` slug — is still caught. - No API, schema, or migration changes. Behavior change is confined to the two probe helpers. Low risk overall. ## Model Used Anthropic **Claude Opus 4.8** (`claude-opus-4-8`), used via Claude Code with agentic tool use (repo search, file editing, shell/code execution) and extended reasoning. Used to diagnose the crash, implement the fix, and write the tests; the change was reviewed before submission. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change (`fix/opencode-model-probe-non-fatal`) and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes — N/A (internal adapter behavior; no user-facing docs affected) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green (functional gates: tests/build/e2e/typecheck/security). Review/Greptile gate re-running after this update. - [ ] Greptile is 5/5 with no open P2s — re-triggered after addressing both P2s (remote test coverage + this template-complete description) - [x] I will address all Greptile and reviewer comments before requesting merge --- .../opencode-local/src/server/execute.test.ts | 68 ++++++++++++++++++- .../opencode-local/src/server/execute.ts | 24 +++++-- .../opencode-local/src/server/models.test.ts | 4 +- .../opencode-local/src/server/models.ts | 33 +++++++-- 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/packages/adapters/opencode-local/src/server/execute.test.ts b/packages/adapters/opencode-local/src/server/execute.test.ts index 3b7bcadd23..136cd6ca8a 100644 --- a/packages/adapters/opencode-local/src/server/execute.test.ts +++ b/packages/adapters/opencode-local/src/server/execute.test.ts @@ -1,6 +1,27 @@ -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@paperclipai/adapter-utils/execution-target", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { ...actual, runAdapterExecutionTargetProcess: vi.fn() }; +}); import { ensureRemoteOpenCodeModelConfiguredAndAvailable } from "./execute.js"; +import { runAdapterExecutionTargetProcess } from "@paperclipai/adapter-utils/execution-target"; + +const runProcessMock = vi.mocked(runAdapterExecutionTargetProcess); + +function probeResult(overrides: Record) { + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + pid: 123, + startedAt: new Date().toISOString(), + ...overrides, + } as never; +} describe("ensureRemoteOpenCodeModelConfiguredAndAvailable", () => { afterEach(() => { @@ -60,3 +81,48 @@ describe("ensureRemoteOpenCodeModelConfiguredAndAvailable", () => { ).rejects.toThrow(); }); }); + +describe("ensureRemoteOpenCodeModelConfiguredAndAvailable — probe is non-fatal when it cannot run", () => { + const target = { kind: "remote", transport: "ssh" } as never; + const base = { + runId: "run-probe", + executionTarget: target, + command: "opencode", + cwd: "/tmp", + env: {} as Record, + timeoutSec: 30, + graceSec: 5, + }; + + beforeEach(() => { + runProcessMock.mockReset(); + }); + + it("proceeds when the remote probe exits non-zero (e.g. a transient `Unexpected error`)", async () => { + runProcessMock.mockResolvedValueOnce(probeResult({ exitCode: 1, stderr: "Unexpected error" })); + await expect( + ensureRemoteOpenCodeModelConfiguredAndAvailable({ ...base, model: "openai/gpt-5" }), + ).resolves.toBeUndefined(); + }); + + it("proceeds when the remote probe times out", async () => { + runProcessMock.mockResolvedValueOnce(probeResult({ timedOut: true, exitCode: null })); + await expect( + ensureRemoteOpenCodeModelConfiguredAndAvailable({ ...base, model: "openai/gpt-5" }), + ).resolves.toBeUndefined(); + }); + + it("proceeds when the remote probe returns no models", async () => { + runProcessMock.mockResolvedValueOnce(probeResult({ exitCode: 0, stdout: "" })); + await expect( + ensureRemoteOpenCodeModelConfiguredAndAvailable({ ...base, model: "openai/gpt-5" }), + ).resolves.toBeUndefined(); + }); + + it("still rejects when the probe succeeds but the configured model is absent (guard retained)", async () => { + runProcessMock.mockResolvedValueOnce(probeResult({ exitCode: 0, stdout: "openai/gpt-4.1\n" })); + await expect( + ensureRemoteOpenCodeModelConfiguredAndAvailable({ ...base, model: "openai/gpt-5" }), + ).rejects.toThrow("Configured OpenCode model is unavailable on the remote execution target"); + }); +}); diff --git a/packages/adapters/opencode-local/src/server/execute.ts b/packages/adapters/opencode-local/src/server/execute.ts index 91f953f2c3..2f17e3dbd6 100644 --- a/packages/adapters/opencode-local/src/server/execute.ts +++ b/packages/adapters/opencode-local/src/server/execute.ts @@ -124,24 +124,34 @@ export async function ensureRemoteOpenCodeModelConfiguredAndAvailable(input: { }, ); + // The remote availability probe is a best-effort pre-flight guard, not a gate. + // If `opencode models` itself cannot run on the target — timeout, transient CLI + // error, provider hiccup — do NOT abort the run. The real invocation is + // authoritative, so a probe that can't execute must never be fatal. (Previously + // these threw and crashed runs mid-flight, losing the agent's work + disposition.) if (probe.timedOut) { - throw new Error(`\`opencode models\` timed out on the remote execution target after ${probeTimeoutSec}s.`); + console.warn( + `[opencode-local] Remote model availability probe for "${model}" timed out after ${probeTimeoutSec}s; proceeding with the configured model.`, + ); + return; } if ((probe.exitCode ?? 1) !== 0) { const detail = firstNonEmptyLine(probe.stderr) || firstNonEmptyLine(probe.stdout); - throw new Error( - detail - ? `\`opencode models\` failed on the remote execution target: ${detail}` - : "`opencode models` failed on the remote execution target.", + console.warn( + `[opencode-local] Remote \`opencode models\` could not run for "${model}"${ + detail ? ` (${detail})` : "" + }; proceeding with the configured model.`, ); + return; } const models = parseOpenCodeModelsOutput(probe.stdout); if (models.length === 0) { - throw new Error( - "OpenCode returned no models on the remote execution target. Run `opencode models` there and verify provider auth.", + console.warn( + `[opencode-local] Remote \`opencode models\` returned no models; proceeding with the configured model "${model}".`, ); + return; } if (!models.some((entry) => entry.id === model)) { diff --git a/packages/adapters/opencode-local/src/server/models.test.ts b/packages/adapters/opencode-local/src/server/models.test.ts index 4a3f775c6b..71748e3792 100644 --- a/packages/adapters/opencode-local/src/server/models.test.ts +++ b/packages/adapters/opencode-local/src/server/models.test.ts @@ -37,13 +37,13 @@ describe("openCode models", () => { ); }); - it("rejects when discovery cannot run for configured model", async () => { + it("proceeds with the configured model when discovery cannot run (probe is best-effort, never fatal)", async () => { process.env.PAPERCLIP_OPENCODE_COMMAND = "__paperclip_missing_opencode_command__"; await expect( ensureOpenCodeModelConfiguredAndAvailable({ model: "openai/gpt-5", }), - ).rejects.toThrow("Failed to start command"); + ).resolves.toEqual([{ id: "openai/gpt-5", label: "openai/gpt-5" }]); }); it("skips the availability check when OPENCODE_ALLOW_ALL_MODELS is set in the run env", async () => { diff --git a/packages/adapters/opencode-local/src/server/models.ts b/packages/adapters/opencode-local/src/server/models.ts index 5838e070ab..172a5ce242 100644 --- a/packages/adapters/opencode-local/src/server/models.ts +++ b/packages/adapters/opencode-local/src/server/models.ts @@ -199,14 +199,35 @@ export async function ensureOpenCodeModelConfiguredAndAvailable(input: { return [{ id: model, label: model }]; } - const models = await discoverOpenCodeModelsCached({ - command: input.command, - cwd: input.cwd, - env: input.env, - }); + let models: AdapterModel[]; + try { + models = await discoverOpenCodeModelsCached({ + command: input.command, + cwd: input.cwd, + env: input.env, + }); + } catch (err) { + // The availability probe is a best-effort pre-flight guard, not a gate. If + // `opencode models` itself cannot run — a transient CLI error, a timeout, a + // provider hiccup — do NOT abort the run. The real invocation is + // authoritative, so a probe that can't execute must never be fatal. + // (Previously this threw and crashed runs mid-flight, discarding the agent's + // completed work and its terminal disposition, which then reopened the issue.) + console.warn( + `[opencode-local] Model availability probe could not run for "${model}" (${ + err instanceof Error ? err.message : String(err) + }); proceeding with the configured model.`, + ); + return [{ id: model, label: model }]; + } if (models.length === 0) { - throw new Error("OpenCode returned no models. Run `opencode models` and verify provider auth."); + // The probe ran but returned nothing (e.g. a transient provider-auth blip). + // Same reasoning as above: warn, don't block the run. + console.warn( + `[opencode-local] \`opencode models\` returned no models; proceeding with the configured model "${model}".`, + ); + return [{ id: model, label: model }]; } if (!models.some((entry) => entry.id === model)) {