Skip to content

fix(server): t3 project commands wait for a busy server's reply - #24090

Open
only21mil wants to merge 2 commits into
pingdotgg:mainfrom
only21mil:r13/b157-give-project-cli-dispatch-a-longer-live-
Open

only21mil wants to merge 2 commits into
pingdotgg:mainfrom
only21mil:r13/b157-give-project-cli-dispatch-a-longer-live-

Conversation

@only21mil

Copy link
Copy Markdown
Contributor

Problem

t3 project add, rename and remove talk to the running server when there is one. One 1 s timeout (PROJECT_CLI_LIVE_SERVER_TIMEOUT) covers three requests: the probe that picks live or offline mode, the snapshot read after it, and the mutation itself.

The probe needs a short timeout. The other two don't. By the time they run, the server has already answered. If it then takes more than a second to commit and reply, for example while it is busy with other work, the CLI prints "Failed to call the running server." The server may still commit the mutation, so the user can get an error for a project that was added.

Repro on main: the new test points the CLI at a live server whose mutate reply arrives after 1 s. The command fails with ProjectLiveServerRequestError, caused by TimeoutError: Operation timed out after '1s'.

Change

apps/server/src/cli/project.ts now has two timeouts:

  • PROJECT_CLI_LIVE_SERVER_PROBE_TIMEOUT, 1 s, used only by the probe in tryResolveLiveProjectExecutionMode.
  • PROJECT_CLI_LIVE_SERVER_REQUEST_TIMEOUT, 10 s, used for the snapshot read and the mutation once the probe has succeeded.

fetchLiveOrchestrationSnapshot takes the timeout as an argument, since it serves both the probe and the live read. Error mapping is unchanged: a timeout still becomes ProjectLiveServerRequestError.

10 s is a judgment call. It is long enough for a loaded server to commit one project command and short enough that a hung server doesn't leave the CLI waiting. t3 connect uses 5 s for its own live-server calls. Either value fixes the 1 s case.

Scope and approval

There's no open issue. #7504 reported that any failed 1 s probe was treated as "no server". #16102 fixed the dangerous half of that: offline commands now take the server's ownership lock, so they can't write behind a live server. The timeout itself was left as it was. This PR changes only the part that still misreports a successful live change. Server behavior, the probe and the offline path are unchanged. No app screen changes. The only visible difference is the CLI's exit status and output when the server is slow.

Verification

New test in apps/server/src/cli/project.test.ts, "waits for a busy live server to apply a project change". It writes a server-runtime.json for a live server and runs the real t3 project add command. A stand-in fetch answers the probe and snapshot at once but holds the mutate reply. Once the mutate request arrives, the test advances Effect's test clock by 1 s, then sends the reply. It checks that the command succeeds and that nothing was written to the offline database. It doesn't wait on the real clock.

cd apps/server
vp test run --reporter=verbose src/cli/project.test.ts
  • On main (05696c6) with the new test: 1 failed, ProjectLiveServerRequestError caused by TimeoutError: Operation timed out after '1s'. The other 12 passed.
  • On this branch: 13 passed.

Before, on main with this branch's test: the new case fails with a 1 s timeout.

Vitest on main with this branch's test: the new case fails with a 1s timeout

After, on this branch: all 13 project CLI tests pass, including the new case.

Vitest on this branch: all 13 project CLI tests pass, including the new case

Other checks on this branch:

  • vp lint --report-unused-disable-directives and vp fmt --check on both changed files: 0 warnings, 0 errors, formatted.
  • tsc-rs --noEmit in apps/server: exit 0.
  • The same test, lint, fmt and typecheck on a local merge of this branch into a newer green main (5ffe045), which has the newer test lint rules: all pass.

Not checked:

  • A real t3 serve slowed past 1 s. The test runs the real CLI path with only fetch replaced.
  • A mutation that takes longer than 10 s still reports failure even if it lands. This PR only moves that limit.
  • A probe that times out still sends the CLI to the offline path, where fix(server): enforce one database owner and guard update rollback #16102's ownership lock stops it. That path is unchanged.

