Repository navigation
Conversation
…at 60 s Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — The desktop backend gets three minutes rather than one to become ready before timeout-based recovery reporting. Human review is required for this product-default change and its tradeoff between accommodating slow startup and delaying recovery feedback. No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Readiness window and timing tests apps/desktop/src/backend/DesktopBackendManager.ts, apps/desktop/src/backend/DesktopBackendManager.test.ts |
The default readiness timeout increases to three minutes. Tests verify that readiness after 66 seconds does not report a startup failure and that continued unavailability through 180 seconds reports a timeout. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related issues
pingdotgg/t3code#23372describes a desktop backend that remains unready during startup recovery. This change extends the readiness window but does not address startup that exceeds three minutes.pingdotgg/t3code#26200reports a WSL backend readiness timeout. This change extends the same readiness window, but does not address the separate port-release race.pingdotgg/t3code#23877reports readiness timeouts during slow cleanup and startup. This change extends the timeout but does not address the reported post-start issues.
Possibly related PRs
- pingdotgg/t3code#21780: Adds desktop startup behavior that waits for backend readiness; this PR changes the readiness timeout used by that flow.
- pingdotgg/t3code#25903: Reports backend startup failures through recovery when the backend misses readiness; this PR changes when that failure can occur.
- pingdotgg/t3code#26063: Handles WSL startup failures tied to backend readiness; this PR extends the desktop manager’s default readiness window.
Suggested reviewers: t3dotgg, juliusmarminge, macroscopeapp
Merge Risk: 
Minimal · up to 21cef
-
The longer startup window allows slow backends more time to become ready. No merge-blocking risk remains.
Pre-merge checks: 
4
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: extending desktop backend startup time before showing recovery. Linked Issues check Issue #26237asks that a backend that becomes ready shortly after the fixed 60 s readiness budget not be reported as a failed start. It allows either a larger budget or a late success that replaces th…Out of Scope Changes check The reviewed diff between merge base df4132b and head 21cefe8 changes only two files. DesktopBackendManager.ts changes one constant, a… Description check The description is detailed and covers the problem, implementation, scope, linked issue, verification results, evidence, limitations, and test outcomes. It uses “Evidence” and “Validation” instead of … Full details: Linked Issues check
-
Explanation
Issue
#26237asks that a backend that becomes ready shortly after the fixed 60 s readiness budget not be reported as a failed start. It allows either a larger budget or a late success that replaces the failure state. The change at head sets DEFAULT_BACKEND_READINESS_TIMEOUT from Duration.minutes(1) to Duration.minutes(3) in apps/desktop/src/backend/DesktopBackendManager.ts. The new test in DesktopBackendManager.test.ts covers the reported case: a backend that returns 503 until 66 s and then returns 200 produces no startup failure report. The same test shows a never-ready backend produces no report at 179 s and the failure report at 180 s. Per-request timeout, probe interval, and crash-loop handling are unchanged in the diff. The issue's stated database startup cost is not reduced. The issue does not require that. Related reports#26200and#24895are contextual and are not coding requirements of this PR. Non-coding verification (macOS fixture, recordings) does not affect this verdict.
Full details: Out of Scope Changes check
-
Explanation
The reviewed diff between merge base df4132b and head 21cefe8 changes only two files. DesktopBackendManager.ts changes one constant, and adds a comment that explains the new value. DesktopBackendManager.test.ts adds one regression test for readiness timing at 66 s, 179 s, and 180 s. Both changes directly implement the readiness-budget objective in
#26237. No unrelated code, configuration, or dependency changes appear in the reviewed diff. The PR description references images and recordings, which are not repository changes.
Full details: Description check
-
Explanation
The description is detailed and covers the problem, implementation, scope, linked issue, verification results, evidence, limitations, and test outcomes. It uses “Evidence” and “Validation” instead of the template headings and does not explicitly quote maintainer approval, but the required information is otherwise substantially present.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
…pstream pingdotgg#26247) Cherry-picked from pingdotgg#26247 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A managed desktop backend can become ready just after the 60-second limit. The app opens "T3 Code could not start" even though that same process goes on to serve the app seconds later.
Give startup three minutes before reporting a readiness failure. Successful probes still open the app immediately, and crash-loop recovery still triggers on the third restart. A stuck server takes two extra minutes to show recovery diagnostics. This changes the wait, not database startup performance.
The reporter used macOS arm64 with a local managed backend. The corex recording exercises the same
probeReadinessandDEFAULT_BACKEND_READINESS_TIMEOUTinapps/desktop/src/backend/DesktopBackendManager.ts, which has no macOS branch. Windows and WSL use this manager too. Web and mobile clients do not launch this backend.Closes #26237
Evidence
Current head
d3febc3cf443ddad7a8076b9ea538adf9249b20emerges main into the recorded head.apps/desktop/src/backend/DesktopBackendManager.tsand its test are byte-for-byte unchanged from21cefe874af40566bf0ab3c1b49c7af3a769613a. The readiness and recovery flow shown below is unchanged, so these recordings still cover the fix. CI on the current head reran all 9 manager tests successfully.Packaged Linux x64 desktop on physical corex, version 0.1.3, Electron 44.9.3. Before is full main
dac8b11ddc25bd5da62a367fdb6c3de777ae2986; after is recorded PR head21cefe874af40566bf0ab3c1b49c7af3a769613a.These are release AppImages built with
vp run dist:desktop:artifact --platform linux --target AppImage --arch x64, with source maps disabled. Each run has a separate disposable T3 home and Chromium profile. The packaged code and assets are unchanged. The test pauses the real managed server child for 70 seconds with SIGSTOP, then resumes that same PID with SIGCONT. Readiness probes, authentication, and the client all run normally. No development server or mocked readiness endpoint is involved.Main showed recovery at 63.411 seconds, then that same child became ready and the app connected at about 76 seconds. The PR head became ready at 79.339 seconds without reporting failure or showing recovery. These are elapsed capture times, not a startup performance comparison.
Before:
before.mp4
After:
after.mp4
The continuous recordings include launch, the wait past 60 seconds, and the connected app. The terminal at the top is the test console with build and process observations. The original 22 GB database workload was not recreated.
Validation
DesktopBackendManager.test.tstests passed on corex. The regression covers readiness at 66 seconds without failure, no failure at 179 seconds for a stuck backend, and failure reporting at 180 seconds. Existing crash-loop coverage passes.vp run check:changedpassed at this head before publication. Required GitHub checks pass, with path-filtered jobs skipped.Build and runtime output
Both builds passed on Boat, attempt 1, default machine,
t3codetemplate, exit 0:j_5rj2rt4p4olftj2kgmak, VMbx_3c25fru6, commitdac8b11ddc25bd5da62a367fdb6c3de777ae2986.j_e3pach3iilhnur6wooqg, VMbx_xjdspeb5, commit21cefe874af40566bf0ab3c1b49c7af3a769613a.AppImage SHA-256 values were verified after download on corex:
Each recording uses a dedicated Xvfb display on corex and an isolated home, with
--password-store=basic. No other desktop windows or account state are present. Main's recording is 113.13 seconds; the head's is 116.13 seconds. Both are continuous H.264 MP4s at 1280 × 960, 15 fps.Focused test output at recorded head
21cefe874af40566bf0ab3c1b49c7af3a769613a:Implemented by Claude Opus 5.5 through Claude Code. Reviewed and verified by GPT-6.1-Sol through the Codex harness in T3 Code.