audit(F4): narrow SSE_STALL_MESSAGE_RE to the emitted format by tesdal · Pull Request #7 · tesdal/opencode

tesdal · 2026-04-24T08:16:49Z

Summary

Audit finding F4 (2026-04-22 diamond audit) — the SSE stall detection regex /SSE (read|chunk) time(d out|out)/ was too loose: it matched 4 shapes but the code only ever emits one (SSE read timed out after <digits>ms, constructed in provider.ts:79).

This PR narrows the regex to /^SSE read timed out after \d+ms$/ so any future change to the wrapSSE() emission format must update the regex in lockstep.

Changes

packages/opencode/src/session/message-v2.ts — regex narrowed + explanatory comment pinning intent to the sole producer.
packages/opencode/test/session/message-v2-sse-stall.test.ts:
- +3 new tests: tag-based detection, exact-format positive fallback, speculative 'chunk timeout' rejection.
- +1 new test covering 5 shapes the old loose regex accepted and the narrowed one correctly rejects (missing suffix, wrong verb, timeout-not-timed-out, non-anchored prefix, trailing whitespace).
- 1 pre-existing test updated from the legacy "SSE chunk timeout" string to the actual emitted format, since the old string can no longer reach the regex path. Negative coverage for that legacy string is preserved in the new rejection tests.

Verification

bun test test/session/message-v2-sse-stall.test.ts → 9 pass / 0 fail
bun typecheck → clean
Diamond pre-review (codex-5.3 spec + general-purpose quality) → both APPROVE, 0 blocking findings, 2 non-blocking nits applied before this PR.

Review-only PR

This PR targets phase-ab-base on the fork for Copilot review. It will not be merged here; the commit merges into the local audit-remediation integration branch (local/integration-v2) via --no-ff after Copilot feedback is addressed.

github-actions · 2026-04-24T08:16:58Z

Hey! Your PR title audit(F4): narrow SSE_STALL_MESSAGE_RE to the emitted format doesn't follow conventional commit format.

Please update it to start with one of:

feat: or feat(scope): new feature
fix: or fix(scope): bug fix
docs: or docs(scope): documentation changes
chore: or chore(scope): maintenance tasks
refactor: or refactor(scope): code refactoring
test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

Copilot

Pull request overview

Narrows SSE stall detection in MessageV2.fromError to match the single, actual wrapSSE()-emitted timeout message format (audit finding F4), and updates/adds tests to ensure detection and rejection behavior is pinned to that format.

Changes:

Tighten SSE_STALL_MESSAGE_RE to ^SSE read timed out after \d+ms$ and document intent.
Update existing SSE stall message test to use the real emitted format.
Add tests for tag-based detection, exact-format message fallback, and rejection of previously-accepted loose matches.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
packages/opencode/src/session/message-v2.ts	Tightens the SSE stall detection regex and updates the surrounding intent comment.
packages/opencode/test/session/message-v2-sse-stall.test.ts	Updates/extends tests to validate exact-format matching and reject legacy/speculative variants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T08:20:09Z

-// Legacy substring fallback for rare paths where the tagged class gets stripped
-// during cross-realm rethrow. Primary signal is `name`/`_tag` set by SSEStallError.
-const SSE_STALL_MESSAGE_RE = /SSE (read|chunk) time(d out|out)/
+// Substring fallback matches the exact wrapSSE() emission format.


The comment says "Substring fallback" but the new regex is anchored (full-string match). This is misleading given the intent here is an exact message-format match; consider rewording to "message-based exact-match fallback" (or similar) so future readers don’t assume substring semantics.

Suggested change

// Substring fallback matches the exact wrapSSE() emission format.

// Message-based exact-match fallback for the wrapSSE() emission format.

Copilot · 2026-04-24T08:20:10Z

+// Substring fallback matches the exact wrapSSE() emission format.
+// The primary signal is `_tag === "SSEStallError"`; this is only
+// used when tag information is stripped during cross-realm rethrow.
+const SSE_STALL_MESSAGE_RE = /^SSE read timed out after \d+ms$/


/^...$/ in JS will also match before a final trailing newline, so this can still accept "SSE read timed out after 120000ms\n", which isn’t the emitted wrapSSE() format. If the goal is truly exact-format matching, either add a trailing negative lookahead to reject \r?\n or switch to a non-regex check that enforces both the prefix and digit+"ms" suffix without $’s newline behavior.

Thanks for the careful review — applied fixes for comments 1 and 3.

Respectfully pushing back on this one with empirical evidence: in V8/Bun (and ECMAScript generally, without the m flag), $ is true end-of-string and does NOT match before a trailing \n. Verified in Bun 1.3.13:

const re = /^SSE read timed out after \d+ms$/ re.test("SSE read timed out after 120000ms") // true re.test("SSE read timed out after 120000ms\n") // false re.test("SSE read timed out after 120000ms\r\n") // false re.test("SSE read\n timed out after 120000ms") // false

The "$ matches before final \n" behavior is Python/Ruby semantics, not JS (in JS you'd need the m flag, which we don't set).

