Repository navigation
test(native-chat): cover paired runtime launch compatibility - #25072
Conversation
Main made journalItems a required outbox input; mirror production by passing the read state's items.
- Move the paired-runtime chat test into its own file and render the real session hook against a paired server that advertises today's capabilities while this machine advertises none, so Stop, prompt cancel, repeated Stop and question answers fail if their capability question goes to the wrong runtime. Add a local and a paired session sharing one id. - Cross-version: state that the file pins the capability strings a v1.4.219 server really advertises, use the desktop's real handshake list, and keep the current server only as the control for the refusal. - agent-launch-routing: give the released-server case a capable client so it is refused by the server gate, not the client one.
…e read - Start the cross-version job when the launch route or the client list the desktop sends a paired host changes, since the paired launch suite runs them. - Check the rail's conversation outline is read from the paired server, and narrow the hook test's header to what it covers.
Review summary (head
|
|
This run was cancelled 🛑 The workflow was cancelled before completion. Please check the link below for details. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change adds tests for paired-server structured-agent sessions, including RPC routing, cancellation, cleanup, and isolation from local sessions. It also expands the paths that trigger the cross-version wire job and adds tests for launch routing with legacy and current server capabilities. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is identified; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
1 participant
ELI5
#24205 lets you open Claude or Codex native chat on a workspace that lives on a paired Orca server (another machine running Orca that this desktop is connected to). That only works if the desktop sends every chat action to the server that owns the chat, and asks that server what it supports before using a newer request shape. Before this PR, nothing checked the second half: if a code change made the desktop ask its own machine "does the server support answering questions / stopping before a turn starts / ..." instead of asking the server, every test still passed. With a newer desktop paired to an older server, the desktop would then send requests the older server rejects, and answering a question or pressing Stop would quietly fail.
There is no user-visible change. This PR only adds and tightens tests so those mistakes fail CI.
What Changed
New
use-structured-agent-session-paired-runtime.test.tsx. It renders the real chat-session hook for a chat on a fake paired server. The fake server advertises today's capability list and this machine advertises none, so asking the wrong machine changes what gets sent. The test checks that:This moves the paired-chat case this PR first added to
use-structured-agent-session-transport.test.tsxinto the new file. That file is now unchanged from main.cross-version-paired-structured-launch.unit.test.ts(cross-version suite). It reads the capability names that releasev1.4.219really advertises, straight from that release's source. That is the newest release that has structured chat but predates the desktop choosing each launch's mode. The test checks:agent-launch-routing.test.ts(one line, existing test on main). The "released server keeps the terminal" case gave no client capabilities, so it was refused by the client check instead of the old-server check it describes. It now passes a capable client, so it really tests the old-server check.CI job routing (
config/scripts/pr-code-change-scope.mjs). The cross-version CI job runs only when a file it covers changes. It now also runs when the launch route or the client capability list the desktop sends a paired server changes, because the cross-version test runs that code. A newpr-code-change-scope-cross-version-launch-route.test.mjspins those paths.Only tests and CI routing change. Production code, the wire format, and mobile are untouched.
Why
The paired-server behavior in #24205 depends on two rules: send each call to the server that owns the session, and ask that same server what it supports. Before this PR, no native-chat test drove these hooks with a chat on a paired server, so neither rule was checked for paired chats. The launch tests also used only hand-written capability lists. I checked each new test by deliberately breaking the production code in a throwaway copy:
agent-launch-routing.test.ts; before the one-line fix, that file stayed green.Coverage I left where it already lives, instead of copying it into the cross-version file:
agent-launch-routing.test.ts;agent-launch-route-input.test.ts;agent-launch-routing.test.ts.The other version mix
An old desktop (v1.4.219) never routes a paired workspace to structured chat: its launch check refuses any host that isn't this machine. A current server with its own chat setting off refusing a client that leaves the launch mode to the server, as a released desktop does, is already covered by
cross-version-agent-session-wire.unit.test.ts("a client that leaves the launch mode to the host").Differences from the common pattern
v1.4.219rather than "newest release". The newest release now has client-chosen launch modes, so a rolling baseline would stop testing refusal of older servers. The tag can't change, and the first test asserts the premise, so a wrong pin fails loudly.Linked Issue
Follow-up to #24205 (maintainer PR; no new issue).
Visual Proof
N/A — tests only; no visual or interaction change.
Testing
What I verified:
vitest --config config/vitest.config.ts):pr-code-change-scope*.test.mjs): 70 tests;agent-launch-routing.test.ts: 17 tests;use-structured-agent-session-transport.test.tsx: 4 tests.CI on
7f9aa9a53a4(this branch with current main merged in): repository guards, test vs non-test line count, and the mobile web bundle pass. The static analysis and typecheck job fails before its typecheck step. The failure is the localization coverage audit flagging two strings insrc/renderer/src/lib/worktree-creation-hook-preparation.ts("setup", "vmRecipe"). That file came from main in #25472, whose own check failed the same way, and this PR doesn't touch it. Because that job failed, CI skipped the unit-test and cross-version jobs on this head. On the previous heade89ba285c0f, before the main merge, typecheck, cross-version, and relay passed. Unit-test shards 2–4 passed there; shards 1 and 5 failed only in two locale tests that main has since fixed, and both pass locally after this merge.What I didn't: no Orca/Electron/daemon launch, real agent run, live paired server, or Linux/Windows run. The release test reads published source and the hook test uses fakes; neither starts an app or an agent.
AI Disclosure
Review
Reviewed against the owning-runtime boundary and per-runtime capability negotiation. Passing tests and failing deliberate breaks show these tests guard those rules; they don't show the whole design is right.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
No new dependency, persisted field, process, RPC, or UI notice. SSH and WSL refusals stay covered by the existing shared route suite (
structured-native-chat-launch-route.test.ts). Noticed while writing the same-id test: unsent messages waiting in this machine's outbox are stored by session id alone, not by machine. Session ids are random, so I didn't change anything here; I'm noting it rather than testing it.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)