Sign up
View stargazersSign up

refactor(server): classify service exports#10276

merged1
cleanup-server/orchestration-internals
cleanup-server/service-internals
openedSep 6, 2026byJJulius Marminge
20 files changed
View on GitHubTwo-way sync enabledPR last imported 7d ago

Server service modules had 21 unclassified runtime exports. This marks 18 canonical Effect construction APIs as intentionally public, keeps two implementation helpers private, and removes one unused default text-generation policy.

This is layer 3 of 9 in the server Knip cleanup stack.

Verification after rebasing onto current main: server typecheck and the server-only Knip export audit pass. Changed-file lint and formatting pass, with one existing lint warning. Focused auth, provider, source-control, preview toolkit, orchestration, and Knip preprocessor tests pass: 90 tests across 9 files. This changes server export visibility and static analysis; screenshots do not apply.

Model: gpt-6 astra. Harness: Codex in T3 Code.

Summary by CodeRabbit

  • Documentation

    • Improved public API documentation for service construction across multiple server capabilities.
    • Clarified which service constructors are part of the supported canonical API.

  • API Changes

    • Several previously exported internal helper functions are now private.
    • Removed the default text-generation policy export; other text-generation policy options remain available.
Ggithub-actionsBotSep 6, 2026
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—13.6 KiB—15.1 KiB✅
CodexThread snapshot wire—7.0 KiB—7.3 KiB✅
CodexLive turn WebSocket wire—6.5 KiB—7.8 KiB✅
CodexLive turn WebSocket decoded—57.0 KiB—66.4 KiB✅
CodexLive turn messages—8—21✅
ClaudeTotal thread wire—13.6 KiB—15.1 KiB✅
ClaudeThread snapshot wire—7.0 KiB—7.3 KiB✅
ClaudeLive turn WebSocket wire—6.6 KiB—7.8 KiB✅
ClaudeLive turn WebSocket decoded—57.9 KiB—66.4 KiB✅
ClaudeLive turn messages—10—21✅

Baseline: unavailable · PR result: 4f890a2 · 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 fa407b5

Macroscope's review found this PR approvable — This is a focused server-internal export cleanup: service factories are documented, two helpers become private, and an unused policy export is removed. No active runtime behavior or product default changes were found; only unsupported out-of-tree imports of the removed names could be affected.

No code changes detected at 4f890a2. Prior analysis still applies.

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

Added 1 commit

  • refactor(server): keep service helpers private4f890a2
CcoderabbitaiBotSep 8, 2026
Synced from GitHub

Review Change StackReview 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: Advanced

Run ID: 25c3fa54-9086-4aee-a13d-d3ec2abc2b4e

📥 Commits

Reviewing files that changed from the base of the PR and between 1c4dfeec34410f080a13c3201fd45494fea2350b and 4f890a2689834d586693264eb984b6392359c77d.

📒 Files selected for processing (20)
  • apps/server/src/assets/NativeAppIconResolver.ts
  • apps/server/src/desktopUpdate/DesktopAppUpdate.ts
  • apps/server/src/git/GitWorkflowService.ts
  • apps/server/src/observability/BrowserTraceCollector.ts
  • apps/server/src/preview/Manager.ts
  • apps/server/src/preview/PortScanner.ts
  • apps/server/src/project/AgentSessionScanner.ts
  • apps/server/src/project/ProjectSetupScriptRunner.ts
  • apps/server/src/project/T3ProjectFileLoader.ts
  • apps/server/src/relay/AgentAwarenessRelay.ts
  • apps/server/src/review/ReviewService.ts
  • apps/server/src/terminal/Manager.ts
  • apps/server/src/textGeneration/TextGeneration.ts
  • apps/server/src/textGeneration/TextGenerationPresets.ts
  • apps/server/src/vcs/VcsProjectConfig.ts
  • apps/server/src/vcs/VcsProvisioningService.ts
  • apps/server/src/vcs/VcsStatusBroadcaster.ts
  • apps/server/src/workspace/WorkspaceEntries.ts
  • apps/server/src/workspace/WorkspaceFileSystem.ts
  • apps/server/src/workspace/WorkspaceSearchIndex.ts
💤 Files with no reviewable changes (1)
  • apps/server/src/textGeneration/TextGenerationPresets.ts

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

📝 Walkthrough

Walkthrough

The change documents Effect service constructors as public API, makes two helper functions module-private, and removes the defaultTextGenerationPolicy export.

Changes

Effect API surface cleanup

Layer / File(s)Summary
Internal helper visibility
apps/server/src/desktopUpdate/DesktopAppUpdate.ts, apps/server/src/relay/AgentAwarenessRelay.ts		desktopUpdateProgressStage and describeThreadShellForAwareness are no longer exported.
Public service annotations
apps/server/src/{assets,git,observability,preview,project,review,terminal,textGeneration,vcs,workspace}/*		Service construction declarations now include @public documentation identifying the canonical Effect module API.
Text generation preset exports
apps/server/src/textGeneration/TextGenerationPresets.ts		The defaultTextGenerationPolicy export was removed.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 4f890

This cleanup documents canonical service constructors and hides internal helpers without changing runtime behavior. No current merge-blocking risk is identified.

Suggested reviewers: t3dotgg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 19 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
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.
Title check✅ PassedThe title clearly and concisely summarizes the main change: classifying server service exports.
Description check✅ PassedThe description clearly explains the export classification, cleanup scope, verification results, and why UI evidence does not apply. It does not use the template headings or include the checklist, but…
  • 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-server/service-internals

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