test(native-chat): a mirrored paired chat carries its host, so a workspace-id collision no longer hides its owner

[brennanb2025]

	Files	Added	Deleted	Net
Test	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​6	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​4
Prod	0	0	0	0

ELI5

main has one failing unit test. Two pull requests that each passed on their own were merged a few hours apart, and the second one's test still describes the app as it was before the first one landed. This PR updates that test to match what the app does now. Nothing in the app changes.

What Changed

The problem. On main, src/renderer/src/components/native-chat/StructuredAgentSessionAttentionBridge.test.tsx fails one test: "keeps a published paired chat subscribed and delivers after a workspace-id collision". It fails with expected 'runtime:env-1' to be undefined. Every pull request's unit-test job inherits that failure (seen first in #25063's CI).

The test checks one scenario. A chat runs on a paired Orca server and is shown on this desktop. Later, a workspace on this computer turns out to have the same workspace id. The desktop must keep listening for that chat's "turn finished" events, and the notification must name the paired server as its source.

Why it broke. Two merges conflict in meaning, though not in text:

• feat(native-chat): open structured chats on the paired Orca server that owns the workspace #24205 (8dc16a7df0b, merged 2026-10-03 01:53 PDT) makes every chat tab mirrored from a paired server carry the id of the host that runs it (executionHostId, here runtime:env-1). That way, closing, reloading or retrying the chat never has to guess the host from a workspace id that two hosts can share.
• Turn desktop notifications on or off per machine #24518 (752871bd678, merged 2026-10-03 12:51 PDT) added this test. Its branch was last updated before feat(native-chat): open structured chats on the paired Orca server that owns the workspace #24205 landed: its head 25b796fc4b0 does not contain 8dc16a7df0b. Before feat(native-chat): open structured chats on the paired Orca server that owns the workspace #24205, a mirrored chat tab carried no host id. The test therefore started by checking that the tab had none (executionHostId undefined), and after the collision it checked that the chat's owner could not be resolved (resolveNotificationTabOwner returns null).

Once both are on main, the mirrored tab carries runtime:env-1. The first check fails. With the host on the tab, the owner also resolves correctly to the paired server despite the collision, so the second check would fail too.

The fix (test only). Both checks now assert what the app does after #24205: the mirrored tab carries runtime:env-1, and after the collision its owner still resolves to the paired server ({ executionHostId: 'runtime:env-1', runtimeEnvironmentId: 'env-1' }). The parts of the test that protect behaviour are unchanged: the desktop keeps its single subscription to the paired server, does not unsubscribe, and delivers the completion with notificationSourceId: 'runtime:env-1' and the unread markers lit.

What users see: no change. Only a test file changes.

Why

The test's own preconditions are what is wrong, not the app. #24205 deliberately put the host on every mirrored chat tab, and says so in its PR description ("carried on … the provisional tab and every mirrored chat tab"). The precondition #24518 checked (a mirrored tab without a host) can no longer happen. The new precondition is stronger: a collision now cannot hide the chat's owner at all.

#24518's fallback is still covered elsewhere. When a tab's owner is ambiguous, the notification is named after the server whose event stream delivered the completion. structured-attention-dispatch.test.ts ("uses the receiving paired subscription over the tab's execution host") tests that directly, so dropping the "owner is unresolved" step here loses no coverage.

Considered and rejected:

• Strip the host id from the tab in the test to bring back the old "unresolved owner" path. That builds a state the app no longer produces.
• Delete the precondition checks. They are what make the test fail loudly if mirrored tabs ever stop carrying their host again.

Linked Issue

None. main is red.

Visual Proof

N/A. Test-only change, nothing rendered changes.

Testing

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

• Reproduced on origin/main (752871bd678): 1 failed, 14 passed in that file.

