Repository navigation
feat(chat): show Grok chat on macOS when Herdr's session is newest - #1858
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
| Layer / File(s) | Summary |
|---|---|
Process and session resolution server/grok-store.ts |
The resolver adds macOS process-environment and lsof evidence. It validates held session files and selects Herdr’s reported session only when its ID is unique and its timestamp is strictly newer than other held sessions. Linux retains /proc checks; other platforms return null. |
Resolver tests and CI coverage server/grok.test.ts, .github/workflows/ci.yml |
Tests cover macOS evidence parsing, session selection, environment handling, and failure cases. The macOS CI command now runs the Grok tests. |
Platform support documentation docs/grok-chat.md, docs/guide.md, README*.md, changelog.d/grok-macos-chat.md, scripts/grok-chat-browser-qa.ts |
Documentation describes macOS Grok chat support and its session conditions. Windows remains on Terminal. The browser QA skip message identifies its Linux /proc dependency. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature · Severity of issue fixed: Medium
Possibly related PRs
- devswha/ovis#1598: Provides
darwinProcess.environment, which the macOS Grok resolver uses to read the process environment.
Suggested reviewers: devswha, infowolfe, snowykr
Merge Risk: 
Minimal · up to 93b36
-
No actionable issue is established that would hold up the macOS Grok chat change; normal checks can proceed.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Description check
-
Explanation
The description explains the behavior, implementation, platform differences, linked issue, documentation changes, and validation results. It uses different headings from the template, but it provides equivalent and complete Change and Validation content.
Full details: Out of Scope Changes check
-
Explanation
The changed workflow entry, resolver tests, documentation, README updates, changelog entry, and QA skip-message update support macOS Grok chat, its fail-closed behavior, or its platform documentation. No unrelated functional change is demonstrated.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (8 skipped: 8 unsupported.)
-
Full details: Linked Issues check
-
Explanation
Issue
#1387requires fail-closed handling for incomplete macOSlsofevidence.grokLsofEvidencecreates a record for eachfline but silently omits records without a followingnline. For example, a truncated newer competitor record can be omitted, after whichreportedSessionPathcan treat the reported session as newest and bind Grok chat. The macOS binding, environment source, executable check, identity checks, and tests otherwise address the issue requirements.Resolution
Make
grokLsofEvidencereturnnullwhen anyfrecord is missing itsnfield or when the listing has another incomplete or malformed required record. Add a regression test where an incomplete competing hold cannot produce a chat binding.
-
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.
93b3639 to
8e9d7f6
Compare
|
Rebased onto current main
Verification:
Review prepared with a code-review agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
|
Merge pass gate review (Astra, gpt-6-astra) of 8e9d7f6: changes requested. The maintainer approved merging this once it passes review. It touches What holds:
Blocking:
Once these are fixed, the merge pass rechecks the new commits only. |
macOS reads lsof and ps for the binding rule already on main. Herdr's reported session must be held and strictly newer than every other held events file. Windows stays on the terminal view. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
…eams Both platforms now classify a held path through one heldSessionDir, so the deleted/renamed/three-part rules live in one place. resolveGrokStore takes a plain platform argument like claude-store, and tests spy on paneProcessInfo and Bun.spawn instead of an injected lookup. lsof and ps run through one hostOutput with the timeout and output cap, and the lsof parser is shorter. The held-file checks run before the store walk, which no longer happens when the evidence has already failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Snvz4RDRkDS4kpDRQqrDDC
The macOS home now comes from KERN_PROCARGS2 through claude-store's darwinProcess.environment, parsed by the same grokHomeFromEnvironment as Linux, so an argument or a value holding " GROK_HOME=" can no longer name the store and no ps child runs. The lsof cap counts bytes. The real-process resolver scenarios now run on macOS too, with the store named by the child's own GROK_HOME, so the Native macOS session identity job exercises real lsof output. The mocked macOS tests add a transcript in another directory than the hold, broken competing holds, a missing inode, an over-cap listing, a real timeout kill and an inode past 2^53; the "does not select" test no longer passes on mtime alone. README in all four languages now lists Grok chat on Linux and macOS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Snvz4RDRkDS4kpDRQqrDDC
8e9d7f6 to
55f09f6
Compare
|
Thank you for the macOS Grok support. I rebased the contributor commits onto current main and addressed the three latest merge-pass findings in
Verification:
There were no review threads to reply to or resolve. No workflow files were changed by these fixes. This delta review does not replace the maintainer's gate for the PR's existing workflow change. Fixes prepared by a maintainer agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
|
Merge pass gate re-check of 55f09f6 (Astra): still REJECT, on two narrow PID-reuse windows. The full report, with reproduction code, is Fixed:
Blocking:
Alternative: if the Claude macOS resolver ( |
|
Thank you again for the macOS support. Round two addresses both PID-reuse findings in
I checked the Claude precedent first: Regression evidence:
There are no review threads to resolve. No workflow files were changed in this delta. The PR's existing workflow change still goes through the maintainer's merge gate. Fixes prepared by a maintainer agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
|
Merge pass review-work report: PASSED. This outside-contributor PR changes session binding and
Final head:
Test results:
CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it. |
|
@devswha |
…a phone demo GIF (#1928) **The maintainer has asked for this to be merged** (2026-10-27). It no longer waits for his own review. ## What it does Makes the README short. The top now answers what Ovis is, shows it, and says how to install it on a computer and a phone, with the desktop browser, Mac, Windows, Linux, iPhone and Android given equal weight. The Korean, Japanese and Simplified Chinese copies have the same sections. - Logo, the tagline "Every coding agent, on every screen.", four badges (license, release, stars, herdr 0.9.0+). The PWA badge is dropped. - One sentence on what Ovis is: the agents on your computer as chats you answer with a click or a tap, in any browser, the Mac app or your phone. "Formerly herdr web UI" stays. - The hero is `docs/media/readme/hero.webp` (1200x675, 12 fps, 25 s, 1.5 MiB), linking to ovis.sh/demo. It is recorded on the demo build in the default Codex theme: a Claude Code turn on the desktop; the desktop and a phone side by side, where Codex's Allow command? card is answered on the phone and closes on both; then the release tab's shell as a live terminal. It replaces `desktop-demo.gif`. - **install**: on your computer, Ovis.dmg (Mac), Ovis.exe (Windows) and the curl one-liner (Linux), which add herdr and Bun when missing (no Node), with INSTALL.md for every other way; on your phone, the iPhone TestFlight and Android Google Play betas (each translation links its own language's beta page) or the browser added to the home screen, and the QR code to connect. - **what you get**: six short bullets, then one line each on the limits and on privacy, including that anonymous usage counts are on by default and how to turn them off. - **docs**, **thanks** (now also crediting @sanguneo for the icon and website design), **agent instructions**, **development**, **license**. ## What moved - The long install section goes to [INSTALL.md](INSTALL.md), which already covers every way to install step by step. - The FAQ goes to the user guide's [FAQ](docs/guide.md#faq), which already has it; the README's docs list links it. - The clip grid and the long video are dropped: their recordings show the previous UI. The `install` anchor stays, because ovis.sh/install redirects to `README.md#install`; the translations keep their explicit `<a id="install"></a>`. Nothing else in the repo or on ovis.sh links to a README anchor that is gone (`git grep`). ## Merge with main main's #1858 (Grok chat on macOS) edited the Grok lines in the README feature list and FAQ. Both of those sections leave the README in this PR, and the same change is already in docs/guide.md on main, so the conflict was resolved by keeping this branch's READMEs. ## Verification - `bun run check fast` passes on the merged head. - The README header test (`src/lib/markdownHtml.test.ts`) passes: logo width 100, language links, website link. Documentation only, so there is no changelog fragment. ## Follow-ups (not in this PR) - `ovis-site/llms.txt` describes the README as "features, install and FAQ"; the FAQ now lives in docs/guide.md. - `src/components/PhonePanel.tsx` links `devswha/ovis#on-your-phone` and `#access-and-safety`, anchors that live in docs/guide.md (this was already the case before this PR). - `scripts/film/capture.ts` sets no theme, so it records the old amber theme from stored settings, and its R5-terminal take no longer runs. --------- Co-authored-by: mp <mp@local> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Closes #1387.
Summary
Grok Chat now works on macOS with the binding rule already on main (#1352). Only the source of the evidence changes:
lsof -nPbw -a -p <pid> -FfDin, the readagy-store.tsmakes, with device and inode fields added. Only numeric descriptors count as holds. The firsttxtentry is the executable; macOS lists mapped files such as dyld as latertxtentries.sysctlKERN_PROCARGS2, throughclaude-store.ts's existingdarwinProcess.environment), parsed by the samegrokHomeFromEnvironmentas Linux's/proc/<pid>/environ. This replacesps -E: its space-joined line cannot tell an argument or a value containingGROK_HOME=from the environment, and it would be a second child per poll.Linux and macOS now share the held-path rules (
heldSessionDir) and the binding check (reportedSessionPath), so the two platforms cannot drift.How each point from #1387 is met
herdr:grok) chooses the session; a held file only confirms it. If the reported id's unique held directory is not the directory holding itsupdates.jsonl, the pane stays on Terminal.txt) must passisGrokExecutable.GROK_HOME, else itsHOME/.grok. A relative value fails, and the bridge's own home is never used.grokTranscriptPath). Duplicates fail closed, and symlinks are not followed.events.jsonlfiles. A tie fails closed.events.jsonl.old), replaced (different inode), or reported without a device or inode keeps the pane on Terminal. This holds for herdr's session and for every competing hold. An inode past 2^53 is treated as unknown.lsofruns as an awaitedBun.spawnchild with a 3 s timeout and a 1 MiB byte cap. Nothing usesspawnSync, and the environment read spawns no process at all.Workflow change
.github/workflows/ci.yml: the Native macOS session identity job's fixed test list now includes./server/grok.test.ts. On macOS, the real-process resolver scenarios already used on Linux run against reallsofand the real kernel environment. They copy bun asgrok, hold the descriptors, and name the store through the child'sGROK_HOME. Covered: older open sessions, an unreported/resume, duplicate ids, tied times, and unlinked, renamed or replaced files.Docs
docs/grok-chat.md: platform line and the macOS evidence sources.docs/guide.mdandREADME(en, ja, ko, zh-CN): Grok chat on Linux and macOS, Windows on Terminal.changelog.d/grok-macos-chat.md.Testing
bun test ./server/grok.test.tson macOS 26.5 (Apple silicon): 45 pass, 0 skip. The 8 real-process scenarios run natively.bun test ./server/claude-store.test.ts ./server/process-start.test.ts ./server/grok.test.ts: the macOS CI list passes.bun run typecheckand the conventions and changelog-fold tests pass.bun run check fastlocally: typecheck and build pass. 14 unit tests in unrelated files (agy,files,shell-agent,remote-entry,scripts/check,install-star,ci-browser) fail identically on an untouchedmaincheckout on this Mac, so they are environmental and not caused by this PR.