Repository navigation
test(runtime): fail when Electron and orcad wire the runtime differently without a reason - #30212
Conversation
…n undeclared differences
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 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 main change: adding a test that detects unexplained runtime wiring differences between Electron and orcad. Description check The description is detailed and covers the change, rationale, testing, visual proof, review, scope, and checklist. The linked issue section references issues in text but does not include a direct issu… 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 detailed and covers the change, rationale, testing, visual proof, review, scope, and checklist. The linked issue section references issues in text but does not include a direct issue link or a
Fixes #...entry.
-
- 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}
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:
eff6727a-ad93-4527-b3b7-a5d74c46fc12
📒 Files selected for processing: 1
src/main/runtime/construction/runtime-composition-parity.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| } else if (char === '/' && source[i + 1] === '*') { | ||
| i = source.indexOf('*/', i + 2) + 1 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Fix the unterminated block comment handling in stripComments.
If source.indexOf('*/', i + 2) returns -1, the expression -1 + 1 sets i to 0. The for loop then increments i to 1. The loop restarts from the file start and never ends. This only happens for a source file with an unterminated /*. The result is a hung test run instead of a clear error. The same loop can also repeat on a /* inside a regex literal.
Throw an error when the terminator is missing.
Proposed fix
--- "a/src/main/runtime/construction/runtime-composition-parity.test.ts"
+++ "b/src/main/runtime/construction/runtime-composition-parity.test.ts"
@@ -165,8 +165,12 @@
i++
}
out += '\n'
} else if (char === '/' && source[i + 1] === '*') {
- i = source.indexOf('*/', i + 2) + 1
+ const end = source.indexOf('*/', i + 2)
+ if (end === -1) {
+ throw new Error('unterminated block comment')
+ }
+ i = end + 1
} else {
if (char === "'" || char === '"' || char === '`') {
quote = char📝 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.
| } else if (char === '/' && source[i + 1] === '*') { | |
| i = source.indexOf('*/', i + 2) + 1 | |
| } else if (char === '/' && source[i + 1] === '*') { | |
| const end = source.indexOf('*/', i + 2) | |
| if (end === -1) { | |
| throw new Error('unterminated block comment') | |
| } | |
| i = end + 1 |
ELI5
Orca builds its runtime in two places: the desktop app (Electron) and the headless server (orcad). Every piece the runtime can be given is optional, so one place can quietly forget a piece the other provides. This adds a test that lists what each place provides and fails when they differ without a written reason.
What Changed
src/main/runtime/construction/runtime-composition-parity.test.ts. It reads both composition roots' source and records:new OrcaRuntimeService(...), following spreads into their helper objects (agentHookRuntimeDeps, the Codex launch resolvers, orcad's account deps);new OrcaRuntimeRpcServer(...)(this is where fix(serve): orca serve on orcad serves the browser client and answers plain HTTP #30120'swebClientRootlives);runtime.set*/rpc.set*setter call, and whether SSH handlers are registered.DECLARED_HOST_DIFFERENCESlists the 18 differences that exist today, each with a reason. An undeclared difference fails, and so does a declaration that no longer matches anything.Why
Round 2 class C2: the Electron and orcad roots drift and nothing compares them (#30120, #29209, #29881). The proposal suggested booting both roots with stubbed services. That needs about 40 module mocks for orcad and the Electron app object for desktop, and a mock list would drift like the roots do. Recording from the source catches the same thing (an option or dependency one root sets and the other doesn't), runs in milliseconds, and refactors nothing in
orcad-entry.ts, which other agents are editing. An unknown spread fails with "teach SPREAD_SOURCES", so the recorder cannot silently miss a dependency.Proof: deleting
webClientRootfrom orcad'sOrcaRuntimeRpcServeroptions (the #30120 shape) fails two of the three tests.Follow-up found while declaring: orcad never wires
getPairedDeviceName, so client-hosted browser rows show no paired-device name there.Linked Issue
Refs round-2 SSH/remote hardening S3 (C2). Regression guard for #30120.
Visual Proof
N/A: test only, no behaviour change.
Testing
pnpm test src/main/runtime/construction/passes. WithwebClientRootremoved fromorcad-entry.tsit fails.The full static-analysis job, run locally.
Not verified live: nothing runtime-visible changed.
Automated tests added/updated, or explained why not below
Review
Codex review (gpt-6.1-sol, high): no findings.
Agent skill upstream boundary
Notes
No wire, SSH, mobile or platform impact: test only.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)