Prior PRs: issue #7504 by @krutftw first pointed out that the 1 s bound covers both the probe and the dispatch. #16102 by @maria-rcks fixed the offline write behind a live server and closed #7504. #8682 by @dlowzzxx also addressed #7504 by limiting the offline fallback after a failed probe. Its author closed it without a stated reason, and it changed no timeouts. #15624 by @maria-rcks and #17172 by @sussdorff were closed as superseded by #16102. #17172 reported uncertain live outcomes instead of changing the timeout. Open PRs #22060, #23531 and #11596 edit other parts of project.ts or its test, and none of them touch these timeouts. #22060 adds a test import next to this branch's new imports, so whichever lands second has a small import-only merge.

Production fix, tests, PR description and images written with Claude Opus 5.5 in T3 Code Nightly.

🤖 Generated with Claude Opus 5.5 in T3 Code Nightly

only21mil and others added 2 commits October 22, 2026 12:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 1 s probe timeout also bounded the snapshot read and the mutation
sent to a server that already answered. A busy server that took over a
second to apply a change made the CLI report failure for a change that
went through. Keep the 1 s probe and give the live requests 10 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 23, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This is a narrowly scoped CLI timeout fix with a deterministic regression test, preserving the one-second server probe while allowing completed live requests up to ten seconds. Because it changes the default user-visible wait and failure behavior for live project commands, the change warrants human review.

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

@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between b2377db and f7be284.

📒 Files selected for processing: 2
  • apps/server/src/cli/project.test.ts
  • apps/server/src/cli/project.ts

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

📝 Walkthrough

The project CLI uses a 1-second timeout to detect a live server and a 10-second timeout for subsequent snapshots and mutations. An integration test checks behavior when a live mutation response is delayed.

Changes: Project CLI live-server timeout handling

Layer / File(s) Summary
Separate probe and request timeouts
apps/server/src/cli/project.ts
The snapshot probe uses a 1-second timeout. Follow-up snapshots and mutation requests use a 10-second timeout.
Delayed mutation integration test
apps/server/src/cli/project.test.ts
The test delays a mocked mutation response while advancing the test clock by one second. It then checks that the local project store remains empty.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: bugfix, size:S

Suggested reviewers: juliusmarminge, yordis

Merge Risk: ⚪ Minimal · up to f7be2

The CLI now waits longer for a busy live server after detecting it, and the regression test checks that the delayed mutation succeeds. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main fix: project commands wait for slow replies from a busy live server.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the failure, the timeout split, test coverage, observed results, limitations, and related …
Linked Issues check Passed The only linked issue, #7504, is closed. It is historical context only, so no active coding requirements apply. The PR also targets a related remaining failure: a successful live mutation that exceeds…
Out of Scope Changes check Passed All changes concern the live-server timeouts of the project CLI. The project.ts diff splits the single 1 s timeout into a probe timeout and a request timeout. It changes the fetchLiveOrchestrationSnap…
Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the failure, the timeout split, test coverage, observed results, limitations, and related issue history.

Full details: Linked Issues check

Explanation

The only linked issue, #7504, is closed. It is historical context only, so no active coding requirements apply. The PR also targets a related remaining failure: a successful live mutation that exceeds the 1 s bound is reported as failed. The reviewed diff for apps/server/src/cli/project.ts confirms the described split. PROJECT_CLI_LIVE_SERVER_PROBE_TIMEOUT (1 s) is passed only to the probe in tryResolveLiveProjectExecutionMode. PROJECT_CLI_LIVE_SERVER_REQUEST_TIMEOUT (10 s) is used for the post-probe snapshot read in runProjectMutation and for dispatchLiveOrchestrationCommand. Timeouts still map through projectCommandErrorFromLiveServerRequest. The issue's suggested fix, which makes runtime-file clearing conditional, is not part of this PR and is not a requirement because the issue is closed. Test results are author-reported; I did not run the test suite.

Full details: Out of Scope Changes check

Explanation

All changes concern the live-server timeouts of the project CLI. The project.ts diff splits the single 1 s timeout into a probe timeout and a request timeout. It changes the fetchLiveOrchestrationSnapshot signature to accept a timeout and removes the withProjectCliLiveServerTimeout wrapper. The PR description states that the server, the probe logic, and the offline path are unchanged, and the diff matches that statement. The test file change adds the regression test for a slow mutate reply and the imports it needs. This test supports the timeout change. No unrelated behavior change appears in the reviewed diff.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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.

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

size:M 30-99 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.

[Bug]: t3 project CLI treats any 1s live-server probe failure as "no server": deletes server-runtime.json and writes offline behind a running server

1 participant