Repository navigation
fix(serve): orca serve on orcad serves the browser client and answers plain HTTP - #30120
Conversation
… plain HTTP orcad, the default local orca serve host since the compatibility launcher, was never handed the app's web client directory, so its HTTP server had no request listener: every GET hung and no Web client URL was printed (#30100). The CLI now passes the app's unpacked out/web to orcad via --web-client-root, a server without a web client answers plain HTTP with 404 instead of hanging, and packaging fails when the unpacked web client or a file it loads is missing.
📝 Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 31.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 17 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the primary change: orca serveonorcadnow serves the browser client and responds to plain HTTP requests.Description check The description is complete and follows the repository template. It explains the user impact, implementation, rationale, linked issue, testing, compatibility considerations, and checklist status. Linked Issues check Issue [ #30100] requires a responsive headlessorca serveHTTP endpoint, browser-client serving when the bundle exists, aWeb client URL, and a response for invalid routes.serveWithOrcadresolve…Out of Scope Changes check The changed production files implement the [ #30100] launch handoff, root discovery, root validation, HTTP fallback, and packaging checks. The changed tests verify those behaviors. No unrelated product…Full details: Linked Issues check
-
Explanation
Issue [
#30100] requires a responsive headlessorca serveHTTP endpoint, browser-client serving when the bundle exists, aWeb client URL, and a response for invalid routes.serveWithOrcadresolves the shared bundled web root and passes it with--web-client-root.orcadvalidatesweb-index.htmlbefore use.WebSocketTransportinstalls a 404 request listener when no web client exists. The PR adds tests for argument propagation, root validation, HTTP 404 behavior, and packaged web assets.
Full details: Out of Scope Changes check
-
Explanation
The changed production files implement the [
#30100] launch handoff, root discovery, root validation, HTTP fallback, and packaging checks. The changed tests verify those behaviors. No unrelated production behavior or unrelated test scope is demonstrated.
-
- 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.
![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:
584f6a36-e32e-4608-a07d-f8b1c103fc55
📒 Files selected for processing: 17
config/electron-builder.config.cjsconfig/scripts/electron-builder-runtime-resources.test.mjsconfig/scripts/verify-packaged-web-client.cjsconfig/scripts/verify-packaged-web-client.test.mjssrc/cli/runtime/launch-serve-runtime.test.tssrc/cli/runtime/launch.tssrc/cli/runtime/serve-orcad-launch.test.tssrc/cli/runtime/serve-orcad-launch.tssrc/main/orcad/orcad-command-arguments.test.tssrc/main/orcad/orcad-command-arguments.tssrc/main/orcad/orcad-entry.tssrc/main/orcad/orcad-launch-contract.test.tssrc/main/runtime/rpc/static-web-client-handler.tssrc/main/runtime/rpc/ws-transport-static-web.test.tssrc/main/runtime/rpc/ws-transport.tssrc/main/startup/main-process-serve.tssrc/shared/bundled-web-client-root.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| /** The relative files web-index.html loads; the page is useless if any is missing. */ | ||
| function referencedAssets(html) { | ||
| const assets = new Set() | ||
| for (const match of html.matchAll(/\b(?:src|href)=["']\.\/([^"'?#]+)/g)) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Inspect the build configuration without running repository code.
rg -n -C 3 'base:|web-index|assetsDir|src=|href=' \
config/scripts/project-renderer-web-client.mjs vite.web.config.ts
# If a built index is already present, inspect its asset references.
if [ -d out ]; then
fd 'web-index\.html$' out --exec rg -n 'src=|href=' {}
fiRepository: stablyai/orca
Length of output: 2172
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- vite.web.config.ts ---'
nl -ba vite.web.config.ts | sed -n '1,55p'
printf '%s\n' '--- src/renderer/web-index.html ---'
nl -ba src/renderer/web-index.html | sed -n '1,180p'
printf '%s\n' '--- project-renderer-web-client.mjs relevant declarations ---'
nl -ba config/scripts/project-renderer-web-client.mjs | sed -n '1,210p'
printf '%s\n' '--- verify-packaged-web-client.cjs ---'
nl -ba config/scripts/verify-packaged-web-client.cjs | sed -n '1,180p'Repository: stablyai/orca
Length of output: 12207
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- build:web-from-renderer references ---'
rg -n -C 5 -F -- 'build:web-from-renderer' package.json pnpm-workspace.yaml config scripts vite*.config.* src || test "$?" -eq 1
printf '%s\n' '--- renderer build configurations ---'
rg -n -C 4 -e 'outDir|base:|web-index|manifest|renderer' vite*.config.* config/scripts config/build-plugins package.json | sed -n '1,260p'Repository: stablyai/orca
Length of output: 6962
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate Vite and Electron-Vite configuration files ---'
rg --files -g '*vite*.config.*' -g '*electron*vite*' -g 'electron.vite.*' -g 'package.json' | sort
printf '%s\n' '--- renderer output and web-index configuration ---'
rg -n -C 5 -e 'outDir' -e 'web-index' -e 'base:' -e 'manifest' -e 'renderer' --glob '*vite*.config.*' --glob 'config/scripts/*.mjs' --glob 'config/scripts/*.cjs' --glob 'package.json' . | sed -n '1,320p'Repository: stablyai/orca
Length of output: 26547
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- root Vite dependencies ---'
rg -n -C 3 -e '"vite"' -e '"electron-vite"' package.json pnpm-lock.yaml | sed -n '1,140p'
printf '%s\n' '--- renderer build runner ---'
nl -ba config/scripts/run-electron-vite-build.mjs | sed -n '1,180p'
printf '%s\n' '--- packaging hook and verifier call ---'
rg -n -C 8 -e 'assertPackagedWebClient' -e 'afterPack' config/electron-builder.config.cjs config --glob '*.cjs' --glob '*.mjs' | sed -n '1,220p'Repository: stablyai/orca
Length of output: 21459
Match the renderer’s root-relative asset URLs.
build:web-from-renderer copies the renderer build, whose Vite configuration does not set base: './'. Its generated index can therefore contain /assets/... URLs. The verifier matches only ./..., returns no assets, and causes afterPack to fail.
🐛 Suggested fix
--- "a/config/scripts/verify-packaged-web-client.cjs"
+++ "b/config/scripts/verify-packaged-web-client.cjs"
@@ -11,7 +11,7 @@
/** The relative files web-index.html loads; the page is useless if any is missing. */
function referencedAssets(html) {
const assets = new Set()
- for (const match of html.matchAll(/\b(?:src|href)=["']\.\/([^"'?#]+)/g)) {
+ for (const match of html.matchAll(/\b(?:src|href)=["'](?:\.\/|\/)([^"'?#]+)/g)) {
assets.add(posix.normalize(match[1]))
}
return [...assets]📝 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.
| for (const match of html.matchAll(/\b(?:src|href)=["']\.\/([^"'?#]+)/g)) { | |
| for (const match of html.matchAll(/\b(?:src|href)=["'](?:\.\/|\/)([^"'?#]+)/g)) { |
|
Live verification (PR head 78c7cb5):
|
1 participant
ELI5
orca serveon Linux (and anywhere else it runs on its standalone server, orcad) opened its port but never answered a web browser. Every page request just hung. The server simply had not been told where the browser app's files are. Noworca servetells it, and a server with no browser app still answers right away with "not found" instead of hanging.What Changed
Before (v1.4.229, headless Ubuntu):
orca serve --port 6768printed noWeb client URL.curl http://127.0.0.1:6768/,/web-index.htmland even/does-not-existall timed out with 0 bytes. WebSocket pairing still worked.After: the server prints its
Web client URL,GET /web-index.htmlreturns the Orca Web page, and a path it does not serve gets a 404.Mechanism:
orca serveruns on orcad by default, from a copy of the server placed under the user's data directory. Electron--servefound the web client next to its own app files (getBundledWebClientRoot). orcad was never given one, so its HTTP server had no request listener. Node accepts the connection and then never answers plain HTTP. Only WebSocket upgrades are handled.out/web(the same lookup Electron serve uses, now shared insrc/shared/bundled-web-client-root.ts) and passes it to orcad as--web-client-root <dir>. orcad checks that the directory holdsweb-index.html. If it doesn't, orcad warns and serves without a web client, so it never advertises a dead URL.ws-transport.ts: a server without a web client now answers plain HTTP with 404 instead of leaving requests hanging. This covers managed SSH/WSL orcad and any other host without the web bundle.config/scripts/verify-packaged-web-client.cjs, run inafterPack. The build fails ifapp.asar.unpacked/out/web/web-index.htmlis missing, if any./assets/...file it loads is missing, or if the unpacked CLI the root is resolved from is absent. orcad runs on plain Node and cannot read insideapp.asar, so these files must be unpacked.Why
orcad --helpand in the launch-contract tests. The CLI and the slot always come from the same install, so the new flag cannot reach an older orcad that would reject it.Wire compatibility: nothing paired clients exchange changes. The pairing offer only gains the existing optional
webClientUrl, which Electron serve already publishes.Linked Issue
Fixes #30100
Visual Proof
N/A. The change is in headless server HTTP behavior and has no UI. The curl transcripts are under Testing.
Testing
Reproduced on a real Linux host (omarchy, Arch x86_64) with the released v1.4.229
.deb, extracted to/tmp(not installed), using an isolatedHOME/XDG dirs and a free port.orca-ide servereportedrunning on orcad, printed no Web client URL, andcurlto/,/web-index.htmland/does-not-existall timed out withHTTP=000, while a WebSocket upgrade returned101. This matches the report.Unit tests, each failing before the fix:
ws-transport-static-web.test.ts: a server without a web client answers/,/web-index.htmland/does-not-existwith 404 (they hung before).launch-serve-runtime.test.ts:orca servehands orcad<appRoot>/out/web.serve-orcad-launch.test.ts: orcad argv carries--web-client-root.orcad-launch-contract.test.ts:--web-client-rootis parsed.orcad-command-arguments.test.ts: a root withoutweb-index.htmlis refused with a warning.verify-packaged-web-client.test.mjs: packaging fails on a missing index, a missing referenced asset, or a missing unpacked CLI.electron-builder-runtime-resources.test.mjs: the afterPack fixtures now carry a web client.Locally:
pnpm tc, the full static-analysis job steps (oxlint, anti-slop, code-quality plugins, type-aware, changed-code quality, every ratchet), and unit tests forsrc/main/runtime/rpc,src/cli/runtime,src/main/startupandsrc/main/orcad. One test insrc/main/orcad,orcad-forked-child-paths, fails on this Mac because of the/private/varrealpath of the temp dir. It fails the same way without this change.Live check of the fixed build: see the comment below once the PR's Linux AppImage was run on the same host.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Codex review (gpt-6.1-sol, high) found two launch tests still expecting four arguments. They are fixed in the second commit. The re-review was clean.
Agent skill upstream boundary
Notes
path.join.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)