Skip to content

fix(worktrees): resolve a two-host project by the worktree's own host (#10634) - #10986

Merged
OrcaWin merged 2 commits into
mainfrom
fix-ambiguous-host-identity-10634
Jul 27, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
fix-ambiguous-host-identity-10634

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Fixes #10634.

What users hit

Register the same project on two hosts — a local folder and an SSH checkout of the same repo — and every workspace in that project breaks permanently:

Workspace identity is ambiguous across hosts. Refresh projects and try again.

"Refresh projects" can never fix it. Nothing is stale; both host setups are valid and intentional. It survives app restarts. And because it was thrown from a background notification path, it surfaced as an uncaught renderer error rather than a handled state.

The ambiguity was manufactured

resolveExactWorktreeRoute starts from a worktree that already carries exactly one hostId — then discards it and asks resolveIndexedRepoOperationRoute which host owns the repo. That question genuinely has two answers once a project spans hosts.

But only the project spans hosts. Every individual worktree names exactly one. The information needed to answer was in hand and thrown away.

Route resolution now filters repo setups to those matching the worktree's own host (checking both getRepoExecutionHostId and the repo's connectionId) before looking for a transport. A two-host project resolves as cleanly as a one-host project. Genuine ambiguity — two setups on the same host — still returns ambiguous.

Second half: it should never have been uncaught

Passive background paths — unread marking, cleared-unread, activity bumps — called a helper that threw. trySettingsForWorktreeOwner now returns null; those paths skip the persist and keep local state consistent. Explicit user actions (remove, force-delete) still throw and surface the error, which is correct — the user asked for something Orca can't safely do.

bumpWorktreeActivity warns once per workspace, not per event: activity bumps fire on every PTY event, so an unbounded warn would flood the console for exactly the users already hitting this bug.

Verification

Adversarial review (4 lenses, 3 independent refuters per finding) confirmed the routing logic sound and found 3 in-scope gaps, all fixed in the second commit:

  • only markWorktreeUnread had an ambiguous-owner test, so restoring the throw in clearWorktreeUnread or bumpWorktreeActivity would have reproduced the uncaught error with the suite green
  • bumpWorktreeActivity skipped silently where its siblings warned
  • no test pinned the warn-once contract

Every guard mutation-tested — reverted individually, mutation confirmed applied, specific tests confirmed failing:

Mutation Result
revert to resolveIndexedRepoOperationRoute #10634 routing test fails
restore the throw in the passive path #10634 error-handling test fails
restore the throw in clearWorktreeUnread only new all-passive-paths test fails
warn on every event instead of once warn-once test fails

pnpm typecheck clean across all three projects. oxlint exits 0. 5,077 renderer store + lib tests pass. max-lines ratchet and reliability gates pass.

Scope

Renderer-only; no main-process or IPC changes. No new user-facing strings, so no locale work. No UI or layout changes — the visible difference is that a two-host project stops erroring. Folder workspaces keep their existing early path. Provider-agnostic: nothing here assumes GitHub.

Not a duplicate of #10809 — that fixes linking a folder to a project (#10620, main process, identity probing). This fixes operating on a worktree once two valid setups already exist. Zero file overlap.

Not verified

The reporter's exact environment (a real SSH host plus a local clone of the same repo) wasn't reproduced end to end; the two-host state is driven directly in tests. The root cause was traced in current source rather than inferred.

OrcaWin added 2 commits July 27, 2026 11:53
…#10634)

A project registered on both a local host and an SSH host permanently poisoned
every one of its workspaces with "Workspace identity is ambiguous across hosts.
Refresh projects and try again." Refresh could never help: nothing was stale,
both host setups were valid and intentional. The error survived restarts.

The ambiguity was manufactured. `resolveExactWorktreeRoute` starts from a
worktree that already carries exactly one `hostId`, then throws that away and
asks `resolveIndexedRepoOperationRoute` which host owns the *repo* — a question
with two right answers once a project spans hosts. Only the project spans hosts;
each worktree never does.

Route resolution now filters repo setups to the ones matching the worktree's own
host before looking for a transport, so a two-host project resolves as cleanly as
a one-host project. Genuine ambiguity still returns `ambiguous`.

Second half: the error escaped as an *uncaught renderer error* because passive
background paths — unread marking, activity bumps — called a helper that threw.
Those callers now degrade: `trySettingsForWorktreeOwner` returns null, the
passive update is skipped with a warning, and local state stays consistent.
Explicit user actions still surface the error.
…owners

Adversarial review found the routing fix sound but its coverage thin: only
markWorktreeUnread had an ambiguous-owner test, so restoring the throw in
clearWorktreeUnread or bumpWorktreeActivity would have reproduced the uncaught
renderer error with the suite still green. Both are now covered, verified by
mutation.

