Skip to content

Commit c8d1df0

Browse files
JDScriptclaude
andcommitted
fix(opencode): stop treating an empty fault-state response as a finished turn
A provider that finishes with `finish: "error"` having produced nothing was being recorded as a normal terminal state. The reason went onto the assistant message, no error object was ever built, `process()` returned "continue", and the next pass through the prompt loop found a `finish` that was not "tool-calls" with no tool calls beside it and simply broke — the `exiting loop` log, then idle. No error, no retry, and from the outside an agent that stops mid-task for no stated reason. Bedrock does this intermittently: eight times in one 48-hour stretch on `openai.gpt-5.6-sol`. The empty case is now raised as a failure from the `step-finish` handler, before the snapshot and before any part is written, so five spent attempts leave nothing behind in the timeline. That single failure is the whole fix: `Effect.retry(SessionRetry.policy(…))` already wraps the stream drain with `llm.stream()` inside it, so it buys the bounded five attempts, the backoff, the `retry` status, and — once those are spent — `halt` storing a visible error and returning "stop". retry.ts, prompt.ts and the AI SDK adapter are untouched. The neighbouring `provider-error` branch has always converted a stream event into a throw exactly like this. `ProviderError.ResponseStreamError` carries it because message-v2.ts already maps that to a retryable `APIError`, and `APIError` is one of the eight members of the closed `AssistantErrorSchema` union. A purpose-made error type would have to be added to that union, to core, and to the generated SDKs — which this fork never regenerates — and any client on an older build would then fail to decode the message it was stored on. The other candidate, `APICallError`, requires a fabricated `url`. Raising it in the processor rather than the adapter is deliberate: two runtimes reach this state, the AI SDK adapter passing the reason through and native Gemini's MALFORMED_FUNCTION_CALL, and a guard in the adapter would silently miss Gemini. Both arrive here as one `step-finish`. A turn that produced output must never be replayed — the retry re-sends the original request, so text would be duplicated and tools would run twice. Those are reported instead: `"error"` after any output sets a non-retryable `APIError` on the message and publishes it, which stops the loop with something on screen, mirroring what prompt.ts already does for a content filter. Reasoning counts as output for this purpose, since the reader has already seen it. `"unknown"` is replayed only when empty; on a turn that said something it is an ordinary completion, and several providers return one. Tests were confirmed to fail without the guards, not merely to pass with them: five of the six new processor tests fail when both are removed, and the sixth asserts the guards do *not* fire on a normal finish. The retry side gets the one genuinely new assertion — that this error classifies as retryable; the five-attempt bound does not vary by error and is already covered. Not reproducible on demand, so the tests are the verification. In production the signal is that `exiting loop` should now be preceded by a `retry` status or a stored `APIError`; a message with `finish="error"`, `error=null` and a straight drop to idle would mean the condition missed a case. Upstreamable, and worth dropping once upstream lands an equivalent — anomalyco#31430 and anomalyco#41469, where PR anomalyco#40531 covered only the `unknown` case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent eda7b84 commit c8d1df0

4 files changed

Lines changed: 291 additions & 0 deletions

File tree

FORK.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ upstream and cannot conflict.
174174
| `packages/session-ui/src/v2/components/prompt-input/index.tsx` | `trailingControl` slot on the prompt action row |
175175
| `packages/app/src/components/prompt-input-v2.tsx` | Fills it (TPS, scanner, context usage), exposes `sessionId`, registers `/usage` |
176176
| `packages/app/src/pages/home/home-projects-view.tsx` | Usage button in `HomeUtilityNav`, above settings and help |
177+
| `packages/opencode/src/session/processor.ts` | Raises an empty fault-state response as a retryable error instead of finishing |
177178

178179
### Non-obvious choices worth keeping
179180

