Skip to content

refactor(desktop): classify app exports - #10267

Merged
juliusmarminge merged 1 commit into
cleanup-desktop/electron-helpersfrom
cleanup-desktop/app-shell-helpers
Sep 7, 2026
Merged

refactor(desktop): classify app exports#10267
juliusmarminge merged 1 commit into
cleanup-desktop/electron-helpersfrom
cleanup-desktop/app-shell-helpers

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 6, 2026

Copy link
Copy Markdown
Member

Desktop app, window, shell, SSH, and update modules had 18 unclassified runtime exports. This marks 14 canonical Effect service constructors as intentional public APIs and keeps four implementation constants and state helpers private without changing runtime behavior.

This is layer 3 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 desktop app helpers private and document public service APIs

  • Removes the export keyword from internal helpers that are only used within their own module: URL_HANDLER_DESKTOP_ENTRY_NAME in DesktopLinuxUrlHandler.ts, isInstallableDesktopUpdateState in remoteUpdateFlow.ts, and nextStatusAfterDownloadFailure and getCanRetryAfterDownloadFailure in updateMachine.ts.
  • Adds public API documentation comments to every exported service-construction effect and service value across the desktop app modules, clarifying the intended public surface.
  • Behavioral Change: the four helpers above are no longer importable from outside their modules; any external consumers will need to be updated.

Macroscope summarized 4c6bb9a.

Summary by CodeRabbit

  • Documentation

    • Improved public API documentation for desktop application services and constructors.
  • Refactor

    • Reduced the public API surface by keeping several internal update helpers private.
    • No runtime behavior changes.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline 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.6 KiB 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.3 KiB
Claude Live turn WebSocket wire 6.6 KiB 7.8 KiB
Claude Live turn WebSocket decoded 57.9 KiB 66.4 KiB
Claude Live turn messages 10 21

Baseline: unavailable · PR result: 4c6bb9a · 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.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 6, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 227925c

Macroscope's review found this PR approvable — This is a contained export-surface cleanup: existing service constructors receive documentation, while four unused helper exports become module-private. Runtime logic, defaults, authentication behavior, and production workflows remain unchanged.

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

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

@juliusmarminge
juliusmarminge force-pushed the cleanup-desktop/app-shell-helpers branch from 0251350 to 227925c Compare September 6, 2026 04:15
@cursor

cursor Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@macroscopeapp
macroscopeapp Bot dismissed their stale review September 6, 2026 04:15

Dismissing prior approval to re-evaluate 227925c

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Sep 6, 2026
@juliusmarminge juliusmarminge changed the title refactor(desktop): keep app helpers private refactor(desktop): classify app exports Sep 6, 2026
@juliusmarminge
juliusmarminge force-pushed the cleanup-desktop/app-shell-helpers branch from 227925c to 4c6bb9a Compare September 7, 2026 21:55
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

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: a8a8fcd8-a5f1-4c34-ad8e-e21e746f802e

📥 Commits

Reviewing files that changed from the base of the PR and between bf20c62 and 4c6bb9a.

📒 Files selected for processing (16)
  • apps/desktop/src/app/DesktopAppActivation.ts
  • apps/desktop/src/app/DesktopAppIdentity.ts
  • apps/desktop/src/app/DesktopAssets.ts
  • apps/desktop/src/app/DesktopClerk.ts
  • apps/desktop/src/app/DesktopConnectionCatalogStore.ts
  • apps/desktop/src/app/DesktopLifecycle.ts
  • apps/desktop/src/app/DesktopLinuxUrlHandler.ts
  • apps/desktop/src/app/DesktopPreReadyPlatform.ts
  • apps/desktop/src/shell/DesktopShellEnvironment.ts
  • apps/desktop/src/ssh/DesktopSshEnvironment.ts
  • apps/desktop/src/ssh/DesktopSshPasswordPrompts.ts
  • apps/desktop/src/updates/DesktopUpdates.ts
  • apps/desktop/src/updates/remoteUpdateFlow.ts
  • apps/desktop/src/updates/updateMachine.ts
  • apps/desktop/src/window/DesktopApplicationMenu.ts
  • apps/desktop/src/window/DesktopWindow.ts

Included review availability: 4 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 changes document desktop service constructors as public Effect APIs and remove exports from selected internal constants and update helpers. Runtime behavior remains unchanged.

Changes

Desktop Effect API surface