That said, the concern is legitimate in spirit, so I added the trailing-newline case explicitly to the regression test suite at message-v2-sse-stall.test.ts:99 and documented this behavior in a code comment. If the regex engine semantics ever change, we'll know.

Copilot · 2026-04-24T08:20:10Z

+  test("hasSSEStallCause: substring fallback matches the exact wrapSSE format", () => {
+    const taggedless = new Error("SSE read timed out after 120000ms")
+    const result = MessageV2.fromError(taggedless, { providerID })


This test overlaps with the earlier "detects SSE stall by timeout message without SSEStallError name" (it exercises the same untagged timeout-message path). Consider either removing the duplication or repurposing this to cover a distinct edge case; also the phrase "substring fallback" is no longer accurate now that the regex is anchored to the full message.

Suggested change

test("hasSSEStallCause: substring fallback matches the exact wrapSSE format", () => {

const taggedless = new Error("SSE read timed out after 120000ms")

const result = MessageV2.fromError(taggedless, { providerID })

test("hasSSEStallCause: exact wrapSSE-format message is detected through cause chain", () => {

const taggedless = new Error("SSE read timed out after 120000ms")

const outer = new Error("outer")

outer.cause = taggedless

const result = MessageV2.fromError(outer, { providerID })

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T08:37:46Z

+// The primary signal is `_tag === "SSEStallError"`; this regex only
+// matches when tag information is stripped during cross-realm rethrow.


The comment says the primary signal is _tag === "SSEStallError", but hasSSEStallCause() also treats err.name === "SSEStallError" as a primary signal. Please update the comment to mention both (name and _tag) so it accurately reflects the logic and avoids misleading future changes.

Suggested change

// The primary signal is `_tag === "SSEStallError"`; this regex only

// matches when tag information is stripped during cross-realm rethrow.

// The primary signals are `name === "SSEStallError"` and `_tag === "SSEStallError"`;

// this regex only matches when that structured error identity is stripped during

// cross-realm rethrow.

Copilot · 2026-04-24T08:37:46Z

+    // The previous regex /SSE (read|chunk) time(d out|out)/ matched all of these;
+    // the narrowed /^SSE read timed out after \d+ms$/ rejects them all.
+    // Any future wrapSSE format change must update the regex in lockstep.
+    // Note: V8's `$` without the `m` flag is true end-of-string and does NOT


This test comment attributes $ end-of-string behavior to V8, but Bun runs on JavaScriptCore and the behavior is specified by ECMAScript (engine-independent). Consider rephrasing to something like “In JS regexes, $ without the m flag matches only end-of-input …” to avoid implying engine-specific semantics.

Suggested change

// Note: V8's `$` without the `m` flag is true end-of-string and does NOT

// Note: In JS regexes, `$` without the `m` flag matches only end-of-input and does NOT

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).

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tesdal · 2026-04-24T08:49:36Z

Review-only PR. Copilot feedback iterated across 3 rounds (final: no new comments). Final commit 27ca380 merges into local/integration-v2 via --no-ff; see LOCAL-INTEGRATION.md tracker.

github-actions Bot added the needs:title label Apr 24, 2026

tesdal requested a review from Copilot April 24, 2026 08:17

Copilot started reviewing on behalf of tesdal April 24, 2026 08:17 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

tesdal force-pushed the audit-f4-sse-regex-narrow branch from d9d1848 to cc529db Compare April 24, 2026 08:33

tesdal requested a review from Copilot April 24, 2026 08:34

Copilot started reviewing on behalf of tesdal April 24, 2026 08:35 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

tesdal force-pushed the audit-f4-sse-regex-narrow branch from cc529db to 27ca380 Compare April 24, 2026 08:40

tesdal requested a review from Copilot April 24, 2026 08:43

Copilot started reviewing on behalf of tesdal April 24, 2026 08:43 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

tesdal closed this Apr 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

audit(F4): narrow SSE_STALL_MESSAGE_RE to the emitted format#7

audit(F4): narrow SSE_STALL_MESSAGE_RE to the emitted format#7
tesdal wants to merge 1 commit intophase-ab-basefrom
audit-f4-sse-regex-narrow

tesdal commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

tesdal Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

tesdal commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// Substring fallback matches the exact wrapSSE() emission format.
	// Message-based exact-match fallback for the wrapSSE() emission format.

		// The primary signal is `_tag === "SSEStallError"`; this regex only
		// matches when tag information is stripped during cross-realm rethrow.

-// The primary signal is `_tag === "SSEStallError"`; this regex only
-// matches when tag information is stripped during cross-realm rethrow.
+// The primary signals are `name === "SSEStallError"` and `_tag === "SSEStallError"`;
+// this regex only matches when that structured error identity is stripped during
+// cross-realm rethrow.

	// Note: V8's `$` without the `m` flag is true end-of-string and does NOT
	// Note: In JS regexes, `$` without the `m` flag matches only end-of-input and does NOT

Conversation

tesdal commented Apr 24, 2026

Summary

Changes

Verification

Review-only PR

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

tesdal Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tesdal commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants