Repository navigation
refactor(hosts): host authorities and owner resolution - #27176
Conversation
Adds HostAuthority {endpoint, at} with authorityKey (existing host-id
spellings kept; nested 'SSH target behind server' gets an in-memory key no
host-id parser accepts), HostRoute {target, at}, and resolveOwner, a
focus-free owner resolver over the existing route resolvers. Today's
focus-reading answer is kept behind resolveOwnerWithLegacyFocus, which
counts every answer focus changed. No caller migrates in this PR.
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to Current users are unaffected, but this owner-resolution error should be fixed before callers migrate to the new path. The PR is mergeable with that follow-up tracked. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 30.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | |
The description is detailed and covers the required change summary, rationale, testing, visual proof, compatibility notes, and checklist. However, it explicitly states that no linked issue exists even… | Add an actual issue reference in the Linked Issue section, such as Fixes #12345. If the author is a maintainer and the repository permits the exception, state that the maintainer exemption applies. |
✅ Passed checks (3 passed)
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely identifies the main changes: host authorities and owner resolution.
Full details: Description check
-
Explanation
The description is detailed and covers the required change summary, rationale, testing, visual proof, compatibility notes, and checklist. However, it explicitly states that no linked issue exists even though the template requires an issue link unless the author is a maintainer.
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
-
-
📝 Generate docstrings 💡
-
-
- Commit to this branch
- Create a new PR
-
-
🛠️ Fix failing CI checks 💡
- 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.
…lection The active workspace's host now resolves through the strict path, so rival servers proxying one ssh:t stay ambiguous. A folder or repo row on a paired server that names an SSH connection resolves to that nested target instead of the server's own machine.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f8e7b1ad-2718-4be9-97a8-b1e76bbac00d
📒 Files selected for processing (3)
src/renderer/src/lib/folder-workspace-runtime-owner.tssrc/renderer/src/lib/resolve-owner.test.tssrc/renderer/src/lib/resolve-owner.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| const owner = folderWorkspace?.executionHostId | ||
| ? folderWorkspace | ||
| : findFolderProjectGroup(state, folderWorkspaceId, executionHostId) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
set -eu
printf '%s\n' '--- helper ---'
nl -ba src/renderer/src/lib/folder-workspace-runtime-owner.ts | sed -n '1,210p'
printf '%s\n' '--- resolver tests around folder routing ---'
nl -ba src/renderer/src/lib/resolve-owner.test.ts | sed -n '150,235p'
printf '%s\n' '--- helper references and exports ---'
rg -n -F --glob '*.ts' -- 'getNestedSshTargetIdForFolderWorkspace' src/renderer/src
rg -n -F --glob '*.ts' -- 'getExecutionHostIdForFolderWorkspace' src/renderer/srcRepository: stablyai/orca
Length of output: 13906
🏁 Script executed:
set -eu
printf '%s\n' '--- resolver call sites ---'
rg -n -F --glob '*.ts' -- 'resolveOwner(' src/renderer/src || test "$?" -eq 1
printf '%s\n' '--- nested target references ---'
rg -n -F --glob '*.ts' -- 'getNestedSshTargetIdForFolderWorkspace' src/renderer/src || test "$?" -eq 1
printf '%s\n' '--- folder fixtures and inherited cases ---'
rg -n -F --glob '*.ts' -- 'function folder' src/renderer/src/lib/resolve-owner.test.ts
rg -n -F --glob '*.ts' -- 'projectGroups:' src/renderer/src/lib/resolve-owner.test.ts
rg -n -F --glob '*.ts' -- 'connectionId' src/renderer/src/lib/folder-workspace-runtime-owner.test.ts src/renderer/src/lib/resolve-owner.test.ts || test "$?" -eq 1
printf '%s\n' '--- owner helper tests ---'
if test -f src/renderer/src/lib/folder-workspace-runtime-owner.test.ts; then
nl -ba src/renderer/src/lib/folder-workspace-runtime-owner.test.ts | sed -n '1,260p'
fiRepository: stablyai/orca
Length of output: 2164
🏁 Script executed:
set -eu
printf '%s\n' '--- activity production callers ---'
nl -ba src/renderer/src/components/activity/activity-event-builder-sources.ts | sed -n '1,115p'
printf '%s\n' '--- resolver contract and route construction ---'
nl -ba src/renderer/src/lib/resolve-owner.ts | sed -n '1,155p'
printf '%s\n' '--- activity caller imports and consumers ---'
rg -n -F --glob '*.ts' -- 'activity-event-builder-sources' src/renderer/src
rg -n -F --glob '*.ts' -- 'buildSource' src/renderer/src/components/activity src/renderer/src | head -80Repository: stablyai/orca
Length of output: 22492
🏁 Script executed:
set -eu
printf '%s\n' '--- activity builder wiring ---'
nl -ba src/renderer/src/components/activity/activity-event-builder.ts | sed -n '1,150p'
rg -n -F --glob '*.ts' -- 'from "../../lib/resolve-owner"' src/renderer/src || test "$?" -eq 1
rg -n -F --glob '*.ts' -- "from '@/lib/resolve-owner'" src/renderer/src || test "$?" -eq 1
rg -n -F --glob '*.ts' -- "from './lib/resolve-owner'" src/renderer/src || test "$?" -eq 1
rg -n -F --glob '*.ts' -- 'appendUnsupportedAndRetainedEvents(' src/renderer/srcRepository: stablyai/orca
Length of output: 7060
Preserve the folder connection for inherited runtime hosts.
When a folder has connectionId: 't' but no executionHostId, the helper selects the project group. If that group supplies runtime:env-a without a connection ID, the helper returns null. resolveOwner then keeps the server machine instead of selecting ssh:t.
This helper is part of a transitional path with no current production caller. The issue is a low-impact functional defect, not a current production data-integrity incident. Fix it before callers migrate to this path.
🐛 Suggested fix
return owner && parseExecutionHostId(owner.executionHostId)?.kind === 'runtime'
- ? owner.connectionId?.trim() || null
+ ? folderWorkspace?.connectionId?.trim() || owner.connectionId?.trim() || null
ELI5
Every project, workspace and terminal in Orca lives in exactly one place: this computer, an SSH host, or a paired Orca server (which can have SSH hosts of its own). Today the app's code has no single way to write down "SSH host t, reached through server E", and many places fall back to the "Active Server" setting (whatever server you last focused) when they cannot tell. This PR adds that missing vocabulary and one function that answers "who owns this workspace?" from the workspace's own rows, without ever looking at the Active Server setting. Nothing uses it yet; later PRs move features onto it one at a time.
What Changed
Users see no change. This is a foundation PR.
HostAuthority = { endpoint, at }(src/shared/host-authority.ts).endpointis this app or a paired server;atis that endpoint's own machine or one of its SSH targets.authorityKeywrites it with the host ids we already use (local,ssh:t,runtime:E). Only the nested case (SSH target behind a server) gets a new spelling,nested:E/t. That spelling exists only in memory, and every existing host-id parser rejects it, so a nested key that leaks somewhere fails closed instead of turning intoruntime:E. The unresolved-owner placeholder can never become an authority.HostRefis the "resource plus its owner" pair that later adapters will take.HostRoute = { target, at }(runtime-client-target.ts).atsits next toRuntimeClientTarget, not inside it. Fences and caches use the target as a key, and puttingatinside would merge(E, ssh:t)intoruntime:E.runtimeTargetForExecutionHostIdis extended instead of copied: it gives the same answers for every existing host id and now also routes a nested key to its server.resolveOwner(state, { workspaceId, hostId? })(lib/resolve-owner.ts) returnsresolved | missing | ambiguous(the existingExecutionHostOwnerMatch). It reuses the existing route resolvers (resolveWorktreeOperationRouteResultand the strict host-qualified resolver) and runs them with the focus setting blanked out. Focus is the only setting those resolvers read, so the result cannot depend on the Active Server.Two corrections on top of those resolvers. First, when the workspace is the active one, its selected host goes through the strict resolver, so two servers that both proxy the same SSH target give "ambiguous" and never a directly dialed SSH host. Second, when the route names only a paired server, the owning folder, project group or repo row can still name the SSH target behind that server (its
connectionId). In that case the result keeps the target, using the same rule asresolveWorktreeExecutionHost.resolveOwnerWithLegacyFocusisresolveOwnerwith today's fallback to the focused server added back, and counts each time focus changed the result. A later slice can delete its fallback once that count stays at zero.Why
The D4 owner-routing plan moves every renderer call that routes by focus over to routing by the resource's own owner. Those later slices need one owner type and one resolver first. Otherwise each slice would invent its own, or move its call sites twice.
Alternatives considered:
atonRuntimeClientTarget. Rejected, because fences and caches key on the target (see above).Prerequisite note: the design lists HBI A0a as a prerequisite. The only part this PR needs is the shared ambiguity-aware match type (
ExecutionHostOwnerMatch), and it is already on main insrc/shared/worktree-execution-host-resolution.ts. A0a's remaining work (moving the first-wins worktree lookups onto the index) is separate and stays in its own stack.Linked Issue
None. This is a refactor from the D4 owner-routing design and closes no issue.
Visual Proof
N/A. No caller uses the new code yet, so nothing visible or interactive changes.
Testing
resolve-owner.test.tsis a characterization table. Each row runs with Active Server set to {local, another server, a deleted server}. Rows: local (stamped and unstamped), direct SSH, a managed server on an SSH host, a paired server, an SSH target behind a server (with and without a named host, and known only from its repo row), the samessh:tbehind two servers (ambiguous with a named host, without one, and when it is the active workspace), a recipe-VM SSH target, the floating workspace, folders (local, paired server, an SSH target behind a paired server on the folder or on its group, unstamped) and an unknown id. Every row asserts thatresolveOwnergives the same answer under all three settings. It also pins today's answer, which reads focus, and checks the fallback counter. With the focus blanking removed, 4 cases fail.host-authority.test.tschecks that keys round-trip, that a nested key is rejected by the host-id and host-identity parsers, and that the unresolved placeholder and contradictory routes are refused.runtime-client-target.test.tschecks that plain host ids give the same answers as before and that a nested key routes to its server.pnpm tc, oxlint,check:code-quality:changed, and the focused tests for the existing callers ofruntimeTargetForExecutionHostId.I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)