Skip to content

refactor(renderer): fold four lib callers off the deprecated runtime-owner resolver - #30458

Merged
nwparker merged 1 commit into
mainfrom
nwparker/r3-fold-f1-lib-read
Oct 19, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/r3-fold-f1-lib-read

Conversation

@nwparker

@nwparker nwparker commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​37 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​34
Prod 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​39 $\color{#cf222e}{\Huge{\mathbf{−}}}$​21 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​18

ELI5

Four places in Orca asked the old "which machine owns this workspace?" question. They now ask the new one. For almost every workspace the answer is identical. Two deliberate differences:

  • A project that exists on two machines at once is no longer quietly treated as this computer's, so "Open in" refuses it rather than opening the wrong checkout.
  • An SSH workspace that only a paired Orca server can reach is now reached through that server.

What Changed

The callers now read the owner through resolveOwnerWithLegacyFallback (from #30451), via a new runtimeEnvironmentIdWithLegacyFallbackUnlessTwin:

  • lib/worktree-row-open-in-owner.ts (sidebar "Open in", for rows without a host). A twin is now ownerUnresolved; it used to launch as this desktop's checkout. A row with no placing owner reads as this desktop, as before.
  • lib/execution-host-facts.ts: the server proxying an SSH host's platform facts.
  • lib/doc-preview-grants.ts: a twin still gets no grant.
  • lib/worktree-activation-pty-inventory.ts: a twin still reads as not server-owned.

Other details:

  • Deprecated importers: 57 → 53.
  • The focus-read ratchet is pruned for the four files.
  • client-creation-action-policy.ts stays on the deprecated resolver until browser creation (browser-tab-actions.ts) folds with it. Codex showed that moving one without the other makes a server-proxied SSH row offer a browser tab that creation then refuses.

Why

This is batch F1 of the SSH-hardening round 3 design (§6). It uses the read-only lib callers, the lowest-risk tier. Each caller's twin decision is written in its diff.

The other deliberate change is C (§4): a worktree row that lost its server stamp, whose only repo row is a server proxying its SSH target. It now routes through that server, which matches where terminals for that row already spawn.

Linked Issue

None (internal hardening; maintainer PR). Stacked on #30451. Its base is that branch, and it retargets to main when #30451 merges.

Visual Proof

N/A: no UI change beyond Open-in refusing a twin, which is the existing unresolved-owner state.

Testing

  • New behaviour tests in worktree-row-open-in-owner.test.ts. Both fail on main:

    • a git twin is unresolved;
    • a server-proxied row routes through hub-env.
  • doc-preview-grants.test.ts gains a twin case.

  • Every suite that mocks worktree-runtime-owner (73 files) passes. The mocks on the folded paths name the new helper.

  • Unit tests, single worker: src/renderer/src/lib, components, runtime and store (4,651 files, before the clean-code tidy), plus the 78 affected suites after it.

  • Full local pr.yml static job: 0 failures.

  • The managed-orcad Docker lane is dispatched on this head; the link is added when green.

  • Codex review (gpt-6.1-sol, high):

  • I manually tested these changes locally

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

…owner resolver

Open-in owner, execution-host platform facts, doc-preview grants and the activation PTY inventory
read the owner through resolveOwnerWithLegacyFallback. Open in now refuses a twin instead of
launching it as this desktop's checkout; the others keep reading a twin as not server-owned. A row
whose only repo row is a server proxying its SSH target routes through that server. The
creation-action policy stays until browser creation folds with it. Deprecated importers 57 -> 53.
@nwparker
nwparker force-pushed the nwparker/r3-fold-f1-lib-read branch from cf01012 to 0aad21a Compare October 19, 2026 00:47
@nwparker
nwparker changed the base branch from nwparker/r3-owner-legacy-adapter to main October 19, 2026 00:47
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: d95acef6-61e5-444f-973e-c083bc8ec50f
📥 Commits

Reviewing files that changed from the base of the PR and between 273068f and 0aad21a.

📒 Files selected for processing: 11
  • config/deprecated-runtime-owner-importers-baseline.txt
  • config/focus-setting-read-baseline.txt
  • src/renderer/src/components/native-chat/NativeChatInlineVisual.open-as-file.test.tsx
  • src/renderer/src/lib/doc-preview-grants.test.ts
  • src/renderer/src/lib/doc-preview-grants.ts
  • src/renderer/src/lib/execution-host-facts.ts
  • src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts
  • src/renderer/src/lib/worktree-activation-pty-inventory.ts
  • src/renderer/src/lib/worktree-row-open-in-owner.test.ts
  • src/renderer/src/lib/worktree-row-open-in-owner.ts
  • src/renderer/src/lib/worktree-runtime-owner.ts
💤 Files with no reviewable changes: 1
  • config/deprecated-runtime-owner-importers-baseline.txt

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

The change adds a helper that returns an environment ID only when legacy-fallback owner resolution identifies an environment endpoint. Worktree-row owner handling and three renderer consumers now use legacy-fallback resolution. Tests and baselines are updated to reflect these changes.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 0aad2

No merge-blocking issue was established in the changed owner-resolution paths; the PR is mergeable after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 9 files. (1 skipped: 1 u…
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.
Title check Passed The title accurately identifies the main change: migrating four renderer library callers away from the deprecated runtime-owner resolver. It is concise and specific.
Description check Passed The description covers the main required sections, including the user-facing impact, implementation details, rationale, issue context, visual proof, and testing. It explains the internal-maintainer ex…
Full details: Docstring Coverage

Explanation

Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 9 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description covers the main required sections, including the user-facing impact, implementation details, rationale, issue context, visual proof, and testing. It explains the internal-maintainer exception for the missing linked issue and reports automated validation. Some optional checklist sections are omitted, but the description is sufficiently complete.

✨ Finishing Touches
📝 Generate docstrings
  • 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.

❤️ Share

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

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.

1 participant