Skip to content

fix(server): stop the startup project sync from delaying the app window - #14912

Open
Mnigos wants to merge 2 commits into
pingdotgg:mainfrom
Mnigos:startup-auto-pull-background
Open

Mnigos wants to merge 2 commits into
pingdotgg:mainfrom
Mnigos:startup-auto-pull-background

Conversation

@Mnigos

@Mnigos Mnigos commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Rebuilt on V2 from #12618, which was closed in the V2 transition with "If the change is still needed on V2, please rebuild it on current main, verify it there, and open a new PR linking back here". It is still needed: measured on main after #2829, automatic pull holds server readiness for about 10 seconds in the fixture below.

Problem

Desktop window creation waits for GET /.well-known/t3/environment, which sits behind the command readiness gate. On V2, serverRuntimeStartup.ts still awaits the projects.auto-pull phase before signalCommandReady, and every project with automatic pull enabled costs a git.statusDetails call that refreshes its upstream with a network git fetch (four at a time, five second timeout each). With slow or unreachable remotes the app looks dead until the last batch finishes. This is the projects.auto-pull half of #12617; that issue was closed as obsolete after V2, and its other half (the unindexed worktree-setups.reconcile scan) is indeed gone, but this phase came through the rewrite unchanged.

Change

The phase is forked with forkParked, the same way the heartbeat and auto-bootstrap roots already are. It still runs once per start and still logs its failures, but it starts at activation and no longer holds the gate. A side effect worth stating: a trial server during an update no longer pulls into checkouts before it commits.

Automatic pull is opt-in (defaultAutoPull is off by default), so this only changes startup for users who enabled it. Eligibility, concurrency and the fetch timeout are unchanged.

Verification

Observed result on V2. Running servers on macOS 15.7.5 (Apple M4 Pro, Node 24.14.1): main at cc1e634 and this branch, each started with node apps/server/src/bin.ts serve on a fresh base directory. Fixture: eight throwaway repositories registered with project add, each with its own local bare remote whose fetch takes 5 s (an uploadpack wrapper that sleeps). All providers disabled, no real remotes. Readiness is the time from process spawn to the first HTTP 200 from GET /.well-known/t3/environment, polled every 50 ms; three starts per cell.

main @ cc1e634 this branch
Automatic pull off 1.61, 1.52, 1.77 s, median 1.61 s 1.58, 2.02, 1.74 s, median 1.74 s
Automatic pull on, 8 projects, 5 s remotes 11.86, 11.82, 11.91 s, median 11.86 s 1.62, 1.61, 1.61 s, median 1.61 s

On both builds the fetches run in two batches of four, starting about 1.9 s and 6.8 s after spawn and ending about 6.7 s and 11.8 s after it; all of them end on the server's five second fetch timeout (the sleep plus git overhead exceeds it), with eight Background Git fetch failed warnings per start. On main readiness arrives after the second batch. On this branch it arrives before the first batch ends, while the same fetches finish in the background. The change does not make the fetches faster.

Tests. serverRuntimeStartup.autoPull.test.ts runs the real startup sequence with test layers and a statusDetails that never returns. It asserts that the sync has not started while activation is closed, that awaitCommandReady completes after activation while the git call is still blocked, and that the parked sync is interrupted on shutdown. With the upstream startup source substituted, it fails in milliseconds, with no timeout involved. 13 tests pass across the four startup test files; server typecheck, targeted lint, format and knip are clean.

Not checked: the Electron window itself (the measurement is the endpoint it waits for), real network latency, fetches that succeed slowly instead of timing out, and other project counts.

Field data. A contributor measured the same path on a Linux nightly desktop with 11 projects and defaultAutoPull: true (comment): the window took 4.6 s on average to appear, projects.auto-pull was 84–97 % of server.startup, and disabling auto-pull in a sandboxed copy cut time-to-ready from ~3.4 s to ~1.7 s. That covers the Electron window and real network remotes listed under Not checked.

Implemented with Claude Code (Claude Fable 5.1); rebase onto V2, tests and the measurement by GPT-6 Astra via Codex.

Mnigos added 2 commits October 3, 2026 00:01
Startup ran the automatic project pull before signalling command readiness.
Each enabled project costs a remote status fetch, so with a few dozen
repositories the HTTP readiness probe timed out for many seconds and the
desktop window stayed closed. Park the sync at activation like the other
background roots; it still runs once per start and still logs its failures.
Run the real startup sequence with a git status check that never returns:
the sync must not start before activation, and command readiness must not
wait for it afterwards. Reverting the parked fork fails the test at once.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 2, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 2, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f689114

Macroscope's review found this PR approvable — This is a narrowly scoped server startup fix that moves an existing opt-in project sync behind activation without changing its pull policy or defaults. The added test verifies that readiness remains independent of a blocked sync and that the background work is interrupted on shutdown.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 2, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 22:08

Dismissing prior approval to re-evaluate f689114

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a6160742-f491-45fc-ab42-9a216eda5a7f

📥 Commits

Reviewing files that changed from the base of the PR and between bf7121d and f689114.

📒 Files selected for processing (2)
  • apps/server/src/serverRuntimeStartup.autoPull.test.ts
  • apps/server/src/serverRuntimeStartup.ts

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


📝 Walkthrough

Walkthrough

Startup now forks the automatic-pull phase as parked work, so project listing, settings retrieval, and automatic pulls do not block startup. A test checks trial preparation and activation ordering, command readiness while Git status is pending, and interruption of that status effect.

Changes

Automatic-pull startup

Layer / File(s) Summary
Fork automatic-pull work and verify startup ordering
apps/server/src/serverRuntimeStartup.ts, apps/server/src/serverRuntimeStartup.autoPull.test.ts
The startup phase runs as parked work, with failures logged as warnings. The test verifies trial preparation before activation, Git status after activation, command readiness while status remains pending, and interruption of the status effect.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: 🔵 Low · up to f6891

Startup no longer waits for automatic pulls, but a narrowly timed branch switch could cause an unexpected, recoverable branch update. This is mergeable with owner awareness and follow-up.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f6891

The change reduces startup delays, but background pulls can overlap branch changes and update a branch that did not pass the automatic-pull eligibility checks. Exposure is limited to projects with automatic pull enabled; no increase in access privileges was established.

Retained concerns

  • Medium · reliability · inferred: Background automatic pull can overlap a branch switch after readiness. Eligibility is evaluated for the initially observed default branch, but pullCurrentBranch subsequently operates on the current checkout without preserving that identity or reapplying the full eligibility predicate. A switch between those steps can therefore cause an unintended fast-forward of another branch, or a failure. This weakens containment of automatic checkout mutations.
Security review details

Security Blast Radius

  • inferred — The timing concern affects saved project checkouts with automatic pull enabled and an overlapping mutation of the same checkout. Multiple enabled roots may be processed, but the inspected path does not establish cross-tenant access or newly acquired privileges. Git continues to run with the server process environment and the checkout’s configured upstream.

Security Findings and Attack Paths

  • inferred — A concurrent branch-switch request can invalidate the branch identity used by automatic-pull eligibility before the pull executes. The inspected RPC path requires existing session authority, so this is an unintended mutation path rather than an established unauthenticated exploit or privilege escalation.

Trust Boundaries and Controls

  • observed — Fast-forward-only pulling and branch/upstream validation constrain the Git operation. The eight-permit subprocess limit bounds concurrency, while mutation wrappers invalidate caches; neither provides exclusive ownership across eligibility checks and checkout mutation.

Resilience and Maintainability Implications

  • observed — Parked work is scoped, and per-root automatic-pull failures are logged so other roots can proceed. These mechanisms contain effect failures, but they do not make the multi-command pull atomic or undo mutations already completed before interruption.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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 describes the main change: preventing automatic project sync from delaying server startup.
Description check ✅ Passed The description covers the problem, change, scope context, verification results, unchecked areas, and implementation tools. It includes detailed test and measurement results.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@TonybynMp4

Copy link
Copy Markdown
Contributor

Adding data from a real setup in case it helps review. It covers two of the "Not checked" items: the Electron window itself, and real network remotes. Linux, nightly desktop, defaultAutoPull: true, 11 projects.

Over my last 10 desktop launches, the main window took 4.6s on average to appear (median 4.4s, range 3.5–6.7s). Almost all of that is desktop.backendProcess.probeReadiness waiting on /.well-known/t3/environment (avg 4.3s). The Electron shell itself is ready in about 250ms.

Server traces from the last three launches show where that time goes:

server.startup projects.auto-pull
2.73s 2.64s
3.58s 3.00s
2.74s 2.69s

Auto-pull is 84–97% of server startup, and it runs before signalCommandReady, so commandReadinessLayer holds the readiness probe for the whole time. In a sandboxed copy of my DB, setting defaultAutoPull: false cut time-to-ready from ~3.4s to ~1.7s. Forking the sync as this PR does should give the same result without turning the feature off.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants