fix(client): tolerate empty OpenCode text events from tool-only steps - #2369
fix(client): tolerate empty OpenCode text events from tool-only steps#2369L42y wants to merge 2 commits into
Conversation
OpenCode stores an empty text part for assistant steps that only call
tools and re-emits stored parts when a session is resumed. The parser
flagged those empty text events as protocol violations, which failed the
whole turn on resume ("unsupported or malformed OpenCode JSONL"). Skip
empty text events instead; they carry no content and are not malformed.
|
All contributors are covered by the First Tree CLA. |
bestony
left a comment
There was a problem hiding this comment.
Requesting changes for one protocol-validation regression.
packages/client/src/providers/opencode/parser.ts:91 uses string(part?.text) ?? string(row.text), which collapses empty/missing text and every non-string value into the same null. The new if (text) branch then silently accepts malformed events such as { "type": "text", "part": { "text": 123 } }.
The PR's intended compatibility exception is limited to empty/missing text emitted by tool-only steps. Please distinguish empty/missing values from present non-string values so malformed known events still produce an unknown protocol diagnostic. Add regression coverage for both the observed text: "" shape and a non-string text value; the current test covers only a missing field (part: {}).
The focused OpenCode suite passes (56/56), as do Biome and the client typecheck with an increased Node heap. No standards-axis violations or code-smell findings were found.
yuezengwu
left a comment
There was a problem hiding this comment.
Reviewed exact head a831ace as an external contribution; this is not an approval.
The protocol exception is broader than the observed compatibility case. A local review-only regression probe for {type:"text", part:{text:123}} fails: the parser returns only the session event and no unknown diagnostic. The same branch also accepts a text event with the text field absent. By contrast, both the minimum supported OpenCode v1.18.7 schema and the current schema require TextPart.text to be a string, while the JSON run path emits the complete part. Empty string is valid and should be skipped; absent or non-string text is malformed and must continue to fail closed.
Please narrow the branch by checking field presence and type before deciding whether an empty string is ignorable. Keep the supported row-level fallback explicit if it is still required. Add regression cases for the actual observed part.text:"" shape, non-string text, and missing text; the current new test covers only part:{} and therefore does not match the production evidence.
Verification at this head: OpenCode provider suite 56/56 passed; client typecheck, Biome on both changed files, and git diff --check passed. The review-only malformed-value probe failed exactly as described and was removed afterwards.
Independent merge blocker: the CLA check is failing because the commit author kimi is not linked to a GitHub account. The contributor must complete the ICLA path and associate or replace that commit identity, then recheck. As the author is outside the confirmed organization roster, a maintainer must perform the final approval and merge after a fresh-head review.
|
Addressed both protocol-validation findings in commit
Verification on the new head: the focused OpenCode parser suite passes 6/6, the affected client typecheck passes, targeted Biome passes, and the repository Biome check passes. Please review the exact current head |
bestony
left a comment
There was a problem hiding this comment.
代码审查通过。新 head 73f150434 已将兼容范围收窄为显式字符串 part.text === "";缺失与非字符串字段继续产生协议诊断,且 row-level fallback 仅在 part 没有 text 字段时使用。新增测试覆盖空字符串、缺失/非字符串值及 fallback 优先级。
本地 OpenCode provider suite 58/58、Biome(改动文件及仓库检查)和 Client typecheck 均通过。GitHub 当前已完成的非 CLA 检查均通过,剩余 Test Client & Web 与 Test Server 仍在运行;CLA 仍因原始提交作者 kimi 未关联 GitHub 账户而失败,这是独立的合并阻塞项。
73f1504 to
232fcc8
Compare
Summary
parseOpenCodeStreamLineflaggedtextJSONL events with an empty/missingpart.textasunknownprotocol violations, and any diagnostic fails the whole OpenCode turn (unsupported or malformed OpenCode JSONL).textevents instead of flagging them. Session-id extraction is preserved.Test plan
parser.test.ts: empty-text line now yields only the session event (nounknowndiagnostic); added explicit regression test.vitest run src/providers/opencode/— 56/56 tests pass.