Skip to content

fix(opencode-go): treat stream block boundaries as liveness#96526

Closed
lin-hongkuan wants to merge 1 commit into
openclaw:mainfrom
lin-hongkuan:codex/opencode-go-boundary-liveness
Closed

fix(opencode-go): treat stream block boundaries as liveness#96526
lin-hongkuan wants to merge 1 commit into
openclaw:mainfrom
lin-hongkuan:codex/opencode-go-boundary-liveness

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

The opencode-go stalled-stream watchdog only treated provider delta events as stream liveness. Live streams that emitted block-boundary events, such as tool call end/start transitions, could sit between token deltas long enough for the idle timer to abort the underlying request and replace the eventual completed answer with an error.

Why This Change Was Made

This keeps the stricter first-delta behavior intact, so synthetic start or block-start events before provider output do not shorten the first-event window. Once a real provider delta has arrived, later provider-owned block boundaries now refresh the idle timer because they prove the SSE stream is still advancing.

User Impact

Users of opencode-go models should no longer lose completed answers when a provider pauses token deltas while closing one block and opening another. Truly stalled streams are still aborted after the configured idle timeout.

Evidence

  • node scripts/run-vitest.mjs run extensions/opencode-go/stream-termination.test.ts
  • & .\node_modules\.bin\oxfmt.cmd --check --threads=1 extensions/opencode-go/stream-termination.ts extensions/opencode-go/stream-termination.test.ts
  • git diff --check
    Live opencode-go wrapper stream proof from this branch:
$ <script invokes extensions/opencode-go/stream-termination.ts#createOpencodeGoStalledStreamWrapper with real timers and opencode-go stream events>
{
  "source": "extensions/opencode-go/stream-termination.ts#createOpencodeGoStalledStreamWrapper",
  "results": [
    {
      "scenario": "post-delta boundary events keep stream live",
      "idleTimeoutMs": 100,
      "elapsedAfterDeltaMs": 210,
      "boundaryEvents": [
        "toolcall_end",
        "toolcall_start"
      ],
      "abortCalled": false,
      "terminalEvent": "done",
      "errorEvents": 0
    },
    {
      "scenario": "true post-delta stall still aborts",
      "idleTimeoutMs": 100,
      "quietAfterDeltaMs": 160,
      "abortCalled": true,
      "terminalEvent": "error",
      "errorMessage": "opencode-go stream timed out after provider-owned SSE boundary stalled"
    }
  ]
}

This exercises the actual opencode-go stalled-stream wrapper. The first stream stays live across post-delta toolcall_end and toolcall_start boundaries for longer than one idle window and completes with done. The second stream has no boundary or delta after its first delta, so the wrapper still aborts and emits the provider-owned stalled-stream error.

Fixes #96518

@openclaw-barnacle openclaw-barnacle Bot added extensions: opencode-go size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 24, 2026
@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 8:05 AM ET / 12:05 UTC.

Summary
The PR updates the opencode-go stalled-stream wrapper so post-delta provider block-boundary events refresh idle liveness and adds regression coverage for that boundary-only gap.

PR surface: Source +14, Tests +76. Total +90 across 2 files.

Reproducibility: yes. source-reproducible: current main forwards boundary events but only re-arms the idle timer on deltas, and the linked issue includes a focused failing timing test. I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Boundary liveness event types: 4 added. text_end, thinking_end, toolcall_start, and toolcall_end now affect opencode-go idle cancellation timing after provider deltas.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96518
Summary: This PR is the candidate fix for the canonical opencode-go watchdog false-abort issue; older stream-stall work overlaps but does not replace this boundary-liveness fix.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The patch intentionally lets post-delta block-boundary events keep an opencode-go stream open longer, so maintainers should accept the availability timing tradeoff before merge.
  • [P0] The supplied real behavior proof exercises the production wrapper with controlled opencode-go events rather than forcing a live external provider outage.

Maintainer options:

  1. Land with wrapper proof (recommended)
    Merge after maintainer review and required checks if controlled production-wrapper proof is enough for this timing fix.
  2. Ask for live provider evidence
    Request redacted live opencode-go provider logs or terminal output if maintainers want service-level confidence beyond controlled wrapper proof.
  3. Pause behind the linked issue
    Keep the PR open and use the linked issue as the active decision point if maintainers do not accept boundary events as liveness.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer review of the provider availability timing tradeoff and normal merge/check gating.

Security
Cleared: The diff only changes provider-local TypeScript stream timing logic and tests, with no dependency, workflow, secret, permission, or package-resolution changes.

Review details

Best possible solution:

Land the provider-owned wrapper fix after maintainer review accepts the cancellation timing tradeoff and required checks remain green; keep the linked issue open until this PR merges.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main forwards boundary events but only re-arms the idle timer on deltas, and the linked issue includes a focused failing timing test. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes, this is the best code layer: the opencode-go provider wrapper owns the stalled-stream watchdog, and the post-delta gate preserves the first-event timeout contract while treating later provider boundaries as liveness.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4ecb45bf7729.

Label changes

Label justifications:

  • P1: The PR targets a provider-stream bug that can replace live opencode-go answers with stalled errors for real users.
  • merge-risk: 🚨 availability: The diff changes when opencode-go provider streams are kept alive versus aborted after provider output begins.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix live output from the production opencode-go wrapper showing boundary events keeping a stream live and true stalls still aborting.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix live output from the production opencode-go wrapper showing boundary events keeping a stream live and true stalls still aborting.
Evidence reviewed

PR surface:

Source +14, Tests +76. Total +90 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 19 5 +14
Tests 1 76 0 +76
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 95 5 +90

What I checked:

Likely related people:

  • zhangguiping-xydt: Authored the merged opencode-go stream-termination work in fix(opencode-go): streaming completes when provider ends responses #93965, which added the wrapper and delta-only liveness predicate modified here. (role: introduced behavior; confidence: high; commits: 769579bcf0c2, 09486e88b8f7, 3a8faee4cc4f; files: extensions/opencode-go/stream-termination.ts, extensions/opencode-go/stream-termination.test.ts, extensions/opencode-go/stream.ts)
  • Josh Lehman: Current-main blame in this checkout points the implicated predicate and re-arm site to the commit carrying the wrapper files into the current checked-out history. (role: current-main provenance; confidence: medium; commits: 0cdb050bac19; files: extensions/opencode-go/stream-termination.ts, extensions/opencode-go/stream-termination.test.ts, extensions/opencode-go/stream.ts)
  • Vincent Koc: Recent local history shows provider/plugin seam and opencode-go-adjacent work in the extension area, making this a plausible routing candidate for provider boundary review. (role: adjacent provider/plugin contributor; confidence: medium; commits: 66701d5a1e16, 4c15f1310bd5, 3f9e93fd28ef; files: extensions/opencode-go, src/plugin-sdk/provider-stream-shared.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 24, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 25, 2026
@lin-hongkuan

lin-hongkuan commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 25, 2026
This was referenced Jun 25, 2026
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: opencode-go merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

1 participant

@lin-hongkuan