[brennanb2025]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​214	$\color{#cf222e}{\Huge{\mathbf{−}}}$​12	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​202
Prod	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​104	$\color{#cf222e}{\Huge{\mathbf{−}}}$​39	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​65

ELI5

Structured chat (Orca's native chat with Claude, Codex, and the agents coming next) has to run in orcad, Orca's headless runtime. That's a plain Node process with no Electron. Orca already has a build-time check that fails CI when any file the runtime loads imports Electron. It only looked at files reachable from the runtime's three entry files, though. Structured-chat code the runtime doesn't load yet, or that's only reached from the desktop app, could take on an Electron dependency without anything noticing. The problem would only show up later, when someone tried to run that code headless.

No user-visible change. This PR widens that existing check to whole structured-chat directories, so new files there are checked from the day they're added.

What Changed

• Scope. The check that stops new Electron imports from creeping in (config/scripts/check-runtime-electron-ratchet.mjs) still bundles the runtime with esbuild and lists every module that imports electron. Besides the three runtime entry files, it now starts from every non-test source file in:
  • src/main/native-chat/, src/main/claude/, src/main/codex/ and src/shared/, as whole directories at any depth;
  • src/main/runtime/, only files named structured-* or agent-session-*, at any depth. That directory still holds desktop-only code (browser commands, the desktop relay), so the whole directory can't be checked yet;
  • src/main/acp/ and src/main/provider-process/, which don't exist on main yet (see Review).
• No silent gaps. If one of the directories that exist today is renamed or removed, the check now fails with a message naming the directory. Before, it would quietly check nothing there.
• Test code stays out. Which files count as test-only is now decided by one shared rule (config/scripts/test-only-source-path.mjs). It covers specs (*.test.*, *.spec.*), helpers beside a spec (*-test-<anything>), fixtures and test doubles (*-fixture(s), *-mock(s), *-fake(s), *-stub(s), *-double(s)), and test directories (__tests__/, __fixtures__/, test-support/, orca-runtime-tests/, and so on). *-self-test* files and names that only end in -test stay in, because they ship. The localization audit (audit-localization-coverage.mjs) had its own narrower copy of this rule and now uses the shared one. It now skips 20 more renderer files, all test helpers with no user-visible strings, and its result is unchanged: 13 allowlisted candidates, as before.
• Failure message. It now explains that the runtime and structured chat both have to run in orcad. It says that renaming or moving the file is not a fix, and it no longer links a design document that isn't in the repository. The baseline file's header says the same.
• The baseline of known Electron importers stays empty. CI enforces it in the existing static-analysis step ("Enforce runtime Electron-import ratchet") and in pnpm lint.

Why

What's newly covered. On current main, the lanes add 2,375 entry files. 1,830 of them were already reached from the three runtime entries, so they were checked before. The other 545 are newly checked: 499 in src/shared, 33 in src/main/codex, 12 in src/main/native-chat and 1 in src/main/claude. Some of those only declare types, so they contain no code. The native-chat ones include the transcript readers that only the desktop IPC handler (the bridge between the window and the main process) reaches today. No newly checked file imports Electron.

Policy note. This widens the rule from "code the headless runtime loads" to "structured-chat and shared code, wherever it runs today". For example, Codex session code that only the desktop process uses can no longer import Electron directly. That keeps such code movable to the runtime without a rewrite.

Why this approach.

• Why whole directories: an earlier version of this PR listed filename prefixes. That already missed files that sibling PRs in this stack add, such as codex-provider-timeline-identity.ts and runtime/rpc/methods/structured-agent-session-agents.ts. Whole directories cover new files by default, and the missing-directory error keeps the list honest.
• Why one check: this reuses the existing esbuild check instead of adding a second import resolver. esbuild follows local imports, package dependencies and package exports, so Electron pulled in through an npm package is caught too.

Linked Issue

N/A — internal foundation work to keep structured chat runnable without Electron; no separate issue was opened.

Visual Proof

N/A — build-time check only; no visual or interaction change.

Testing

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

What I verified / didn't

On macOS, at head 0c968d4b68c:

• Local setup caveat. My local dependency install predates main's new stream-json / stream-chain packages. Because of that, node config/scripts/check-runtime-electron-ratchet.mjs and the two tests that bundle the real source tree can't resolve them here. Main's own three runtime entries fail the same way locally. CI installs fresh dependencies. To cover those checks, I ran the same calls with only those two packages marked external:

  • the full check found 0 Electron importers across 2,378 entry files, matching the empty baseline;
  • no vitest, @vitest/*, chai or rolldown-vite file reaches the bundle any more (before this change, test helpers pulled them in);
  • the bundle took about 3–6 seconds locally (the machine was under load), against about 0.5 seconds for the three runtime entries alone.

• Tests.

  • check-runtime-electron-ratchet.test.mjs at this head: 14 passed. The 2 failures are the real-tree tests hitting the dependency caveat above.
  • At the previous head, test-only-source-path.test.mjs and audit-localization-coverage.test.mjs also passed.

• New tests. They cover:

  • whole-directory coverage, including the sibling-PR file names above;
  • recursive structured-* / agent-session-* files under src/main/runtime;
  • test-helper and test-double exclusion;
  • the missing-directory error, once for each directory that exists today;
  • Electron arriving through an npm package in each of acp/ and provider-process/;
  • the default entry list (runtime entries plus every existing directory);
  • a test that fails once src/main/acp/ or src/main/provider-process/ exists while still allowed to be absent.

  The two real-tree tests call the command-line check itself (main(), exported for this), which is what pnpm lint and CI run:

  • one adds import 'electron' to src/main/native-chat/transcript-read-cache.ts in memory only, never on disk, and expects the check to fail and name that file (the runtime entries alone don't reach it);
  • the other expects the unmodified tree to pass.

• Mutation check. I broke the script by hand, one change at a time, then restored it from the commit:

  • first round (9 variants): drop the directories from the default entry list (2 variants); drop acp, provider-process or src/shared; allow codex to be missing; check all of src/main/runtime; drop the test-file rule; drop the test-directory rule;
  • second round (8 variants): have main() check only the runtime entries; drop the directories from the default list; allow each existing directory in turn to be missing (5 variants); create src/main/acp/ while it's still allowed to be absent.

  Every variant turned at least one test red. The main() and default-list variants are caught by the in-memory injection test. Because that test can't run locally as is, I saw it go red through the same main() call plus the dependency workaround.

• Lint and formatting. oxfmt and oxlint on the changed files and git diff --check are clean. The lockfile is unchanged from origin/main.

• CI.

  • On the previous head b42050bf4e6, static analysis and typecheck passed. That step runs this check with fresh dependencies. Its unit shards were cancelled by the next push.
  • For 0c968d4b68c, check the run on this head.
  • The cross-version qoder-history-search-downgrade test fails on unrelated PRs too. If it fails here, it isn't from this change.

• Known limit: test files are recognised by name. A future shipped file in these directories whose name contains -test- (other than self-test), or ends in -fixture, -mock, -fake, -stub or -double, would be wrongly treated as test code. It would then not be an entry point of its own; it's still checked if other checked code imports it. Today no shipped file in these directories matches. Revisit if a product feature with "test" in its name (an A/B test, a connection test) lands here.

I didn't launch Orca, Electron, orcad or a real agent. I didn't run Windows or Linux, and I didn't run a typecheck locally: only .mjs files changed, and CI's typecheck is the authority. Passing checks show today's code is clean and the scope is wired in; they don't prove the scope is complete for code that later lands outside these directories.

AI Disclosure

Review

Differences from the common pattern

The common pattern keeps Electron out of headless code by package boundaries: only the desktop app's package declares electron, so other code can't resolve it at all.

• Intended: Orca is one package, so this enforces the boundary with a build-time import-graph check over named directories instead. It's safe to keep because whole directories cover new files by default, a missing directory fails loudly, and package-borne Electron is caught.
• Intended: src/main/runtime/ is filtered by name (structured-*, agent-session-*) instead of being checked whole, because it still holds desktop-only files that import Electron. Those runtime files that the three runtime entries load are already checked through them.
• Temporary: src/main/acp/ and src/main/provider-process/ may be absent. This lasts until the ACP adapter and provider-process PRs in this stack land. A test fails as soon as either directory exists while still allowed to be absent, so the PR that creates it has to remove the flag, and the directory then fails loudly like the others.

Possible follow-up (not built here)

The scope that can't drift is the inverse: check every production file under src/main and src/shared except a short list of desktop-shell directories, with a list of today's Electron users that may only shrink. That's an app-wide policy beyond structured chat, so it isn't part of this PR.

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

• Paths are built with Node path utilities, so the check behaves the same on Windows, macOS and Linux.
• It changes no runtime behavior, wire protocol, session ownership or process placement for local or SSH runtimes.
• No dependency, documentation-file or lockfile changes.

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)

[brennanb2025]

Review summary (head 0c968d4b68c)

The problem. Structured chat has to run inside Orca's headless runtime, a plain Node process with no Electron. Orca already had a CI check that fails when anything reachable from the runtime's three entry files imports Electron. Structured-chat code the runtime doesn't load yet was invisible to that check, so it could pick up a desktop-only dependency unnoticed. That code includes the new Agent Client Protocol and provider-process folders, and files only the desktop app loads today.

What changes for users. Nothing visible. This is a CI check.

What the review rounds changed

• The earlier draft added a second, hand-written import checker. It now extends the existing esbuild-based check instead, so there is one answer to "can this run without Electron", and imports that reach Electron through an npm package are caught too.
• Scope is whole folders, not a hand-written list of folders and filename prefixes: src/main/{native-chat,acp,provider-process,claude,codex} and src/shared. In src/main/runtime only structured-session files count, because that folder still holds desktop-only code. The old list already missed structured-chat files that sibling branches add. The new scope checks 545 more production files, with 0 Electron imports found.
• A folder that must exist but is missing now fails the check, instead of silently checking nothing after a rename. acp/ and provider-process/ may be absent until their PRs land. A test fails once either folder exists while it is still marked "may be missing", so that allowance retires itself.
• One rule decides what counts as a test file, shared with the localization audit. Six test helpers named *-test-<thing>.ts had been slipping into the production check. The audit's result was proven unchanged by running it before and after.
• Tests now fail if the folder wiring is dropped, including through the command-line path. 8 of 8 deliberate breakages are caught.
• The failure message no longer links a file that isn't in the repository. The description was corrected.

Heads-up for landing order: whichever of this PR, #24989 (adds src/main/provider-process/) and #24990 (adds src/main/acp/) lands second must drop that folder's "may be missing" allowance, or this check's test fails.

Deferred: test helpers are recognised by name, so an unusually named helper could still slip in (noted in the description). An app-wide version of this check, covering all of src/main with a shrink-only list of today's Electron files, is a possible follow-up and is not built here.

What was verified: two review rounds plus a readiness checklist found no blocking issues. Locally the check found 0 Electron importers across 2,378 entry points; CI cost is about 5 s per run. On this head, CI static analysis and typecheck pass (static analysis is where this check runs), and 3 of 5 unit shards pass. Shards 1 and 5 fail only in renderer translation tests (source-control-discard-localization.test.ts, NativeChatSupportedAgents.test.tsx). They fail the same way on #24989, which touches no renderer files, so they come from current main.

Not verified: no app launch. Locally the shared install is missing two packages main recently added, so the full-tree run used a scratch wrapper that treats them as external; CI is the real run.

[pullfrog]

This run was cancelled 🛑

The workflow was cancelled before completion. Please check the link below for details.

  ^{｜ View workflow run ｜ via Pullfrog ｜ 𝕏}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 7cb64f8b-1930-401d-ad47-83b443bd0e4c

📥 Commits

Reviewing files that changed from the base of the PR and between b354aab and bd141cb.

📒 Files selected for processing (6)

• config/runtime-electron-baseline.txt
• config/scripts/audit-localization-coverage.mjs
• config/scripts/check-runtime-electron-ratchet.mjs
• config/scripts/check-runtime-electron-ratchet.test.mjs
• config/scripts/test-only-source-path.mjs
• config/scripts/test-only-source-path.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds shared helpers that classify test-only source paths and uses them in the localization coverage audit. The Electron import ratchet now checks runtime entry points and configured structured-chat lanes, including files not yet loaded by the runtime. The ratchet supports injected esbuild plugins, returns status codes from main, and includes tests for lane discovery and CLI results.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to bd141

No actionable merge-blocking risk is identified in these audit changes; merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	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 10 functions across 5 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: checking structured-chat code for Electron imports before the runtime loads it.
Description check	✅ Passed	The description covers the required sections and gives clear scope, rationale, testing details, and review notes. It explains that no visual change applies and gives a reason for the missing linked is…
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.

Full details: Docstring Coverage

Explanation

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 10 functions across 5 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

• Autopilot · 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

• X
• Mastodon
• Reddit
• LinkedIn

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