Skip to content

fix(editor): keep file ownership through host focus changes - #25429

Open
nwparker wants to merge 5 commits into
mainfrom
nwparker/editor-file-host-focus
Open

nwparker wants to merge 5 commits into
mainfrom
nwparker/editor-file-host-focus

Conversation

@nwparker

@nwparker nwparker commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​555 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​555
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​43 $\color{#cf222e}{\Huge{\mathbf{−}}}$​19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​24

ELI5

A local file could be opened through the selected peer and fail to load. Editable tabs could also be labeled with a different host from the file they contained. The file and tab now keep the owner captured when the file opens.

What Changed

Editor ownership ignores workspace focus when resolving older owner records, including when checking an already-open folder file. Editable tabs reuse the existing file-host lookup. The legacy single-runtime fallback and generic workspace routing are preserved.

Why

This fixes file routing without changing how terminals and other workspace operations select a host. It reuses the existing routing and save queue, with no dependency or wire changes.

This adopts the three authored commits from @Pr1p in #23349, then adds folder revalidation, shared file-host lookup, and delayed read/save controls.

Linked Issue

Relates to #21121. Supersedes #23349; this fixes the host-focus portion only.

Visual Proof

Controlled selected-peer metadata with actual local file reads and writes:

orca-file-host-focus-before.png

orca-file-host-focus-after.png

orca-file-host-focus-saved.png

The baseline sends the local read to the selected peer and fails. The corrected editor loads the file and saves exactly LOCAL_CONTENT_SAVED_ON_LOCAL to the local path.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated

Main fails four ownership controls; the contributor version additionally fails the folder revalidation control. Final focused tests: 114 passed across 8 suites, including local, runtime, and SSH routes while reads and saves wait for completion, legacy routing, restored drafts, and model ownership.

Full typecheck, full lint including anti-slop, changed-code checks, and fresh build pass. Hidden Electron: baseline fails at the wrong-host read; corrected flow passes 3/3 repetitions. E2E-wide typecheck has pre-existing errors and no errors in the added spec.

Validated on macOS. SSH and runtime routing have automated transport controls; live remote sessions and native Windows/Linux were not run for this patch.

AI Disclosure

Review

Independent production review found no blocker; draft pending final review and CI.

Agent skill upstream boundary

  • Not applicable

Notes

Only editor-file ownership changes. The single-runtime fallback for older host publications remains supported.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Before/after visual proof attached
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • Local typecheck, lint, focused tests, and build pass

Shared CI repair

This branch includes the three-identifier repair from #25494, which removes two unchanged localization-scanner false positives. The editor patches and contributor authors are preserved; every editor commit patch was compared across the rebase. Fresh affected regression suites, changed-code quality and localization coverage pass on this head. The original rendered proof still applies to the unchanged editor patches. Current-head full CI remains the final gate.

@nwparker
nwparker marked this pull request as ready for review October 5, 2026 06:24
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e99f368b-50fa-47df-b835-9c6bc7080bb2
📥 Commits

Reviewing files that changed from the base of the PR and between 1569e06 and 64a234a.

📒 Files selected for processing (9)
  • src/renderer/src/components/editor/editor-file-host-focus.operations.test.tsx
  • src/renderer/src/lib/editor-file-operation-owner.test.ts
  • src/renderer/src/lib/editor-file-operation-owner.ts
  • src/renderer/src/lib/editor-file-owner-focus.test.ts
  • src/renderer/src/lib/unified-tab-host-ownership.ts
  • src/renderer/src/lib/worktree-creation-hook-preparation.ts
  • src/renderer/src/store/slices/editor-tab-placement.test.ts
  • src/renderer/src/store/slices/editor/tabs/workspace-editor-item.ts
  • tests/e2e/editor-file-host-focus.spec.ts
  • 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

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

@pullfrog pullfrog 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.

Important

resolveLegacyEditorFileRoute discards the active host unconditionally, which regresses folder workspaces whose id is published on more than one host — for those, the active selection is the only disambiguator, and editor capture now fails closed. One minor tab-stamping concern is inline as well.

Reviewed changes

  • Legacy editor route ignores focus — captureEditorFileOperationProvenance and the folder revalidation branch of resolveCurrentEditorRoute now resolve through a new wrapper that nulls activeWorktreeId / activeWorkspaceExecutionHostId before resolveWorktreeOperationRoute, so resolveActiveWorkspaceRoute no longer participates.
  • Editor tabs stamped with the file owner — openWorkspaceEditorItem sets executionHostId from getOpenFileExecutionHostId(file) for new editor tabs instead of inheriting the active host.
  • Shared owner lookup — getOpenFileExecutionHostId extracted into unified-tab-host-ownership.ts and reused by isOpenFileOwnedByWorktree (the refactor is behavior-preserving).
  • Tests — capture/revalidation units, tab-ownership units, a delayed read/save operations test for local/runtime/SSH hosts, and an e2e spec that drives the real local filesystem.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment on lines +83 to +96
function resolveLegacyEditorFileRoute(
state: EditorOwnerState,
worktreeId: string
): WorktreeOperationRoute | null {
// Why: a focused workspace selects where the UI is looking, not which host owns a file.
return resolveWorktreeOperationRoute(
{
...state,
activeWorktreeId: null,
activeWorkspaceExecutionHostId: null
},
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.

Nulling the active host here also removes the only disambiguator for a folder workspace id that exists on more than one host, so editor capture for such a folder now throws instead of resolving.

Technical details
# Ambiguous folder ids lose their only tie-breaker

Today a folder workspace keyed `folder:<uuid>` can carry two rows on different hosts (local + a runtime/SSH peer); this is supported and exercised by `src/renderer/src/store/slices/selected-host-active-workspace-identity.test.ts:114` and the collision contracts in `folder-workspace-owner-routed-mutations.test.ts`. For a folder key `resolveExplicitWorktreeOperationRouteResult` is always `missing` (it only scans worktrees/repos), so the legacy branch is the only resolver.

## Affected sites
- `editor-file-operation-owner.ts:83-96` — `resolveLegacyEditorFileRoute` sets `activeWorktreeId`/`activeWorkspaceExecutionHostId` to `null`.
- `editor-file-operation-owner.ts:71` — `captureEditorFileOperationProvenance` uses it for the no-hint branch; a `null` route throws `OWNER_CHANGED_MESSAGE` at `:72`.
- `editor-file-operation-owner.ts:135` — `resolveCurrentEditorRoute` uses it for the folder revalidation branch.
- `worktree-operation-route.ts:303` — `resolveFolderWorkspaceOperationRoute` calls `findFolderWorkspaceOwner(state, folderWorkspaceId)` with no host.
- `worktree-runtime-owner-index.ts:78-83,295-299` — with no host the bare id maps to `{ kind: 'ambiguous' }`, which `findIndexedFolderWorkspaceOwner` returns as `null`.

## Required outcome
- An editor file opened in a folder workspace whose id is published on multiple hosts must still resolve to the host the user selected (the pre-PR behavior via `resolveActiveWorkspaceRoute` / `resolveSelectedHostRoute`), rather than throwing on open and on every subsequent read/save.

## Suggested approach (optional)
- Preserve the active selection as a tie-breaker only when the folder id is genuinely ambiguous (e.g. resolve the folder owner for the active host before falling back to a host-blind lookup), or scope the focus-nulling to git-worktree ids where owner rows carry the host. A test mirroring `selected-host-active-workspace-identity.test.ts` at the editor-capture level would pin this.

## Open questions for the human (optional)
- Is a cross-host folder id a state you expect to persist beyond the catalog-hydration window? If not, is failing closed on open the intended tradeoff?

entityId: fileId,
label,
isPreview,
...(file ? { executionHostId: getOpenFileExecutionHostId(file) } : {}),

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.

Stamping with getOpenFileExecutionHostId(file) forces local for editor files that carry no host evidence, which no longer aliases a remote worktree and drops the tab from the workspace-tab palette and open-tab search. This is most reachable via the read-only View Log path (ai-vault-session-log-open.ts), where provenance capture is skipped (readOnly: true) and runtimeEnvironmentId is null while worktreeId is the active worktree — which can be remote.

Technical details
# Forced local stamp filters no-evidence editor tabs

## Affected sites
- `workspace-editor-item.ts:49` — editor tabs now always pass an explicit `executionHostId`.
- `unified-tab-host-ownership.ts:81-90` — `getOpenFileExecutionHostId` ends in `?? LOCAL_EXECUTION_HOST_ID`.
- `unified-tab-host-ownership.ts:62-65` — with a host present, `isUnifiedTabOwnedByWorktree` never reaches the `!ambiguousWorktreeIds` fallback and requires an exact/alias host match.
- `workspace-tab-palette-entry-builder.ts:181` — the tab gate rejects `local` against a remote worktree before the file-level gate at `:272-282` is consulted.

## Required outcome
- Editor tabs for files that carry no owner evidence should keep their prior visibility (the worktree's active host, or the ambiguity fallback), rather than being pinned `local` and filtered out.

## Suggested approach (optional)
- Pass the owner only when there is evidence, mirroring the file-side gate: `...(file && hasOpenFileExecutionHostEvidence(file) ? { executionHostId: getOpenFileExecutionHostId(file) } : {})`.

@nwparker
nwparker force-pushed the nwparker/editor-file-host-focus branch from 02f1794 to 04e8da1 Compare October 5, 2026 09:34
@nwparker
nwparker force-pushed the nwparker/editor-file-host-focus branch from 04e8da1 to 64a234a Compare October 5, 2026 10:50

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants