Skip to content

fix(build): import the Electron remote capability list from its own module - #24494

Merged
brennanb2025 merged 1 commit into
mainfrom
brennanb2025/fix-main-electron-capability-import
Oct 2, 2026
Merged

brennanb2025 merged 1 commit into
mainfrom
brennanb2025/fix-main-electron-capability-import

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 2, 2026 •

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

ELI5

main stopped typechecking right after #24203 merged. Nothing about the app changes in this PR; it puts back four lines so the build passes again.

What Changed

What broke: pnpm tc:node on main (7577366) fails with four errors. CI on every PR based on current main will be red for the same reason.

Why: two changes passed review separately and conflict once both are on main:

After this PR: the three files import the list from its new module, and the test passes a logger the same way the other structured-host tests already do (createStructuredAgentSessionLogger()). No runtime behaviour changes: it is the same list, imported from where it now lives.

Why

Restoring an export from protocol-version.ts would keep the old import working, but that file is at its line limit, which is why the list moved. Pointing the new importers at the module the list now lives in is the smaller and lasting fix.

Visual Proof

N/A: build fix, no UI or behaviour change.

Testing

  • tc-gate pnpm tc:node passes on this branch (it fails on main 7577366 with these four errors).

  • structured-agent-session-held-signal.test.ts and runtime-ssh-access-verification.test.ts pass.

  • I manually tested these changes locally

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

…odule

#24203 moved ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES out of protocol-version.ts into
electron-remote-runtime-client-capabilities.ts. Three files that landed on main while it was in
review (SSH access links and managed orcad server work) still import it from protocol-version.ts,
and a test #24203 added predates main making the structured host's logger option required, so
main's node typecheck fails. Point the three imports at the new module and pass the logger.
@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Low risk] Moves an Electron capability constant to its own module.

The PR appears safe to merge; no issue introduced by these changes was identified.

Summary

This PR repairs four typecheck failures caused by imports and a test fixture that no longer match their dependencies.

  • Points three capability-list imports at the module that exports the list.
  • Supplies the required logger when the held-signal test installs a structured agent session host.

Reviews (1) · Last reviewed commit: "fix(build): import the Electron remote c..."

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Mechanical build fix restoring pnpm tc:node on main; no runtime behavior change.

  • Repointed capability import — runtime-environment-identity-verification.ts, orcad-terminal-census-client.ts, and runtime-ssh-access-verification.test.ts now import ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES from its post-#24203 module, splitting the import where unrelated protocol-version symbols remain.
  • Added required logger in test — structured-agent-session-held-signal.test.ts passes createStructuredAgentSessionLogger(), matching sibling structured-host tests.

Verified: all 52 references to the capability list now resolve from the new module with no stale protocol-version importers; ORCAD_TERMINAL_CENSUS_RUNTIME_CAPABILITY and RUNTIME_PROTOCOL_VERSION remain exported and used; the logger factory exists and logger is required at structured-agent-session-runtime.ts:122.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

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: 7fa92b8d-7e6e-497b-a84a-825fbe30134a

📥 Commits

Reviewing files that changed from the base of the PR and between 7577366 and d8b818a.

📒 Files selected for processing (4)
  • src/main/runtime/runtime-environment-identity-verification.ts
  • src/main/runtime/structured-agent-session-held-signal.test.ts
  • src/main/ssh/orcad-terminal-census-client.ts
  • src/main/ssh/runtime-ssh-access-verification.test.ts

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


📝 Walkthrough

Walkthrough

Runtime and SSH code and tests now import client capabilities from the dedicated module. The structured-agent-session test creates a logger and passes it to installHost.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d8b81

The import updates and test setup match the described contracts, with no identified behavior regression; the change is ready for normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 2 functions across 4 files. 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 identifies the main change: importing the Electron remote capability list from its dedicated module. It is concise and relevant.
Description check ✅ Passed The description covers the problem, cause, changes, rationale, testing, and lack of behavior or UI changes. It is mostly complete, although it does not include the template's Linked Issue, Review, Not…
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.
  • 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
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@brennanb2025
brennanb2025 merged commit 4534087 into main Oct 2, 2026
45 of 47 checks passed
brennanb2025 added a commit that referenced this pull request Oct 2, 2026
…through the logger, and widen the ratchet to match

- Claude: a descendant that survives a close and a failed at-rest `/` command scan now go to the
  chat's logger with its session id. The stream-json connection and the at-rest catalog take the
  logger as a required dependency; only structured Claude code opens or builds either.
- Runtime: a failed structured host install before a worker stop or release, and a forced
  worktree removal that leaves structured sessions unclosed, report through the runtime's logger.
  The removal keeps its console line when no runtime is passed. The runtime wraps an injected
  logger so a throwing one cannot fail the step it reports.
- The structured mail redrive's catch moves into structured-session-mail-target.ts, so the
  ratchet covers it without scanning the PTY record file's own mailbox warnings.
- The WSL transcript gate's deadline warning also writes a failed span to the trace file through
  the existing tracer; the console line stays because scanner workers install no trace sink.
- Ratchet scope: drop the trailing dash that skipped rpc/methods/structured-agent-session.ts,
  cover the runtime's structured-* files, the structured worker stop/release files and the two
  Claude files, and fail if a listed file stops being scanned.
- The held-signal test is back to main's version (#24494 fixed it the same way).
- The option-restore test asserts the logger entry instead of a console spy that no longer fired.
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