Repository navigation
fix(status): yield during OmO and GJC transcript replay - #2361
Conversation
Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Cooperative transcript reader server/omo-records.ts |
A shared chunk reader supports synchronous and cooperative line reading. The cooperative reader checks currentness and yields between bounded reads. |
Asynchronous status polling and publication server/omo-status.ts, server/omo-records.ts, server/omo-status.test.ts, changelog.d/cooperative-status-replay.md |
Status polling awaits transcript reads and publishes staged status after a valid read. Tests cover health requests during replay and OmO/GJC polling. |
Refresh and replay invalidation server/omo-status.ts, server/omo-status.test.ts |
Refresh and stop use generation checks for pending discovery and reads. Tests cover stopping, stale replay, and file replacement or truncation. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Bug fix · Severity of issue fixed: High
Suggested reviewers: devswha
Merge Risk: 
e1730
-
Large OmO and GJC transcript replays now yield to the event loop, and obsolete replays are discarded before they can publish status. I found no concrete merge-blocking risk in the supplied context. Normal CI should still cover the browser and native Windows lanes that were not run locally.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: yielding during OmO and GJC transcript replay to keep the event loop responsive. Description check The description includes the required Change and Validation sections. It explains the problem, resulting behavior, design trade-offs, test coverage, environment, and known baseline failures. Linked Issues check Issue #2360is directly linked and has coding requirements.server/omo-records.tsshares the decoder and projection path, andreadOmoLinesCooperativelyyields after each 256 KiB chunk, including u…Out of Scope Changes check The changed source, tests, and changelog implement or validate issue #2360. The bridge-health test directly exercises the reported event-loop stall. Replay lifecycle and file-validity tests support th…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Linked Issues check
-
Explanation
Issue
#2360is directly linked and has coding requirements.server/omo-records.tsshares the decoder and projection path, andreadOmoLinesCooperativelyyields after each 256 KiB chunk, including unfinished JSONL records.server/omo-status.tsstages replay state, coalesces one read per tracked pane, cancels obsolete reads, and checks pane and file identity before publication. The added tests cover the real bridge-health request, OmO/GJC baselines, long records, session and pane changes, replacement, truncation, and shutdown. The reported focused suite passed 107 tests.
Full details: Out of Scope Changes check
-
Explanation
The changed source, tests, and changelog implement or validate issue
#2360. The bridge-health test directly exercises the reported event-loop stall. Replay lifecycle and file-validity tests support the required stale-publication behavior. No unrelated change is demonstrated by the reviewed summary or focused diff.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
|
Pushed cdce0cc: Invalidate both native trackers on pane end so a suspended OmO/GJC replay cannot publish stale status after pane-exited (merge-pass review finding). |
|
Merging cdce0cc, thank you @eddieparc! Fixes #2360. The first OmO/GJC status replay used to scan a whole transcript synchronously, starving Bun's event loop (bridge health included); it now yields every 256 KiB, also inside one long JSONL record, sharing one decoder with the synchronous reader. Measured by the merge pass on a 100 MiB transcript: event-loop lag max/p99 1061/1061 ms on main -> 15.7/2.2 ms with this PR; bridge health answers in 18.7 ms during replay instead of 1061 ms; replay wall time 1061 -> 925 ms. 12 decoder-equivalence fixtures (UTF-8 split across chunks, partial-line resume) give identical projections. Review finding fixed on the branch by the merge pass (cdce0cc): a replay suspended across pane end was still applied, because the pane-ended handler never invalidated the OmO/GJC trackers (pane-exited followed by a stale 'working' over the real WebSocket); both trackers are now cancelled on pane end, with tests on the production callback that fail without it. A replay for a reused pane id is unaffected. Delta re-review PASS, no findings left. |
## Release 0.4.21 The release PR, prepared by step 1 of [docs/development.md#releasing](https://github.com/devswha/ovis/blob/main/docs/development.md#releasing), now re-folded on main `1ef6ea329108a0985b823e8235b03daddcd0a5f4` after the owner held 0.4.21 for more work. The merge pass reported the 0.4.21 queue empty. **Do not merge yet:** the owner judges the final build before release. No tag and no Release run from this PR's author. - **Version:** `version` is 0.4.21 in `package.json` and `herdr-plugin.toml`. - **Changelog:** `bun scripts/changelog-fold.ts 0.4.21 2026-10-30` folded 63 fragments into the 2026-10-30 section of `CHANGELOG.md` (71 entries), linked their PRs and deleted the fragments; `changelog.d` holds only `README.md`. - `by @login` on every outside contributor's PR link: #2361 @eddieparc, #2333 @IdrisGit, #2355 @imurfutur3, #2341 @sulthonzh, #2359 @YuuuXie, #2377 @IdrisGit, #2269 @CharleoYang, #2405 @jpcarranza94, #2414 @homebodify, #2417 @cheol-95, #2418 @eddieparc, #2435 @cheol-95. - Two outside PRs had no fragment and got an entry by hand: #2341 (OpenCode background commands, Added) and #2359 (installer test timing, Maintenance). - Issue reporters are credited in their entries: @blaker3115-Agri (#1488), @Hyd3-14 (#2026), @osamahbeig (#2343), @Shunamxiao (#2329), @matiemer (#2362), @kilhyeonjun (#2344), @eddieparc (#2360), @Ploppy3 (#2236), @sulthonzh (#2340), @WOULDU-pres (#2056), @odfalik (#2038), @aNNdii (#511), @Noobear (#2281, fixed by #2424); #2435 fixes @cheol-95's own #2434. - Two fragments ran past the changelog's width (#2418, #2269); their entries are re-wrapped, words unchanged. #2382's entry reads "OpenCode's installer puts its folder first" (reviewed earlier on this PR). - Maintainer PRs without a fragment and without a user-facing change have no entry: #2353 (check tool), #2371 (guide docs), #2396 and #2397 (tests), #2403 and #2433 (server and client transport seams, no behaviour change), #2437 (test-only fix for #2435's check). #2438's own entry (the sidebar selection marked by its fill again) is dropped: the stroke #2352 added never shipped, so users see no change; #2352's entry keeps only the Windows high-contrast Highlight bar. - **Release summaries:** en, ko, ja and zh each have 5 new, 5 improved and 5 fixed lines. Since the first review: New lists the in-browser Claude Code / Codex sign-in (#2428) in place of the tab/workspace shortcut line; Improved lists Settings → Agents (#2427) in place of the sidebar fold line; Fixed lists the Codex wrong-session fix (#2426, #2430) in place of the phone screenshot paste line. The Artifacts and Claude status-line lines keep their reviewed wording (turned on in Settings). Not in 0.4.21: #2420 (Codex chat behind Kiro), #2065 (Portal sign-in) and #2436 (relay foundations) stay open for a later release. - **Remote bundle:** `REMOTE_BUNDLE_VERSION` rises from 39 to 40 because the bundle inputs (`server/`, `shared/`, `patches/`) changed since remote-v39. After merge: push `remote-v40` on the merged commit and wait for the Remote PC bundles workflow before running Release. - **iOS:** `CURRENT_PROJECT_VERSION` stays 3 (#2370 changed `Info.plist`; build 3 is not known to be uploaded yet). ## Validation - `bun scripts/release-guards.ts --base v0.4.20`: "Release 0.4.21: remote bundle, telemetry Worker and release metadata are in order." - `dori heavy … bun run check fast` on this branch: workflow syntax, generated types and themes, typecheck, build, unit tests all ok. Unit: 9166 pass, 0 fail, 36 skip in 591 files. - Every PR in `v0.4.20..main` was pre-run for the owner's hands-on guide on isolated copies (own herdr session, fresh state, never the owner's live session). Items that need a real device (iPhone tap dictation #2370, dictation keyboards #2386, the installed desktop app's title-bar overlay #2381) are left for the owner's final check. This is a release-metadata PR: it changes no code path. Per the release steps, merge it after CI and the owner's go, then push `remote-v40`, wait for its bundles, and run Release with `0.4.21`.
Change
Fixes #2360.
The initial OmO/GJC status replay used bounded buffers but unbounded synchronous
work: it scanned the entire transcript, including every character in long tool
results, before returning to Bun's event loop. This prevented even bridge-health
requests from being served.
reader and a cooperative reader that yields after each 256 KiB chunk, including
inside one long JSONL record.
captured range has finished. Coalesce overlapping polls for the same pane.
obsolete read. Cancel obsolete work between chunks and before publication.
results; a replacement or truncation restarts the next read at byte zero.
changelog fragment.
Async contract and trade-off
poll()returns a completion promise, but a small read still reduces and publishesinline.
refresh()awaits outstanding replay for snapshot consumers. While alarge read is suspended, no intermediate historical RUN/INPUT/DONE transitions
are published. OmO retains its last published status. GJC preserves its stricter
readiness gate: it yields authority to herdr while the read is incomplete and
retells its own status when authority returns. A first idle replay still remains
a silent baseline, rather than a synthetic finish.
Pane replacement/removal invalidates that pane's tracked object;
stop()invalidates all reads. Renaming a different pane must not cancel unrelated work.
The read captures an upper byte offset rather than chasing an actively appended
file forever. Later appends are consumed by the next poll. GJC only treats the read
as complete when it also reaches the file size rechecked at publication.
The trade-off is that a status-dependent snapshot can still await replay, while
unrelated HTTP/WebSocket work is free to run. Publishing partial history or adding
a new public loading status would alter completion/notification semantics and
expand the wire contract; neither is necessary for this fix. Cooperative chunking
bounds parsing work, not the latency of an individual filesystem syscall.
Validation
The real
/api/health?scope=bridgeregression fails on untouched main: replay hasalready finished when health responds. It passes with this fix while replay is
still in progress. Fixtures use temporary state, synthetic transcripts and port 0,
and synchronize on replay events rather than fixed sleeps.
old unanswered question before a 16 MiB record, first-idle baselines, overlapping
polls, GJC authority while an append is pending, session changes, pane removal,
agent replacement, shutdown, file replacement, truncation, and unrelated panes.
bun run check fast: generated types/themes, typecheck and build pass.The broad unit suite has local failures also reproduced on an untouched-main
clone. Workflow syntax is skipped by the runner on macOS (its actionlint
download is Linux x64 only).
All five failures reproduce on untouched main: maximum-length Pi receipt,
launcher replacement ordering, malformed immediate framing on attached and
mirror paths, and wrapper launch identity. These are outside the changed code.
different versions. Full browser and native Windows lanes were not run locally;
the real HTTP regression exercises the responsiveness behavior changed here.
The matching failing unit files are
scripts/check.test.ts,scripts/self-hosted-runner.test.ts,server/session-history.test.ts,server/letta.test.ts,scripts/install-star.test.ts,server/voice.test.ts,server/remote-entry.test.ts,scripts/ci-browser.test.ts,scripts/ci-browser-parallel.test.ts,server/core/composer-framing.test.ts, andserver/bridge.test.ts. No failure is suppressed or changed in this PR.