@@ -204,6 +205,33 @@ upstream and cannot conflict.
204205
already a seam and is mounted for every session's composer, so the session entry point costs no extra
205206
upstream file. `CommandOption.slash` is the whole integration: `prompt-input-v2.tsx` builds its slash menu
206207
by filtering registered commands for that field.
208+
- **An empty response in a fault state is raised as an error from `processor.ts`, and only from there.** A
209+
provider that finishes with `finish: "error"` having produced nothing used to stop the session in silence:
210+
the reason went onto the message, no error object was ever built, and the next pass through the prompt loop
211+
found a `finish` that was not `tool-calls` and broke (`exiting loop`). Bedrock does it intermittently. Three
212+
things about the fix are easy to undo by accident:
213+
- **`processor.ts` is the only seam, and that is not a compromise.** `Effect.retry(SessionRetry.policy(…))`
214+
already wraps the stream drain while `llm.stream()` sits inside it, so one `Effect.fail` in the event
215+
handler buys the bounded retry, the backoff, the `retry` status, and `halt` storing a visible error and
216+
returning `"stop"`. `retry.ts`, `prompt.ts` and the AI SDK adapter need nothing. The neighbouring
217+
`provider-error` branch has always done exactly this with a bare `throw`.
218+
- **It must not move into `ai-sdk.ts`.** Two runtimes produce this state: the AI SDK adapter (which passes
219+
the reason through) and native Gemini's `MALFORMED_FUNCTION_CALL` (`protocols/gemini.ts`). A guard in the
220+
adapter silently misses Gemini. `processor.ts` sees both as one `step-finish`.
221+
- **`ProviderError.ResponseStreamError`, not a purpose-made error type.** `message-v2.ts` already maps it to
222+
a retryable `APIError`, and `APIError` is one of the eight members of the **closed** `AssistantErrorSchema`
223+
union in `packages/schema/src/v1/session.ts`. A ninth member would mean editing that union, core, and the
224+
generated SDKs — which this fork never regenerates — and any client on an older build would fail to decode
225+
the message it was stored on. `APICallError` was the other candidate and needs a fabricated `url`.
226+
227+
Reasoning counts as produced output, so a reasoning-only turn is reported rather than replayed: a retry
228+
re-sends the original request, and the reader has already seen the reasoning. A turn that produced anything
229+
and _then_ reported `"error"` is likewise reported, never replayed. `"unknown"` is replayed only when empty —
230+
on a turn that said something it is an ordinary completion, which several providers return.
231+
232+
Upstreamable, and worth dropping if upstream lands an equivalent: issues #31430 and #41469, where PR #40531
233+
covered only the `unknown` case.
234+
207235
- **The live TPS meter reads the sync store, not the SSE stream.** The TUI plugin it is ported from
208236
(`opencode-tps`, MIT — its README says the web UI cannot run it) subscribes to `message.part.delta`. There is
209237
no public event hook in this app, so doing the same would mean cutting seams into `server-sdk.tsx` and

packages/opencode/src/session/processor.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { SessionRetry } from "./retry"
1919
import { SessionStatus } from "./status"
2020
import { SessionSummary } from "./summary"
2121
import type { Provider } from "@/provider/provider"
22+
import { ProviderError } from "@/provider/error" // FORK
2223
import { Question } from "@/question"
2324
import { errorMessage } from "@/util/error"
2425
import { isRecord } from "@/util/record"
@@ -72,6 +73,10 @@ interface ProcessorContext extends Input {
7273
needsCompaction: boolean
7374
currentText: SessionV1.TextPart | undefined
7475
reasoningMap: Record<string, SessionV1.ReasoningPart>
76+
// FORK: whether this stream attempt produced anything a reader would have seen — text, reasoning or a tool
77+
// call. Per attempt, not per step: reset where the stream is (re)opened, so a retry starts clean. Gates the
78+
// empty-response guard in `step-finish`, which may only replay a turn that produced nothing.
79+
produced: boolean
7580
}
7681

7782
type StreamEvent = LLMEvent
@@ -111,6 +116,7 @@ const layer = Layer.effect(
111116
needsCompaction: false,
112117
currentText: undefined,
113118
reasoningMap: {},
119+
produced: false, // FORK
114120
}
115121
let aborted = false
116122

