Skip to content

test(orcad): orca serve's command line makes orcad serve the web client (#30120 regression) - #30222

Merged
nwparker merged 1 commit into
mainfrom
nwparker/r2-orcad-serve-http
Oct 18, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/r2-orcad-serve-http

Conversation

@nwparker

@nwparker nwparker commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​49 $\color{#cf222e}{\Huge{\mathbf{−}}}$​11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​38
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​44 $\color{#cf222e}{\Huge{\mathbf{−}}}$​44 0

ELI5

orca serve starts the headless Orca server (orcad) and should print a browser link that opens the web client. In #30120 the link never appeared and every page request hung, because the CLI didn't tell orcad where the web client was. Unit tests now pin each half of that fix, but nothing ran the two halves together. This adds a test that does.

What Changed

  • src/main/orcad/orcad-serve-parity.integration.test.ts gets a new case, run against the built orcad slot:
    • it builds orcad's command line with the CLI's own orcadServeArgs and findBundledWebClientRoot, pointing at a scratch app root that holds out/web/web-index.html. The bind is switched to loopback so the test never listens on every interface;
    • it checks the readiness line names a webClientUrl, that GET /web-index.html returns 200 with the file, and that an unknown path returns 404.
  • serveOnce now takes the orcad arguments and returns the readiness payload. The D7 restart test reads the daemon pid through a small serveDaemonPid wrapper. Its behaviour is unchanged.
  • ServeOrcaAppArgs, serveOptionArgs and orcadServeArgs move unchanged from src/cli/runtime/serve-orcad-launch.ts to src/shared/orcad-serve-argv.ts. They are the pure CLI→host argv contract, and the main-process tsconfig can't import from src/cli. Their callers import the new path directly, with no re-export left behind. Behaviour is identical.

Why

Round 2, C2 / S3.2: #30120 was an option dropped while crossing a process boundary (CLI → orcad argv). That kind of bug passes every unit test on either side. This test runs the real handoff: the CLI's argv builder, orcad's argument parser and orcad's HTTP server, all together. The proposal leaves a SERVE_OPTION_SPECS table for later, to be added only if another option gets dropped.

Proof: with --web-client-root removed from orcadServeArgs (the #30120 bug), the new case fails with expected null to deeply equal StringMatching.

Linked Issue

Refs round-2 SSH/remote hardening S3 (C2). Regression test for #30120.

Visual Proof

N/A: test only.

Testing

  • Local macOS, with a freshly built out/orcad: pnpm exec vitest run src/main/orcad/orcad-serve-parity.integration.test.ts passes all 5 cases. With the fix(serve): orca serve on orcad serves the browser client and answers plain HTTP #30120 bug restored, the new case fails.

  • In CI it runs in Node server tests (--artifact) on Linux, macOS and Windows.

  • The full static-analysis job, run locally.

  • I manually tested these changes locally

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

Review

Codex review (gpt-6.1-sol, high): no findings.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Test only; no wire or platform change. On Windows the profile root uses tmpdir(), as the existing D7 case does.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 18, 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 9 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: 77f4b6e6-3bd5-4f2b-8f9f-5dbb57bdd093
📥 Commits

Reviewing files that changed from the base of the PR and between d4b3a65 and 653aa2d.

📒 Files selected for processing: 5
  • src/cli/runtime/launch.ts
  • src/cli/runtime/serve-orcad-launch.test.ts
  • src/cli/runtime/serve-orcad-launch.ts
  • src/main/orcad/orcad-serve-parity.integration.test.ts
  • src/shared/orcad-serve-argv.ts
  • 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.

@nwparker
nwparker merged commit d3d6162 into main Oct 18, 2026
53 checks passed
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