Synced from GitHub
refactor(desktop): classify electron exports#10266
Electron modules had 15 unclassified runtime exports. This marks ten canonical Effect service constructors as intentional public APIs, narrows four protocol implementation exports, and removes one unused safe-storage predicate without changing the exported error schema.
This is layer 2 of 5 in the desktop Knip cleanup stack.
Verification after the full stack: desktop typecheck; desktop Knip export audit; 831 desktop tests; changed-file lint and format checks. The native libsecret test cannot start on this machine because the system libsecret-1 development package is unavailable.
Model: gpt-6 astra. Harness: Codex in T3 Code.
Note
Make electron helper functions and constants private across desktop modules
- Changes
PRODUCTION_SCHEME,DEVELOPMENT_SCHEME,getDesktopOrigin, andregisterDesktopSchemePrivilegesSyncin ElectronProtocol.ts from named exports to module-private bindings. - Removes the
isElectronSafeStorageErrornamed export from ElectronSafeStorage.ts. - Adds public API documentation comments to the
makeservice construction export across all electron service modules. - Behavioral Change:
isElectronSafeStorageError, the protocol scheme constants,getDesktopOrigin, andregisterDesktopSchemePrivilegesSyncare no longer importable from their modules; any out-of-tree consumers must update.
Macroscope summarized bf20c62.
Summary by CodeRabbit
- Documentation
- Added public API documentation for desktop service construction APIs.
- Clarified canonical entry points for Electron services.
- Refactor
- Restricted visibility of internal desktop protocol helpers and constants.
- Removed an internal safe-storage error predicate from the public API.
Synced from GitHub
Approvability
Verdict: Approved at eca58d0
Macroscope's review found this PR approvable — This is a narrowly scoped Electron export cleanup: service construction remains intact, internal protocol helpers are no longer exported, and an unused safe-storage guard is removed. The diff does not change request-path behavior, product defaults, schemas, or production workflows.
No code changes detected at bf20c62. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Thread transfer impact
✅ Thread transfer remains within every enforced ceiling.
ℹ️ No successful
mainbaseline artifact is available yet. This run establishes the initial measurement.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | — | 13.6 KiB | — | 15.1 KiB | ✅ |
| Codex | Thread snapshot wire | — | 7.0 KiB | — | 7.3 KiB | ✅ |
| Codex | Live turn WebSocket wire | — | 6.6 KiB | — | 7.8 KiB | ✅ |
| Codex | Live turn WebSocket decoded | — | 57.1 KiB | — | 66.4 KiB | ✅ |
| Codex | Live turn messages | — | 10 | — | 21 | ✅ |
| Claude | Total thread wire | — | 13.5 KiB | — | 15.1 KiB | ✅ |
| Claude | Thread snapshot wire | — | 7.0 KiB | — | 7.3 KiB | ✅ |
| Claude | Live turn WebSocket wire | — | 6.5 KiB | — | 7.8 KiB | ✅ |
| Claude | Live turn WebSocket decoded | — | 57.8 KiB | — | 66.4 KiB | ✅ |
| Claude | Live turn messages | — | 8 | — | 21 | ✅ |
Baseline: unavailable · PR result: bf20c62 · Source CI: success
Scenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 113.9 KiB
- Claude decoded thread snapshot: 114.6 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
Synced from GitHub
The same change (export const make → const make) is applied in nine other service modules in this PR: ElectronDialog.ts, ElectronMenu.ts, ElectronPowerMonitor.ts, ElectronProtocol.ts, ElectronSafeStorage.ts, ElectronShell.ts, ElectronTheme.ts, ElectronUpdater.ts, ElectronWindow.ts. The inline note below applies to each of them. The other unexports in this PR (DESKTOP_PRODUCTION_SCHEME, DESKTOP_DEVELOPMENT_SCHEME, getDesktopOrigin, registerDesktopSchemePrivilegesSync) are module-local pure helpers, not part of a service's public shape, so narrowing those looks correct.
Posted via Macroscope — Effect Service Conventions
| … | … | @@ -98,7 +98,7 @@ const addScopedAppListener = <Args extends ReadonlyArray<unknown>>( | |
| 99 | 99 | ).pipe(Effect.asVoid); | |
| 100 | 100 | ||
| 101 | - | export const make = ElectronApp.of({ | |
| 101 | + | const make = ElectronApp.of({ | |
electron/ElectronApp.ts:101
make is part of the canonical public shape of a service module that owns construction (Foo.Foo, Foo.make, Foo.layer), so consider keeping it exported even though only the local layer consumes it today — that is what packages/client-runtime/src/connection/driver.ts does, where the unused-export lint is silenced with a tag comment (/** @public Service construction is part of the canonical Effect module API. */) rather than by dropping export.
suggestion
| 1 | export const make = ElectronApp.of({ |
No longer relevant as of eca58d022256cf5584ba343ff8925a02aecc00bb
Synced from GitHub
Added 1 commit
- refactor(desktop): keep electron helpers privatebf20c62
Synced from GitHub
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Team
Run ID: a750c205-3c45-4e63-bc05-f571da6a19aa
📥 Commits
Reviewing files that changed from the base of the PR and between fe8fffac36e33a6872351f261b8d21745b409df2 and bf20c623db759b3d1f960d3250dacadb6cf348cf.
📒 Files selected for processing (10)
apps/desktop/src/electron/ElectronApp.tsapps/desktop/src/electron/ElectronDialog.tsapps/desktop/src/electron/ElectronMenu.tsapps/desktop/src/electron/ElectronPowerMonitor.tsapps/desktop/src/electron/ElectronProtocol.tsapps/desktop/src/electron/ElectronSafeStorage.tsapps/desktop/src/electron/ElectronShell.tsapps/desktop/src/electron/ElectronTheme.tsapps/desktop/src/electron/ElectronUpdater.tsapps/desktop/src/electron/ElectronWindow.ts
Included review availability: 7 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.
📝 Walkthrough
Walkthrough
The change documents Electron service constructors as public Effect APIs. It also makes selected protocol helpers module-private and removes the exported safe-storage error predicate.
Changes
Electron API surface cleanup
| Layer / File(s) | Summary |
|---|---|
Document Electron service constructors apps/desktop/src/electron/ElectronApp.ts, ElectronDialog.ts, ElectronMenu.ts, ElectronPowerMonitor.ts, ElectronProtocol.ts, ElectronSafeStorage.ts, ElectronShell.ts, ElectronTheme.ts, ElectronUpdater.ts, ElectronWindow.ts | Public API documentation comments were added to the Electron service constructors. |
Restrict internal Electron helpers apps/desktop/src/electron/ElectronProtocol.ts, ElectronSafeStorage.ts | Selected Electron protocol declarations and isElectronSafeStorageError are no longer exported. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Merge Risk: ⚪ Minimal · up to bf20c
This change clarifies the Electron service API surface and removes internal or unused exports. No current merge-readiness risk remains.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 10 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| 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. |
| Title check | ✅ Passed | The title clearly summarizes the main change: refactoring and classifying Electron exports. |
| Description check | ✅ Passed | The description explains the export changes, rationale, scope, verification, and known test limitation. It does not use the template headings or checklist, but it is mostly complete and directly relat… |
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
cleanup-desktop/electron-helpers
Comment @coderabbitai help to get the list of available commands.