Skip to content

Commit cc529db

Browse files
committed
fix(message-v2): narrow SSE_STALL_MESSAGE_RE to the emitted format
The fallback regex previously allowed 'chunk timeout' / 'timeout' variants that no caller ever emits. The only producer is wrapSSE() in provider/provider.ts, which always emits the exact string 'SSE read timed out after Nms'. Anchors the regex and pins it to that format with a test, so any future change to wrapSSE() format forces a corresponding update here. Addresses audit finding F4 (Opus diamond review, 2026-04-22).
1 parent 552fd0b commit cc529db

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

packages/opencode/src/session/message-v2.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,9 +1076,10 @@ export const filterCompactedEffect = Effect.fnUntraced(function* (sessionID: Ses
10761076
return filterCompacted(stream(sessionID))
10771077
})
10781078

1079-
// Legacy substring fallback for rare paths where the tagged class gets stripped
1080-
// during cross-realm rethrow. Primary signal is `name`/`_tag` set by SSEStallError.
1081-
const SSE_STALL_MESSAGE_RE = /SSE (read|chunk) time(d out|out)/
1079+
// Message-based exact-match fallback for the wrapSSE() emission format.
1080+
// The primary signal is `_tag === "SSEStallError"`; this regex only
1081+
// matches when tag information is stripped during cross-realm rethrow.
1082+
const SSE_STALL_MESSAGE_RE = /^SSE read timed out after \d+ms$/
10821083

10831084
function hasSSEStallCause(e: unknown, depth = 0): boolean {
10841085
if (depth > 8) return false

packages/opencode/test/session/message-v2-sse-stall.test.ts

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ describe("session.message-v2.fromError — SSEStallError", () => {
2929
})
3030

3131
test("detects SSE stall by timeout message without SSEStallError name", () => {
32-
const error = new Error("SSE chunk timeout after 120000ms")
32+
const error = new Error("SSE read timed out after 120000ms")
3333

3434
const result = MessageV2.fromError(error, { providerID })
3535

3636
expect(result.name).toBe("SSEStallError")
3737
expect(MessageV2.SSEStallError.isInstance(result)).toBe(true)
3838
if (!MessageV2.SSEStallError.isInstance(result)) throw new Error("Expected SSEStallError")
39-
expect(result.data.message).toBe("SSE chunk timeout after 120000ms")
39+
expect(result.data.message).toBe("SSE read timed out after 120000ms")
4040
})
4141

4242
test("detects SSE stall through two-deep cause chain", () => {
@@ -65,4 +65,44 @@ describe("session.message-v2.fromError — SSEStallError", () => {
6565
expect(result.name).toBe("SSEStallError")
6666
expect(MessageV2.APIError.isInstance(result)).toBe(false)
6767
})
68+
69+
test("hasSSEStallCause: tag-based detection still works", () => {
70+
const tagged = Object.assign(new Error("anything"), { _tag: "SSEStallError" })
71+
const result = MessageV2.fromError(tagged, { providerID })
72+
expect(MessageV2.SSEStallError.isInstance(result)).toBe(true)
73+
})
74+
75+
test("hasSSEStallCause: exact wrapSSE-format message is detected through cause chain", () => {
76+
const taggedless = new Error("SSE read timed out after 120000ms")
77+
const outer = new Error("outer")
78+
outer.cause = taggedless
79+
const result = MessageV2.fromError(outer, { providerID })
80+
expect(MessageV2.SSEStallError.isInstance(result)).toBe(true)
81+
})
82+
83+
test("hasSSEStallCause: message-regex fallback rejects speculative 'chunk timeout' variants", () => {
84+
const speculative = new Error("SSE chunk timeout")
85+
const result = MessageV2.fromError(speculative, { providerID })
86+
expect(MessageV2.SSEStallError.isInstance(result)).toBe(false)
87+
})
88+
89+
test("hasSSEStallCause: narrowed regex rejects shapes the old loose regex accepted", () => {
90+
// The previous regex /SSE (read|chunk) time(d out|out)/ matched all of these;
91+
// the narrowed /^SSE read timed out after \d+ms$/ rejects them all.
92+
// Any future wrapSSE format change must update the regex in lockstep.
93+
// Note: V8's `$` without the `m` flag is true end-of-string and does NOT
94+
// match before a trailing "\n", so newline-suffixed messages are rejected.
95+
const cases = [
96+
"SSE read timed out", // missing "after Nms" suffix
97+
"SSE chunk timed out after 120000ms", // wrong verb ("chunk" never emitted)
98+
"SSE read timeout after 120000ms", // "timeout" not "timed out"
99+
"prefix: SSE read timed out after 120000ms", // non-anchored prefix
100+
"SSE read timed out after 120000ms ", // trailing whitespace
101+
"SSE read timed out after 120000ms\n", // trailing newline ($ does not match before \n)
102+
]
103+
for (const message of cases) {
104+
const result = MessageV2.fromError(new Error(message), { providerID })
105+
expect(MessageV2.SSEStallError.isInstance(result)).toBe(false)
106+
}
107+
})
68108
})

0 commit comments

Comments
 (0)