fix(qa): defer partial Crabline recorder rows#99649
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 6:18 PM ET / 22:18 UTC. Summary PR surface: Source +15, Tests +77. Total +92 across 2 files. Reproducibility: yes. Current main source parses every non-empty active recorder fragment, and the linked issue plus PR tests provide a concrete split-write/reset path that reaches the reported JSON.parse failure. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the focused recorder-tail fix after normal maintainer review; the linked bug can close after this PR merges. Do we have a high-confidence way to reproduce the issue? Yes. Current main source parses every non-empty active recorder fragment, and the linked issue plus PR tests provide a concrete split-write/reset path that reaches the reported JSON.parse failure. Is this the best way to solve the issue? Yes. Using newline as the live completed-record boundary while keeping final cleanup strict is narrower and safer than catching or ignoring JSON.parse failures broadly. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 525c4c7ce1b8. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +15, Tests +77. Total +92 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
Non-maintainer review pass on What I checked:
Result:
/Users/harjothk/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node scripts/run-vitest.mjs extensions/qa-lab/src/crabline-transport.test.ts -- -t "partial recorder tail|malformed newline-terminated|permanently truncated" --reporter=verboseResult: 1 test file passed; 3 tests passed, 15 skipped. Residual scope: I did not rerun the full QA smoke scenario or the Testbox lane; this was a focused source review plus the new regression cases on the PR head. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
What Problem This Solves
Fixes #99648.
QA Lab polls Crabline's JSONL recorder while the provider is actively appending records. A read can end inside the final record, and the transport previously passed that fragment directly to
JSON.parse. The observed failure wasUnterminated string in JSON at position 191innative-command-session-target.Why This Change Was Made
User Impact
Crabline-backed QA scenarios no longer fail when a recorder read overlaps a valid append. Corrupt completed records remain failures rather than being ignored.
Evidence
8074574444recorded 24 passing scenarios and onlynative-command-session-targetfailing at JSON byte 191.node scripts/run-vitest.mjs extensions/qa-lab/src/crabline-transport.test.ts -- -t 'partial recorder tail|malformed newline-terminated|permanently truncated' --reporter=verbose— 3 passed.tbx_01kwmwgp9ysjr4p1vx3gcc8p01:corepack pnpm test extensions/qa-lab/src/crabline-transport.test.ts -- --reporter=verbose— 18 passed.corepack pnpm openclaw qa run --repo-root . --qa-profile smoke-ci --scenario native-command-session-target --concurrency 1 --output-dir .artifacts/qa-e2e/native-command-json-fix— passed.