Skip to content

Commit 257bdb2

Browse files
committed
Merge F4: narrow SSE_STALL_MESSAGE_RE to the emitted format
Audit finding F4 (2026-04-22 diamond audit): narrow the SSE stall detection regex from /SSE (read|chunk) time(d out|out)/ to /^SSE read timed out after \d+ms$/ to match exactly the one shape wrapSSE() actually emits. Diamond pre-review (codex-5.3 + general-purpose): both APPROVE, 0 blocking. Copilot iteration on #7: 3 rounds, converged to no new comments. - Strengthened negative test suite with 6 shapes the old loose regex accepted (missing suffix, wrong verb, timeout-not-timed-out, prefix, trailing whitespace, trailing newline). - Repurposed overlapping positive test to cover cause-chain path. - Documented JS regex $ end-of-input semantics (no m flag = no newline match).
2 parents 552fd0b + 27ca380 commit 257bdb2

2 files changed

Lines changed: 48 additions & 5 deletions

File tree

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,9 +1076,11 @@ 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 signals are `name === "SSEStallError"` and `_tag === "SSEStallError"`;
1081+
// this regex only matches when that structured error identity is stripped during
1082+
// cross-realm rethrow.
1083+
const SSE_STALL_MESSAGE_RE = /^SSE read timed out after \d+ms$/
10821084

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

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

Lines changed: 43 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,45 @@ 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: In JS regexes, `$` without the `m` flag matches only end-of-input
94+
// and does NOT match before a trailing "\n", so newline-suffixed messages
95+
// are rejected.
96+
const cases = [
97+
"SSE read timed out", // missing "after Nms" suffix
98+
"SSE chunk timed out after 120000ms", // wrong verb ("chunk" never emitted)
99+
"SSE read timeout after 120000ms", // "timeout" not "timed out"
100+
"prefix: SSE read timed out after 120000ms", // non-anchored prefix
101+
"SSE read timed out after 120000ms ", // trailing whitespace
102+
"SSE read timed out after 120000ms\n", // trailing newline ($ does not match before \n)
103+
]
104+
for (const message of cases) {
105+
const result = MessageV2.fromError(new Error(message), { providerID })
106+
expect(MessageV2.SSEStallError.isInstance(result)).toBe(false)
107+
}
108+
})
68109
})

0 commit comments

Comments
 (0)