• At Turn desktop notifications on or off per machine #24518's own pre-merge head (25b796fc4b0, which lacks feat(native-chat): open structured chats on the paired Orca server that owns the workspace #24205): 15 passed. At Turn desktop notifications on or off per machine #24518's two merge parents the test does not exist yet (13 tests, all pass). This places the failure in the combination of Turn desktop notifications on or off per machine #24518 with feat(native-chat): open structured chats on the paired Orca server that owns the workspace #24205.

• With this change: StructuredAgentSessionAttentionBridge.test.tsx, structured-attention-dispatch.test.ts and notification-sources.test.ts give 53 passed.

• Ablation: the unchanged test file on origin/main fails again (1 failed, 14 passed).

• Web typecheck (tc:web), oxlint and oxfmt on the changed file, the changed-code quality gate and the React Doctor changed-files check all pass.

AI Disclosure

Review

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

No production code changes, so security, cross-platform, SSH, mobile, backwards compatibility and performance are unaffected.

Separate finding, not changed here: the bridge picks which host to listen to by looking up the tab's workspace id (getRuntimeEnvironmentIdForWorktree), not by the host the tab now carries. The test passes because the colliding workspace is the selected one, so the selected-workspace route still resolves to the paired server. When I made a different workspace the selected one in this same scenario, the bridge dropped the paired subscription. A completion from that chat would then be missed. Fixing that changes app behaviour, so it belongs in its own pull request.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[pullfrog]

✅ No new issues found.

Reviewed changes

• Mirrored chat carries its host (precondition) — tab.executionHostId now asserted as runtime:env-1 instead of undefined, matching buildMirroredAgentTabs (web-session-tabs-sync/terminal-surfaces.ts:126), which stamps every mirrored agent-session tab with the publishing host.
• Owner resolves despite the collision — resolveNotificationTabOwner now asserted as { executionHostId: 'runtime:env-1', runtimeEnvironmentId: 'env-1' } instead of null; with the host on the tab, the hostId-qualified lookup in attention/notification-subject-owner.ts:53 finds the runtime worktree and routeForOwner produces exactly that route.

The behavioural parts of the test are untouched: one paired subscription, no unsubscribe, and delivery with notificationSourceId: 'runtime:env-1' plus unread markers. The new assertions are exact, so the test still fails loudly if mirrored tabs ever stop carrying their host. The unresolved-owner fallback retains coverage in the sibling test at StructuredAgentSessionAttentionBridge.test.tsx:488.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: cbac8f83-e83c-4193-a5b6-959e7c636bd3

📥 Commits

Reviewing files that changed from the base of the PR and between 752871b and 4d8e0e6.

📒 Files selected for processing (1)

• src/renderer/src/components/native-chat/StructuredAgentSessionAttentionBridge.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The collision test now expects the published chat tab to retain execution host runtime:env-1. After a worktree ID collision, it expects notification ownership to resolve to execution host runtime:env-1 and runtime environment env-1, rather than null.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 4d8e0

This test-only update is mergeable with minimal risk.

Architecture Summary

Architecture risk: 🔵 Low · up to 4d8e0

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in src/renderer/src/components/native-chat/StructuredAgentSessionAttentionBridge.test.tsx: The test now expects the published chat tab to carry runtime:env-1 as its execution host; it previously expected that field to be undefined.
• observed — Modified behavior in src/renderer/src/components/native-chat/StructuredAgentSessionAttentionBridge.test.tsx: After the worktree ID collision, the test now expects notification ownership to resolve to execution host runtime:env-1 and runtime environment env-1, rather than null.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title identifies this as a native-chat test change and describes the updated expectation after a workspace-ID collision. It is specific, though longer than necessary.
Description check	✅ Passed	The description clearly explains the problem, test-only change, rationale, and reported test results. It includes visual proof marked N/A and completes most template sections. The linked issue is list…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

• Autopilot · 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[brennanb2025]

Superseded by af13da5 on main, which fixed this test a different way. It keeps the published tab's host stamp check, then strips the stamp on purpose to model a legacy unstamped tab, so it still exercises the unresolved-owner path. Closing this PR so the two fixes don't overlap. The related subscription change is in #25081, which is being rechecked against that test.