Layer / File(s) Summary
Document public service constructors
apps/desktop/src/app/*, apps/desktop/src/shell/*, apps/desktop/src/ssh/*, apps/desktop/src/updates/DesktopUpdates.ts, apps/desktop/src/window/*
Added public API documentation comments to exported make service constructors.
Restrict internal exports
apps/desktop/src/app/DesktopLinuxUrlHandler.ts, apps/desktop/src/updates/remoteUpdateFlow.ts, apps/desktop/src/updates/updateMachine.ts
Made URL_HANDLER_DESKTOP_ENTRY_NAME, isInstallableDesktopUpdateState, nextStatusAfterDownloadFailure, and getCanRetryAfterDownloadFailure module-private without changing their behavior.

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

Merge Risk: ⚪ Minimal · up to 4c6bb

This change clarifies the desktop Effect API surface and makes four internal helpers unavailable to external imports without changing runtime behavior. No current merge-blocking risk remains.

Suggested reviewers: t3dotgg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 16 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 and concisely describes the main change: classifying desktop application exports.
Description check ✅ Passed The description clearly explains the export changes, rationale, scope, verification, and the unavailable libsecret test. It does not use the template headings or checklist, but it is otherwise complet…
  • 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/app-shell-helpers

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

@macroscopeapp macroscopeapp 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.

All clear

Posted via Macroscope — Effect Service Conventions

@juliusmarminge
juliusmarminge merged commit 89dd9ab into main Sep 7, 2026
32 of 43 checks passed
@juliusmarminge
juliusmarminge deleted the cleanup-desktop/app-shell-helpers branch September 7, 2026 22:00
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 8, 2026
## What's Changed
* fix(mobile): keep pending messages in the chat timeline by @juliusmarminge in pingdotgg/t3code#10449
* fix(mobile): show connection status in the floating pill instead of a second one by @juliusmarminge in pingdotgg/t3code#10440
* fix: use Pierre icons consistently for attachments by @juliusmarminge in pingdotgg/t3code#10475
* feat(mobile): open the thread screen as soon as a new task is submitted by @juliusmarminge in pingdotgg/t3code#10435
* fix(devcontainer): make repository setup work by @saphid in pingdotgg/t3code#7875
* fix(projects): prevent invalid script IDs from crashing threads by @saphid in pingdotgg/t3code#10019
* fix(mobile): hide changed-files navigator and restore refresh in raw diff fallback by @lnieuwenhuis in pingdotgg/t3code#9828
* fix(ios): scroll short source files from blank space by @juliusmarminge in pingdotgg/t3code#10178
* feat(mobile): start a new thread on an existing branch by @StiensWout in pingdotgg/t3code#10359
* fix(mobile): improve font-size slider performance and prevent maximum update depth errors by @bbernag in pingdotgg/t3code#7138
* fix(web): keep composer toolbar controls anchored during transitions by @juliusmarminge in pingdotgg/t3code#10478
* fix(web): resize the floating preview from any edge by @juliusmarminge in pingdotgg/t3code#10467
* fix(mobile): prevent chat from disappearing when scrolling by @juliusmarminge in pingdotgg/t3code#10479
* fix(mobile): smooth composer status pill resizing by @juliusmarminge in pingdotgg/t3code#10484
* fix(mobile): release initial scroll target after dragging by @juliusmarminge in pingdotgg/t3code#10483
* fix(mobile): animate thread lifecycle transitions consistently by @juliusmarminge in pingdotgg/t3code#10487
* fix(mobile): restore assistant message bottom padding by @juliusmarminge in pingdotgg/t3code#10491
* fix(mobile): preserve chat rows when toggling commands by @juliusmarminge in pingdotgg/t3code#10492
* feat(web): group onboarding project import by repository by @t3dotgg in pingdotgg/t3code#10493
* fix(web): remove inserted citations on cancel by @extoci in pingdotgg/t3code#10518
* fix(mobile): match Working status color to desktop by @baptisteArno in pingdotgg/t3code#10515
* fix(mobile): wait for native thread scroll before reveal by @juliusmarminge in pingdotgg/t3code#10486
* fix(web): tolerate servers that predate git identity in project import by @juliusmarminge in pingdotgg/t3code#10547
* fix(web): restore settled PR colors on hover by @flamboh in pingdotgg/t3code#10023
* fix(web): keep popup triggers steady when pressed by @dominic-r in pingdotgg/t3code#10468
* fix(web): keep project favicon shape consistent across sizes by @sameerr03 in pingdotgg/t3code#10502
* fix(claude): report usage limits on retried turns by @StiensWout in pingdotgg/t3code#10549
* refactor(desktop): classify backend exports by @juliusmarminge in pingdotgg/t3code#10265
* refactor(desktop): classify electron exports by @juliusmarminge in pingdotgg/t3code#10266
* refactor(desktop): classify app exports by @juliusmarminge in pingdotgg/t3code#10267
* refactor(desktop): classify preview exports by @juliusmarminge in pingdotgg/t3code#10268
* ci(knip): enforce desktop exports by @juliusmarminge in pingdotgg/t3code#10269
* fix(web): remove excess sidebar thread spacing by @maria-rcks in pingdotgg/t3code#10569
* fix(web): make settings project scopes searchable and scrollable by @maria-rcks in pingdotgg/t3code#10570
* fix(web): use `tabular-nums` with the ui font for sidebar timer by @extoci in pingdotgg/t3code#10592
* fix(web): correct pending question attachment message by @dominic-r in pingdotgg/t3code#10599
* fix(mobile): restore brand artwork in the Android adaptive icon by @juliusmarminge in pingdotgg/t3code#10598
* fix(mobile): configure iOS Keychain access group by @VedankPurohit in pingdotgg/t3code#3665
* fix(web): remember Composer Fast mode across new chats by @aravhawk in pingdotgg/t3code#2981
* fix(server): disable executable capabilities in Claude metadata generation by @maxwellyoung in pingdotgg/t3code#4169
* Add stop thread keybinding command by @jakeleventhal in pingdotgg/t3code#4308

## New Contributors
* @bbernag made their first contribution in pingdotgg/t3code#7138
* @baptisteArno made their first contribution in pingdotgg/t3code#10515
* @aravhawk made their first contribution in pingdotgg/t3code#2981

**Full Changelog**: pingdotgg/t3code@v0.0.39...v0.0.40

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.40
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 8, 2026
## What's Changed
* fix(mobile): wait for native thread scroll before reveal by @juliusmarminge in pingdotgg/t3code#10486
* fix(web): tolerate servers that predate git identity in project import by @juliusmarminge in pingdotgg/t3code#10547
* fix(web): restore settled PR colors on hover by @flamboh in pingdotgg/t3code#10023
* fix(web): keep popup triggers steady when pressed by @dominic-r in pingdotgg/t3code#10468
* fix(web): keep project favicon shape consistent across sizes by @sameerr03 in pingdotgg/t3code#10502
* fix(claude): report usage limits on retried turns by @StiensWout in pingdotgg/t3code#10549
* refactor(desktop): classify backend exports by @juliusmarminge in pingdotgg/t3code#10265
* refactor(desktop): classify electron exports by @juliusmarminge in pingdotgg/t3code#10266
* refactor(desktop): classify app exports by @juliusmarminge in pingdotgg/t3code#10267
* refactor(desktop): classify preview exports by @juliusmarminge in pingdotgg/t3code#10268
* ci(knip): enforce desktop exports by @juliusmarminge in pingdotgg/t3code#10269
* fix(web): remove excess sidebar thread spacing by @maria-rcks in pingdotgg/t3code#10569
* fix(web): make settings project scopes searchable and scrollable by @maria-rcks in pingdotgg/t3code#10570
* fix(web): use `tabular-nums` with the ui font for sidebar timer by @extoci in pingdotgg/t3code#10592
* fix(web): correct pending question attachment message by @dominic-r in pingdotgg/t3code#10599
* fix(mobile): restore brand artwork in the Android adaptive icon by @juliusmarminge in pingdotgg/t3code#10598
* fix(mobile): configure iOS Keychain access group by @VedankPurohit in pingdotgg/t3code#3665
* fix(web): remember Composer Fast mode across new chats by @aravhawk in pingdotgg/t3code#2981
* fix(server): disable executable capabilities in Claude metadata generation by @maxwellyoung in pingdotgg/t3code#4169
* Add stop thread keybinding command by @jakeleventhal in pingdotgg/t3code#4308

## New Contributors
* @aravhawk made their first contribution in pingdotgg/t3code#2981

**Full Changelog**: pingdotgg/t3code@v0.0.40-nightly.20260907.1359...v0.0.40-nightly.20260907.1372

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.40-nightly.20260907.1372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant