Sign up
View stargazers13Sign up

refactor(desktop): classify backend exports#10265

merged1
main
cleanup-desktop/runtime-core
openedSep 6, 2026byJJulius Marminge
10 files changed
View on GitHubTwo-way sync enabledPR last imported 5d ago

Desktop backend, settings, telemetry, WSL, and launcher modules had 15 unclassified runtime exports. This marks eight canonical Effect service constructors as intentional public APIs, narrows seven implementation exports, and removes an unused 53-line saved-environments test layer. Exported types and schemas remain unchanged.

This is layer 1 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

Tighten desktop backend export surface and document service constructors

  • Removes named exports for constants and helpers in electron-launcher.mjs, DesktopServerExposure.ts, tailscaleEndpointProvider.ts, and DesktopAppSettings.ts, keeping their internal definitions unchanged.
  • Adds API doc comments to the .make service construction values across the desktop backend modules.
  • Removes the in-memory DesktopSavedEnvironments.layerTest test layer and its mutable test-state implementations.
  • Risk: DesktopSavedEnvironments.layerTest removal may break out-of-tree tests that import it; in-tree tests should be updated accordingly.

Macroscope summarized fe8fffa.

Summary by CodeRabbit

  • Documentation

    • Added public API documentation across desktop backend, settings, telemetry, and WSL modules.

  • Refactor

    • Reduced the public API surface by keeping implementation constants and internal helpers private.
    • Removed an internal test-only environment layer.
    • Limited endpoint utility re-exports to the supported parsing functionality.
Ggithub-actionsBotSep 6, 2026
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.6 KiB13.6 KiB+2 B (+0.0%)15.1 KiB✅
CodexThread snapshot wire7.0 KiB7.0 KiB−7 B (−0.1%)7.3 KiB✅
CodexLive turn WebSocket wire6.6 KiB6.6 KiB+9 B (+0.1%)7.8 KiB✅
CodexLive turn WebSocket decoded57.1 KiB57.1 KiB0 B (0.0%)66.4 KiB✅
CodexLive turn messages10100 (0.0%)21✅
ClaudeTotal thread wire13.6 KiB13.6 KiB−19 B (−0.1%)15.1 KiB✅
ClaudeThread snapshot wire7.0 KiB7.0 KiB+2 B (+0.0%)7.3 KiB✅
ClaudeLive turn WebSocket wire6.6 KiB6.5 KiB−21 B (−0.3%)7.8 KiB✅
ClaudeLive turn WebSocket decoded57.9 KiB57.9 KiB0 B (0.0%)66.4 KiB✅
ClaudeLive turn messages10100 (0.0%)21✅

Baseline: 0d34579 · PR result: fe8fffa · 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.

MmacroscopeappBotSep 6, 2026
Synced from GitHub

Approvability

Verdict: Approved at d12e01b

Macroscope's review found this PR approvable — This is a contained cleanup of internal exports and Effect API annotations; runtime logic and the existing product defaults remain unchanged. The removed code is an unused test-only layer, with no production request path or user-facing behavior affected.

No code changes detected at fe8fffa. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

MmacroscopeappBotreviewed · 1 inline threadSep 6, 2026
Synced from GitHub

Cross-cutting: the same export const make → const make change is applied to eight service modules that own their construction (DesktopBackendConfiguration.ts:740, DesktopLocalEnvironmentAuth.ts:45, DesktopNetworkInterfaces.ts:42, DesktopServerExposure.ts:414, DesktopAppSettings.ts:453, DesktopSavedEnvironments.ts:367, DesktopTelemetryPublisher.ts:148, DesktopWslServerTree.ts:119). The inline note on the first occurrence applies to all eight; restoring export on each keeps the canonical module shape consistent. The un-exported constants (DESKTOP_LOOPBACK_HOST, DEFAULT_TAILSCALE_SERVE_PORT) and the removed unused layerTest are not service-shape exports and are fine as-is.

Posted via Macroscope — Effect Service Conventions

apps/desktop/src/backend/DesktopBackendConfiguration.tsline 740Outdated · b6306d8Open thread
……@@ -737,7 +737,7 @@ const resolveWslStartConfig = Effect.fn("desktop.backendConfiguration.resolveWsl
738738});
739739
740-export const make = Effect.gen(function* () {
740+const make = Effect.gen(function* () {
MmacroscopeappBoton line 740Sep 6, 2026

backend/DesktopBackendConfiguration.ts:740

This module owns construction of the service, so make is part of its public shape (DesktopBackendConfiguration.make alongside .layer) — the convention is to export a real make in that case, even when only layer is consumed today (cf. packages/client-runtime/src/connection/driver.ts, which documents the exported make as "part of the canonical Effect module API"). Suggest keeping the export.

suggestion
1export const make = Effect.gen(function* () {

No longer relevant as of d12e01b8c6b22163a4f80b577c1e6ed8f6c4469e

Added 1 commit

  • refactor(desktop): keep backend helpers privatefe8fffa
CcoderabbitaiBotSep 7, 2026
Synced from GitHub

Review Change Stack

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: 84d83483-d538-4c71-ab5a-e48cef53c740

📥 Commits

Reviewing files that changed from the base of the PR and between 0d34579d674920cc47fc5c908494f51ed3895204 and fe8fffac36e33a6872351f261b8d21745b409df2.

📒 Files selected for processing (10)
  • apps/desktop/scripts/electron-launcher.mjs
  • apps/desktop/src/backend/DesktopBackendConfiguration.ts
  • apps/desktop/src/backend/DesktopLocalEnvironmentAuth.ts
  • apps/desktop/src/backend/DesktopNetworkInterfaces.ts
  • apps/desktop/src/backend/DesktopServerExposure.ts
  • apps/desktop/src/backend/tailscaleEndpointProvider.ts
  • apps/desktop/src/settings/DesktopAppSettings.ts
  • apps/desktop/src/settings/DesktopSavedEnvironments.ts
  • apps/desktop/src/telemetry/DesktopTelemetryPublisher.ts
  • apps/desktop/src/wsl/DesktopWslServerTree.ts

Included review availability: 5 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 narrows desktop module exports, adds public API annotations to service constructors, and removes the exported in-memory test layer from saved environments.

Changes

Desktop API cleanup

Layer / File(s)Summary
Restrict internal exports
apps/desktop/scripts/electron-launcher.mjs, apps/desktop/src/backend/DesktopServerExposure.ts, apps/desktop/src/backend/tailscaleEndpointProvider.ts, apps/desktop/src/settings/DesktopAppSettings.ts		Selected constants, resolveElectronPath, and isTailscaleIpv4Address are no longer publicly exported.
Document service construction APIs
apps/desktop/src/backend/*.ts, apps/desktop/src/settings/DesktopAppSettings.ts, apps/desktop/src/telemetry/DesktopTelemetryPublisher.ts, apps/desktop/src/wsl/DesktopWslServerTree.ts		Public documentation annotations identify the canonical service construction APIs.
Remove the saved-environment test layer
apps/desktop/src/settings/DesktopSavedEnvironments.ts		The exported layerTest implementation and unused Ref import are removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to fe8ff

This change narrows desktop implementation exports, documents supported service constructors, and removes an unused test helper without changing production behavior. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.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 nameStatusExplanation
Title check✅ PassedThe title is concise and accurately describes the main change: classifying and narrowing desktop backend exports.
Description check✅ PassedThe description clearly explains the export changes, rationale, scope, verification, and known test limitation. It omits the template headings and checklist, but the required information is mostly pre…
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • 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/runtime-core

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