bumpWorktreeActivity also skipped silently where the other paths warned. It now
warns — but once per workspace, not per event: activity bumps fire on every PTY
event, so an unbounded warn would flood the console for exactly the users already
hitting this bug.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

SSH worktree route resolution now matches repository execution or connection hosts against the owner’s SSH host and reports resolved, ambiguous, or missing routes. Passive metadata operations use centralized best-effort persistence that handles ambiguous ownership, deduplicates warnings, and refetches worktrees after persistence failures. Tests cover SSH routing, multi-host unread persistence, ambiguous ownership, passive unread operations, and warning deduplication.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description lacks required template sections, including Screenshots, Testing, AI Review Report, Security Audit, and Notes. Add the full PR template sections with concrete content for Summary, Screenshots, Testing, AI Review Report, Security Audit, and Notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: resolving worktree routing by the worktree's host for a two-host project.
Linked Issues check ✅ Passed The changes address #10634 by resolving routes from the worktree host and handling passive failures without uncaught renderer errors.
Out of Scope Changes check ✅ Passed The changes stay focused on worktree routing, passive persistence, and tests, with no obvious unrelated or extraneous code.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d3046ffa-e4af-4b61-89b4-da19a42dca85

📥 Commits

Reviewing files that changed from the base of the PR and between 05603a2 and f1bd584.

📒 Files selected for processing (4)
  • src/renderer/src/lib/worktree-operation-route.test.ts
  • src/renderer/src/lib/worktree-operation-route.ts
  • src/renderer/src/store/slices/worktrees.test.ts
  • src/renderer/src/store/slices/worktrees.ts

Comment on lines +4405 to +4410
const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId)
if (!ownerSettings) {
warnAmbiguousOwnerOnce(worktreeId, 'persist worktree activity timestamp')
return
}
void persistWorktreeMeta(ownerSettings, worktreeId, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use the passive persistence helper for activity updates.

This path returns on selector_not_found without refetching, unlike persistPassiveWorktreeMetaForOwner; activity metadata can remain stale after the runtime no longer recognizes the worktree.

Proposed fix
-    const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId)
-    if (!ownerSettings) {
-      warnAmbiguousOwnerOnce(worktreeId, 'persist worktree activity timestamp')
-      return
-    }
-    void persistWorktreeMeta(ownerSettings, worktreeId, {
-      lastActivityAt: now
-    }).catch((err) => {
-      if (isRuntimeSelectorNotFoundError(err)) {
-        return
-      }
-      console.error('Failed to persist worktree activity timestamp:', err)
-      void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId))
-    })
+    persistPassiveWorktreeMetaForOwner(
+      get,
+      worktreeId,
+      { lastActivityAt: now },
+      'persist worktree activity timestamp'
+    )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId)
if (!ownerSettings) {
warnAmbiguousOwnerOnce(worktreeId, 'persist worktree activity timestamp')
return
}
void persistWorktreeMeta(ownerSettings, worktreeId, {
persistPassiveWorktreeMetaForOwner(
get,
worktreeId,
{ lastActivityAt: now },
'persist worktree activity timestamp'
)

@OrcaWin
OrcaWin merged commit c140a51 into main Jul 27, 2026
2 checks passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…stablyai#10634) (stablyai#10986)

* fix(worktrees): resolve a two-host project by the worktree's own host (stablyai#10634)

A project registered on both a local host and an SSH host permanently poisoned
every one of its workspaces with "Workspace identity is ambiguous across hosts.
Refresh projects and try again." Refresh could never help: nothing was stale,
both host setups were valid and intentional. The error survived restarts.

The ambiguity was manufactured. `resolveExactWorktreeRoute` starts from a
worktree that already carries exactly one `hostId`, then throws that away and
asks `resolveIndexedRepoOperationRoute` which host owns the *repo* — a question
with two right answers once a project spans hosts. Only the project spans hosts;
each worktree never does.

Route resolution now filters repo setups to the ones matching the worktree's own
host before looking for a transport, so a two-host project resolves as cleanly as
a one-host project. Genuine ambiguity still returns `ambiguous`.

Second half: the error escaped as an *uncaught renderer error* because passive
background paths — unread marking, activity bumps — called a helper that threw.
Those callers now degrade: `trySettingsForWorktreeOwner` returns null, the
passive update is skipped with a warning, and local state stays consistent.
Explicit user actions still surface the error.

* fix(worktrees): cover every passive path and warn once for ambiguous owners

Adversarial review found the routing fix sound but its coverage thin: only
markWorktreeUnread had an ambiguous-owner test, so restoring the throw in
clearWorktreeUnread or bumpWorktreeActivity would have reproduced the uncaught
renderer error with the suite still green. Both are now covered, verified by
mutation.

bumpWorktreeActivity also skipped silently where the other paths warned. It now
warns — but once per workspace, not per event: activity bumps fire on every PTY
event, so an unbounded warn would flood the console for exactly the users already
hitting this bug.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant