Conversation
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.
ApprovabilityVerdict: Approved at 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:
You can add or adjust custom eligibility rules. Learn more. |
Dismissing prior approval to re-evaluate f689114
|
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 configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughStartup 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. ChangesAutomatic-pull startup
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: 🔵 Low · up to 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 ReviewSecurity architecture risk: 🔵 Low · up to 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
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
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, 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 Server traces from the last three launches show where that time goes:
Auto-pull is 84–97% of server startup, and it runs before |
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
mainafter #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.tsstill awaits theprojects.auto-pullphase beforesignalCommandReady, and every project with automatic pull enabled costs agit.statusDetailscall that refreshes its upstream with a networkgit 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 theprojects.auto-pullhalf of #12617; that issue was closed as obsolete after V2, and its other half (the unindexedworktree-setups.reconcilescan) 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 (
defaultAutoPullis 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):
mainat cc1e634 and this branch, each started withnode apps/server/src/bin.ts serveon a fresh base directory. Fixture: eight throwaway repositories registered withproject add, each with its own local bare remote whose fetch takes 5 s (anuploadpackwrapper that sleeps). All providers disabled, no real remotes. Readiness is the time from process spawn to the first HTTP 200 fromGET /.well-known/t3/environment, polled every 50 ms; three starts per cell.main@ cc1e634On 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 failedwarnings per start. Onmainreadiness 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.tsruns the real startup sequence with test layers and astatusDetailsthat never returns. It asserts that the sync has not started while activation is closed, thatawaitCommandReadycompletes 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-pullwas 84–97 % ofserver.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.