Repository navigation
Conversation
…nt does herdr keeps DONE until one of its own clients shows the pane, so a finish read only in the web UI kept its dot until someone visited the pane in the TUI. Bring back the per-browser seen record #1120 removed with Quiet opened finishes, without the setting: the pane on screen while the page is visible is recorded at herdr's state_change_seq, and a DONE still at that counter draws as ready in the Agents list and the workspace roll-up. Its next finish moves the counter and draws the dot again. The first-use seed that started the old setting quiet is gone: a PC with no record starts with nothing opened, so no finish is hidden that was never opened here. Restart handling, stand-in carry and storage rules are the ones the setting had. Nothing calls pane.focus. Fixes #1279 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 8 files. (3 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | |
The description includes the required Change and Validation sections and provides detailed test results. However, it describes a server-issued finish ID and finish epoch implementation that conflicts … | Rewrite the description to match the actual changeset. Describe per-browser state_change_seq tracking, visibility-based acknowledgement, localStorage persistence, restart and cleanup handling, and the tests that validate those changes. Remo… |
-
Check name Status Explanation Linked Issues check Issue #1279requires a per-browser seen record for a finished pane, matchingstate_change_seq, with READY display until a later finish.sidebarOrder.tsadds exact-sequence seen checks, counter car…Out of Scope Changes check The source changes implement issue #1279directly. The tests, regression scripts, DESIGN.md, development documentation, and changelog document or verify the same browser-local seen behavior. No unrela…Title check The title clearly identifies the main change: a finish opened in the sidebar is displayed as ready. It is concise and related to the pull request objectives. Full details: Linked Issues check
-
Explanation
Issue
#1279requires a per-browser seen record for a finished pane, matchingstate_change_seq, with READY display until a later finish.sidebarOrder.tsadds exact-sequence seen checks, counter carry across stand-ins, restart clearing, and browser storage.sidebarActivity.tsxrecords the selected pane only while the page is visible.MachineSidebar.tsxsupplies the selection context to both sidebar views. The changed regression scripts andsidebarOrder.test.tscover READY display, reloads, later finishes, ordering, stand-ins, restarts, and persistence.
Full details: Out of Scope Changes check
-
Explanation
The source changes implement issue
#1279directly. The tests, regression scripts, DESIGN.md, development documentation, and changelog document or verify the same browser-local seen behavior. No unrelated product behavior or unrelated source area is shown in the reviewed change summary.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 8 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Description check
-
Explanation
The description includes the required Change and Validation sections and provides detailed test results. However, it describes a server-issued finish ID and finish epoch implementation that conflicts with the summarized changes, which implement browser-local tracking using state_change_seq.
Resolution
Rewrite the description to match the actual changeset. Describe per-browser state_change_seq tracking, visibility-based acknowledgement, localStorage persistence, restart and cleanup handling, and the tests that validate those changes. Remove unsupported claims about server-issued finish IDs, finish epochs, bridge changes, and related server-side validation.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- 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.
|
Ran the browser checks on a MacBook Air (macOS 27.0.1, arm64, Chrome, its own isolated test herdr session; fresh clones of this branch at 135cb15 and of main at b5cdbdb):
|
Keep finishes unread behind the phone drawer or a zoomed pane, and drop ambiguous stand-in carry across counter advances and connection gaps. Invalidate loaded records on proven restarts and merge marks from other browser tabs. Cover the review cases with real-hook and demo regressions, including persistent raw DONE on reload.
A chat scrolled back to older turns, or showing New messages, no longer reports its content as presented, so the answer a finish lands at the end is not acknowledged until the reader returns to it. Adds a sidebar demo regression that fails without the gate.
This branch has not been deployed
1 participant
Change
Fixes #1279.
Replace the browser's
state_change_seqacknowledgement heuristic with server-issued finish identities.finish_idwhen its final client-visible status transitions into DONE, after the completion tracker, OmO, GJC and launched-agent overrides. Authoritative recovery observations feed the tracker before an asynchronous refresh, preserving the resync'snewerexclusions. Snapshots andpane-statusevents carry the optional fields; local machine rosters and remote relays preserve them.finish_epochidentifies the bridge incarnation. It changes on web server startup and on observed replacement of herdr's socket identity (Windows: itspid:startmarker). Failed reads and reconnects alone do not prove a restart. The browser never derives finish epochs from counter rollback; the legacy ordering-only reset remains separate.(PC, epoch, pane), synchronize tabs through storage events, remove the oldherdr-web-ui:seen:<pc>records, and cap retained keys at 2,000, oldest first. A tab's roster never prunes another tab's pane records.This follows herdr's own opened-DONE behavior and the Codex app's unread completion dot, with intentionally browser-local rather than cross-device read state. There is no new setting.
Why this is safe
state_change_seqis unchanged. A WORKING explicitly observed during recovery also reaches the finish tracker, so a DONE overtaking the refresh cannot reuse the preceding finish's ID. Panes that became newer or ended during the resync read are excluded.pane.focusor other herdr call was added. A transition the server never observes cannot be counted. Socket identity evidence, not a failed connection or PID alone, governs herdr restart detection.Merged
origin/maininto the existing branch first, without a rebase or force push.Validation
bun run typecheckandbun run check fast: passed. Final run: 4,522 passed, 0 failed, 8 existing skips across 348 files, including the direct CSS-selector regression tests.newerexclusions separately failed the ended-pane regression (2 instead of 1). Browser cases against the pre-fix build reproduced acknowledgement under a crashed pane, loading/failed/refreshing conversation, held or absent terminal attachment, slash suggestions and failed file suggestions; all eight now pass. The duplicate no-op ordering assertion was removed and the meaningful rollback regression added; no prose or test-source meta-test was introduced.bun run check run bun test --timeout 15000 server/api.contract.test.ts server/collector.contract.test.ts server/pending-input.contract.test.ts: passed, 201 tests. HTTP snapshots and WS status frames agree on finish IDs and epoch.bun run build: passed.bun scripts/sidebar-activity-demo-regression.ts: passed, including crashed/loading/failed/refreshing content, held and absent terminal attachments, both phone composer popup variants, phone drawer, full-screen file viewer while the selected pane finishes, zoom redirect, reload with an unread control, storage-loss control, Activity order and pins.bun scripts/droplet-demo-regression.ts: passed, including server finish acknowledgement, portrait/landscape, keyboard geometry and opening a finished pane.bun run check run --build bun scripts/gjc-status-browser-regression.ts: passed, all 6 native-status scenarios on an isolated test session, with current-source build provenance.bun scripts/release-guards.ts --base origin/main: passed: this is not a release.REMOTE_BUNDLE_VERSIONstays at31; server/shared changes need a newly published remote bundle in the release that ships them. Until then, older bridges keep their dots rather than guessing acknowledgements.All herdr-backed checks used isolated test sessions and disposable state. No merge or GitHub comment was made. Native Windows execution and an independent reviewer were not run in this task; Windows marker identity is covered with fixtures. The session's nested-agent limit prevented a separate reviewer, so the implementation review and screenshot inspection were performed directly.