Repository navigation
fix(orcad): folder workspaces can run scoped browser commands on a headless host - #29881
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: enabling folder workspaces to run scoped browser commands on a headless host. Description check The description is complete and explains the user impact, mechanism, rationale, testing, scope, and checklist status. It identifies the linked issue as an internal review finding and documents that th… 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. Full details: Description check
-
Explanation
The description is complete and explains the user impact, mechanism, rationale, testing, scope, and checklist status. It identifies the linked issue as an internal review finding and documents that the external Chromium provider received unit-test coverage only.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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:
940e9404-2f32-4492-a807-5a8de37e3dc3
📒 Files selected for processing: 4
src/main/orcad/electron-serve-browser-process.test.tssrc/main/orcad/electron-serve-browser-process.tssrc/main/orcad/external-chromium-tab-registry.test.tssrc/main/orcad/external-chromium-tab-registry.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| resolveBrowserWorkspace: async () => ({ id: 'folder:docs' }) | ||
| } | ||
| // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these tab paths read only the two selector resolvers and optional notifiers. | ||
| const commandHost = host as RuntimeBrowserCommandHost |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
git rev-parse HEAD; git diff 027eb0ec835a83641c6d21e82e7de0379eeec237 46cc8b45ed5efeb9e4aa9ca07fdbf46a41ff9c77 -- src/main/orcad/external-chromium-tab-registry.test.ts
sed -n '1,100p' src/main/orcad/external-chromium-tab-registry.test.ts
rg -n 'consistent-type-assertions|check-changed-code-quality' config package.json .github/workflowsRepository: stablyai/orca
Length of output: 7980
🏁 Script executed:
set -e
printf '%s\n' '--- gate implementation ---'
sed -n '1,260p' config/scripts/check-changed-code-quality.mjs
printf '%s\n' '--- gate tests ---'
sed -n '1,220p' config/scripts/check-changed-code-quality.test.mjs
printf '%s\n' '--- casting config ---'
cat config/oxlint-code-quality-casting.json
printf '%s\n' '--- source SHA metadata ---'
for rev in 46cc8b45ed5efeb9e4aa9ca07fdbf46a41ff9c77 48312f7200369a49b9ad5fd8cb42f51c18d717c5; do
printf '%s: ' "$rev"
git cat-file -t "$rev" 2>&1 || true
done
printf '%s\n' '--- relevant revision history ---'
git log --oneline --all --decorate -12 -- src/main/orcad/external-chromium-tab-registry.test.ts config/scripts/check-changed-code-quality.mjsRepository: stablyai/orca
Length of output: 19977
🏁 Script executed:
set -e
printf '%s\n' '--- remainder of gate implementation ---'
sed -n '240,620p' config/scripts/check-changed-code-quality.mjs
printf '%s\n' '--- quoted source commit ---'
git show --no-ext-diff --stat --oneline 48312f7200369a49b9ad5fd8cb42f51c18d717c5
git show -s --format='source=%H%nparents=%P%nauthor=%an <%ae>%nd%nsubject=%s' 48312f7200369a49b9ad5fd8cb42f51c18d717c5
printf '%s\n' '--- test file at both revisions ---'
for rev in 46cc8b45ed5efeb9e4aa9ca07fdbf46a41ff9c77 48312f7200369a49b9ad5fd8cb42f51c18d717c5; do
printf '\n### %s ###\n' "$rev"
git show "$rev:src/main/orcad/external-chromium-tab-registry.test.ts" 2>&1 || true
done
printf '%s\n' '--- diff between reviewed head and quoted source for relevant paths ---'
git diff --no-ext-diff --unified=20 46cc8b45ed5efeb9e4aa9ca07fdbf46a41ff9c77 48312f7200369a49b9ad5fd8cb42f51c18d717c5 -- src/main/orcad/external-chromium-tab-registry.test.ts config/scripts/check-changed-code-quality.mjs .github/workflows/pr.ymlRepository: stablyai/orca
Length of output: 17541
🏁 Script executed:
set -e
printf '%s\n' '--- numbered test source ---'
nl -ba src/main/orcad/external-chromium-tab-registry.test.ts
printf '%s\n' '--- registry declarations and relevant methods ---'
rg -n -F -- 'createTab(' src/main/orcad/external-chromium-tab-registry.ts
rg -n -F -- 'listTabs(' src/main/orcad/external-chromium-tab-registry.ts
sed -n '1,280p' src/main/orcad/external-chromium-tab-registry.ts
printf '%s\n' '--- sibling test patterns ---'
rg -n -F -- 'RuntimeBrowserCommandHost' src/main/orcad --glob '*.test.ts'
rg -n -F -- 'CreatedResult' src/main/orcad src --glob '*.ts' || true
printf '%s\n' '--- zod declarations and package entries ---'
rg -n -F -- '"zod"' package.json pnpm-lock.yaml src/main/orcad --glob '*.ts' || true
printf '%s\n' '--- workflow invocation ---'
sed -n '285,330p' .github/workflows/pr.yml
printf '%s\n' '--- merge-base helper ---'
sed -n '1,220p' config/scripts/git-pull-request-diff-base.mjsRepository: stablyai/orca
Length of output: 14994
🏁 Script executed:
set -e
python3 - <<'PY'
import json
import urllib.request
url = 'https://api.github.com/repos/stablyai/orca/check-runs/105646284299/annotations'
request = urllib.request.Request(url, headers={'Accept': 'application/vnd.github+json', 'User-Agent': 'CodeRabbit-review'})
try:
with urllib.request.urlopen(request, timeout=20) as response:
print(response.read().decode())
except Exception as exc:
print(f'ANNOTATIONS_FETCH_ERROR: {exc}')
PYRepository: stablyai/orca
Length of output: 202
Remove the two un-suppressed result assertions.
The changed-code gate reports the assertions on created and listed because both results are unknown. The session and commandHost assertions already have line-specific SAFETY: suppressions and do not need to change for this finding.
Suggested fix
+import { z } from 'zod'
+
+const CreatedResult = z.object({ browserPageId: z.string() })
+const ListedResult = z.object({ tabs: z.array(z.object({ browserPageId: z.string() })) })
+
- const created = (await registry.createTab(commandHost, { worktree: 'id:folder:docs' })) as {
- browserPageId: string
- }
- const listed = (await registry.listTabs(commandHost, { worktree: 'id:folder:docs' })) as {
- tabs: { browserPageId: string }[]
- }
+ const created = CreatedResult.parse(
+ await registry.createTab(commandHost, { worktree: 'id:folder:docs' })
+ )
+ const listed = ListedResult.parse(
+ await registry.listTabs(commandHost, { worktree: 'id:folder:docs' })
+ )
nwparker
force-pushed
the
nwparker/orcad-browser-folder-scope
branch
from
October 16, 2026 23:48
46cc8b4 to
5cae4bd
Compare
Both orcad browser adapters resolved --worktree with the git-only selector, so a folder workspace failed with selector_not_found before reaching the provider.
nwparker
force-pushed
the
nwparker/orcad-browser-folder-scope
branch
from
October 17, 2026 00:25
5cae4bd to
d306d30
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
On a headless Orca server (orcad, the kind that runs on an SSH or WSL host), browser commands scoped to a folder workspace (not a git worktree) failed with "selector not found", so an agent working in a folder workspace could not open or drive a browser tab there.
What Changed
orca tab create --worktree id:folder:<id>(and every other--worktree-scoped browser command) against orcad returnedselector_not_foundbefore reaching the browser provider. Git worktrees worked.Mechanism: both orcad browser adapters, the Electron sidecar (
electron-serve-browser-process.ts) and the external Chromium tab registry (external-chromium-tab-registry.ts), resolved--worktreewithresolveWorktreeSelector, which only knows git worktrees. They now useresolveBrowserWorkspace, the resolver the desktop browser paths and the client-hosted/bridge routes already use, which accepts folder workspaces and falls back to the git selector for everything else.Why
Same resolver everywhere a browser scope is resolved, rather than a second folder check in each adapter. With #29856 (remote CLI control on by default) merged, these adapters are reachable by default, so the gap now hits more users.
Found by an external Codex review (finding 12) and confirmed on current main.
Linked Issue
N/A, internal review finding.
Visual Proof
N/A: no UI change; CLI/RPC behavior only (live transcript below).
Testing
New tests, both failing on main and passing with the fix:
electron-serve-browser-process.test.ts: a host whose git selector throwsselector_not_foundbut whose browser resolver knows the folder workspace. Create and goto now reach the sidecar.external-chromium-tab-registry.test.ts(new): create + list for a folder workspace, and asserts the git-only selector is never consulted.Ran
src/main/orcadbrowser tests andsrc/main/runtime/browser(336 tests, single worker),pnpm tc, the static-analysis job steps locally, and the orcad terminal smoke.Live (macOS, isolated): built orcad from this branch and booted it under a throwaway
/tmpHOME and user-data dir on a free port, with the installed Orca app as the Electron browser sidecar. Created a project group plus a folder workspace over RPC, then drove the real orcad CLI with a pairing code:tab create --worktree id:folder:<id>returnedselector_not_found.tab createreturned a page id,goto --worktree id:folder:<id>(no--page) loaded the fixture (title: "Folder Browser Live"),evalreturnedfolder-ok, andtab listscoped to the folder listed the page.I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
codex review --base origin/main(gpt-6.1-sol, high): no findings.Agent skill upstream boundary
Notes
SSH/WSL: these are the adapters that serve browser commands on managed hosts. Git worktrees are unchanged (
resolveBrowserWorkspacefalls through to the same git selector). No wire change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)