@@ -218,6 +224,9 @@ const layer = Layer.effect(
218224
name: string
219225
providerExecuted?: boolean
220226
}) {
227+
// FORK: the one place all four tool events pass through. A turn that got as far as naming a tool must
228+
// never be replayed — the retry re-sends the original request, so the tool would run twice.
229+
ctx.produced = true
221230
const existing = yield* readToolCall(input.id)
222231
if (existing) {
223232
if (!input.providerExecuted || existing.part.metadata?.providerExecuted) return existing
@@ -294,6 +303,9 @@ const layer = Layer.effect(
294303
case "reasoning-delta":
295304
// Match dev: silently drop orphan deltas (no preceding reasoning-start).
296305
if (!(value.id in ctx.reasoningMap)) return
306+
// FORK: reasoning counts as output — it is shown like any other part, so a retry would append a
307+
// second copy of it.
308+
if (value.text.length > 0) ctx.produced = true
297309
ctx.reasoningMap[value.id].text += value.text
298310
if (value.providerMetadata) ctx.reasoningMap[value.id].metadata = value.providerMetadata
299311
yield* session.updatePartDelta({
@@ -433,6 +445,20 @@ const layer = Layer.effect(
433445
return
434446

435447
case "step-finish": {
448+
// FORK: a fault-state finish with nothing to show for it is a failed request, not a finished
449+
// turn. Recorded as a finish it stopped the session in silence — no error, no retry, straight to
450+
// idle via prompt.ts's `exiting loop`. Failing is the whole fix: the retry policy already wraps
451+
// this stream, so it brings the bounded attempts and the visible error on exhaustion.
452+
//
453+
// Raised before the snapshot and before any part is written, so spent attempts leave no
454+
// step-finish parts behind. See FORK.md for why this error type and why not in the adapter.
455+
if ((value.reason === "error" || value.reason === "unknown") && !ctx.produced) {
456+
yield* Effect.fail(
457+
new ProviderError.ResponseStreamError(
458+
`Provider returned no output and finished with reason "${value.reason}"`,
459+
),
460+
)
461+
}
436462
const completedSnapshot = yield* snapshot.track()
437463
yield* Effect.forEach(Object.keys(ctx.reasoningMap), finishReasoning)
438464
const usage = Session.getUsage({
@@ -443,6 +469,19 @@ const layer = Layer.effect(
443469
ctx.assistantMessage.finish = value.reason
444470
ctx.assistantMessage.cost += usage.cost
445471
ctx.assistantMessage.tokens = usage.tokens
472+
// FORK: the other half — output first, then "error". Replaying it would duplicate text or re-run a
473+
// tool, so it is reported instead; the error makes `process()` return "stop". Set before the
474+
// `updateMessage` below so that write persists it.
475+
//
476+
// Never "unknown" here: an unmapped finish reason on a turn that produced output is an ordinary
477+
// completion, and several providers return one.
478+
if (value.reason === "error" && ctx.produced && !ctx.assistantMessage.error) {
479+
ctx.assistantMessage.error = new SessionV1.APIError({
480+
message: 'Provider reported finish reason "error" after producing partial output',
481+
isRetryable: false,
482+
metadata: { code: "ProviderResponseStreamError" },
483+
}).toObject()
484+
}
446485
yield* session.updatePart({
447486
id: PartID.ascending(),
448487
reason: value.reason,
@@ -454,6 +493,14 @@ const layer = Layer.effect(
454493
cost: usage.cost,
455494
})
456495
yield* session.updateMessage(ctx.assistantMessage)
496+
// FORK: announced the way `halt` and prompt.ts's content-filter branch announce theirs; without
497+
// it a client not re-reading the message would show no change.
498+
if (ctx.assistantMessage.error) {
499+
yield* events.publish(Session.Event.Error, {
500+
sessionID: ctx.assistantMessage.sessionID,
501+
error: ctx.assistantMessage.error,
502+
})
503+
}
457504
if (ctx.snapshot) {
458505
const patch = yield* snapshot.patch(ctx.snapshot)
459506
if (patch.files.length) {
@@ -498,6 +545,9 @@ const layer = Layer.effect(
498545

499546
case "text-delta":
500547
if (!ctx.currentText) return
548+
// FORK: on a delta, not on `text-start` — an empty response can still open and close a text
549+
// block without putting a character in it.
550+
if (value.text.length > 0) ctx.produced = true
501551
ctx.currentText.text += value.text
502552
if (value.providerMetadata) ctx.currentText.metadata = value.providerMetadata
503553
yield* session.updatePartDelta({
@@ -636,6 +686,8 @@ const layer = Layer.effect(
636686
yield* Effect.gen(function* () {
637687
ctx.currentText = undefined
638688
ctx.reasoningMap = {}
689+
// FORK: inside the block `Effect.retry` re-runs, so each attempt is judged on its own output.
690+
ctx.produced = false
639691
yield* status.set(ctx.sessionID, { type: "busy" })
640692
const stream = llm.stream(streamInput)
641693

packages/opencode/test/session/processor-effect.test.ts

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,3 +1112,200 @@ itFragmentFailure.live("session.processor effect tests retain partial legacy par
11121112
{ config: cfg },
11131113
),
11141114
)
1115+
1116+
// ---------------------------------------------------------------------------
1117+
// FORK: empty provider responses. An empty fault is replayed; one that produced output is reported instead.
1118+
//
1119+
// On a stub `LLM.Service` rather than the HTTP mock, because `finish: "error"` cannot be reached through it —
1120+
// the OpenAI-chat protocol maps every unrecognised finish_reason to "unknown". The counter is the assertion:
1121+
// `stream()` runs once per attempt, so it says how many times the request was sent.
1122+
// ---------------------------------------------------------------------------
1123+
1124+
function countingLLM(streams: () => ReadonlyArray<ReadonlyArray<LLMEvent>>) {
1125+
const attempts = { count: 0 }
1126+
const all = streams()
1127+
const layer = Layer.succeed(
1128+
LLM.Service,
1129+
LLM.Service.of({
1130+
stream: () => {
1131+
const index = attempts.count++
1132+
return Stream.fromIterable(all[Math.min(index, all.length - 1)])
1133+
},
1134+
}),
1135+
)
1136+
return { attempts, layer }
1137+
}
1138+
1139+
const emptyStep = (reason: "error" | "unknown") => [
1140+
LLMEvent.stepStart({ index: 0 }),
1141+
LLMEvent.stepFinish({ index: 0, reason }),
1142+
LLMEvent.finish({ reason }),
1143+
]
1144+
1145+
const textStep = (text: string) => [
1146+
LLMEvent.stepStart({ index: 0 }),
1147+
LLMEvent.textStart({ id: "text-1" }),
1148+
LLMEvent.textDelta({ id: "text-1", text }),
1149+
LLMEvent.textEnd({ id: "text-1" }),
1150+
LLMEvent.stepFinish({ index: 0, reason: "stop" }),
1151+
LLMEvent.finish({ reason: "stop" }),
1152+
]
1153+
1154+
const run = (
1155+
name: string,
1156+
streams: () => ReadonlyArray<ReadonlyArray<LLMEvent>>,
1157+
check: (input: {
1158+
attempts: { count: number }
1159+
value: "stop" | "continue" | "compact"
1160+
message: SessionV1.Assistant
1161+
parts: SessionV1.Part[]
1162+
seen: string[]
1163+
}) => void,
1164+
) => {
1165+
const { attempts, layer } = countingLLM(streams)
1166+
const scoped = testEffect(LayerNode.compile(root, [...replacements, [LLM.node, layer]]))
1167+
scoped.live(`session.processor effect tests ${name}`, () =>
1168+
provideTmpdirInstance(
1169+
(dir) =>
1170+
Effect.gen(function* () {
1171+
const { processors, session, provider } = yield* boot()
1172+
const events = yield* EventV2Bridge.Service
1173+
const chat = yield* session.create({})
1174+
const parent = yield* user(chat.id, name)
1175+
const msg = yield* assistant(chat.id, parent.id, path.resolve(dir))
1176+
const mdl = yield* provider.getModel(ref.providerID, ref.modelID)
1177+
const seen: string[] = []
1178+
const off = yield* events.listen((event) => {
1179+
seen.push(event.type)
1180+
return Effect.void
1181+
})
1182+
const handle = yield* processors.create({ assistantMessage: msg, sessionID: chat.id, model: mdl })
1183+
1184+
const value = yield* handle.process({
1185+
user: {
1186+
id: parent.id,
1187+
sessionID: chat.id,
1188+
role: "user",
1189+
time: parent.time,
1190+
agent: parent.agent,
1191+
model: { providerID: ref.providerID, modelID: ref.modelID },
1192+
} satisfies SessionV1.User,
1193+
sessionID: chat.id,
1194+
model: mdl,
1195+
agent: agent(),
1196+
system: [],
1197+
messages: [{ role: "user", content: name }],
1198+
tools: {},
1199+
})
1200+
yield* off
1201+
1202+
check({ attempts, value, message: handle.message, parts: yield* MessageV2.parts(msg.id), seen })
1203+
}),
1204+
{ config: cfg },
1205+
),
1206+
)
1207+
}
1208+
1209+
run(
1210+
"retry an empty error response and recover",
1211+
() => [emptyStep("error"), textStep("after")],
1212+
(r) => {
1213+
expect(r.attempts.count).toBe(2)
1214+
expect(r.value).toBe("continue")
1215+
expect(r.parts).toEqual(expect.arrayContaining([expect.objectContaining({ type: "text", text: "after" })]))
1216+
expect(r.message.error).toBeUndefined()
1217+
// The spent attempt must not leave its own step-finish behind.
1218+
expect(r.parts.filter((part) => part.type === "step-finish")).toHaveLength(1)
1219+
},
1220+
)
1221+
1222+
run(
1223+
"retry an empty unknown response and recover",
1224+
() => [emptyStep("unknown"), textStep("after")],
1225+
(r) => {
1226+
expect(r.attempts.count).toBe(2)
1227+
expect(r.value).toBe("continue")
1228+
expect(r.message.error).toBeUndefined()
1229+
},
1230+
)
1231+
1232+
run(
1233+
"do not retry an empty response that finished normally",
1234+
() => [
1235+
[
1236+
...emptyStep("error").slice(0, 1),
1237+
LLMEvent.stepFinish({ index: 0, reason: "stop" }),
1238+
LLMEvent.finish({ reason: "stop" }),
1239+
],
1240+
],
1241+
(r) => {
1242+
expect(r.attempts.count).toBe(1)
1243+
expect(r.message.error).toBeUndefined()
1244+
},
1245+
)
1246+
1247+
run(
1248+
"report but never replay an error after text",
1249+
() => [
1250+
[
1251+
LLMEvent.stepStart({ index: 0 }),
1252+
LLMEvent.textStart({ id: "text-1" }),
1253+
LLMEvent.textDelta({ id: "text-1", text: "partial" }),
1254+
LLMEvent.textEnd({ id: "text-1" }),
1255+
LLMEvent.stepFinish({ index: 0, reason: "error" }),
1256+
LLMEvent.finish({ reason: "error" }),
1257+
],
1258+
],
1259+
(r) => {
1260+
expect(r.attempts.count).toBe(1)
1261+
expect(r.value).toBe("stop")
1262+
expect(r.message.error?.name).toBe("APIError")
1263+
expect(r.parts).toEqual(expect.arrayContaining([expect.objectContaining({ type: "text", text: "partial" })]))
1264+
expect(r.seen).toContain(Session.Event.Error.type)
1265+
},
1266+
)
1267+
1268+
run(
1269+
"report but never replay an error after reasoning",
1270+
() => [
1271+
[
1272+
LLMEvent.stepStart({ index: 0 }),
1273+
LLMEvent.reasoningStart({ id: "reasoning-1" }),
1274+
LLMEvent.reasoningDelta({ id: "reasoning-1", text: "thinking" }),
1275+
LLMEvent.reasoningEnd({ id: "reasoning-1" }),
1276+
LLMEvent.stepFinish({ index: 0, reason: "error" }),
1277+
LLMEvent.finish({ reason: "error" }),
1278+
],
1279+
],
1280+
(r) => {
1281+
expect(r.attempts.count).toBe(1)
1282+
expect(r.value).toBe("stop")
1283+
expect(r.message.error?.name).toBe("APIError")
1284+
},
1285+
)
1286+
1287+
run(
1288+
"report but never replay an error after a tool call",
1289+
() => [
1290+
[
1291+
LLMEvent.stepStart({ index: 0 }),
1292+
LLMEvent.toolInputStart({ id: "call-1", name: "lookup" }),
1293+
LLMEvent.toolInputEnd({ id: "call-1", name: "lookup" }),
1294+
LLMEvent.toolCall({ id: "call-1", name: "lookup", input: {}, providerExecuted: true }),
1295+
LLMEvent.toolResult({
1296+
id: "call-1",
1297+
name: "lookup",
1298+
result: { type: "text", value: "ok" },
1299+
providerExecuted: true,
1300+
}),
1301+
LLMEvent.stepFinish({ index: 0, reason: "error" }),
1302+
LLMEvent.finish({ reason: "error" }),
1303+
],
1304+
],
1305+
(r) => {
1306+
// The point of the whole guard: one send, so the tool ran exactly once.
1307+
expect(r.attempts.count).toBe(1)
1308+
expect(r.value).toBe("stop")
1309+
expect(r.parts.filter((part) => part.type === "tool")).toHaveLength(1)
1310+
},
1311+
)

packages/opencode/test/session/retry.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,20 @@ describe("session.retry.retryable", () => {
257257
})
258258
})
259259

260+
// FORK: the classification half of the empty-response fix, and the only new part of it. Everything
261+
// downstream — five bounded attempts, the backoff, the stored error — is shared machinery that does not vary
262+
// by error, so `policy stops after five retries` above already covers the bound for this one too.
263+
test("retries empty provider responses", () => {
264+
const request = MessageV2.fromError(
265+
new ProviderError.ResponseStreamError('Provider returned no output and finished with reason "error"'),
266+
{ providerID },
267+
)
268+
expect(SessionV1.APIError.isInstance(request)).toBe(true)
269+
expect(SessionRetry.retryable(request, retryProvider)).toEqual({
270+
message: 'Provider returned no output and finished with reason "error"',
271+
})
272+
})
273+
260274
test("does not retry context overflow errors", () => {
261275
const error = new SessionV1.ContextOverflowError({
262276
message: "Input exceeds context window of this model",

0 commit comments

Comments
 (0)