Skip to content

fix(cli): t3 update no longer calls a failed download a missing release - #29454

Closed
MarcoDotIO wants to merge 2 commits into
pingdotgg:mainfrom
MarcoDotIO:fix/update-404
Closed

MarcoDotIO wants to merge 2 commits into
pingdotgg:mainfrom
MarcoDotIO:fix/update-404

Conversation

@MarcoDotIO

@MarcoDotIO MarcoDotIO commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Update: #29911 has since landed the same fix on main (isUnpublishedRelease checks the response status). After merging main, this PR changes only update.test.ts: it adds end-to-end runUpdate coverage for a version containing 404, failing with a reset or a 503, and for a real 404. The description below is the original report.

Problem

t3 update tells you a release doesn't exist when the download of that release merely failed, if the version number happens to contain 404.

The missing-release check in runUpdate (apps/server/src/cli/update.ts) tests String(error.cause).includes("404"). That string carries the request URL as well as the status, and the URL carries the version:

HttpClientError: Transport error (GET https://github.com/pingdotgg/t3code/releases/download/v1.2.3-nightly.20260404.1/SHA256SUMS)

So for any version containing 404, such as a nightly dated 20260404 or a run number like .2404, a connection reset or a 503 comes back as No release archive was published for t3@…. The same happens for every failed download when T3CODE_RELEASE_BASE_URL contains 404, such as a mirror on port 4040. The release exists, so the message sends you looking for the wrong problem. A proxy failure like the ECONNRESET in #21922 would be misreported this way too.

Call path: t3 update [version] → runUpdate → ensurePinnedRuntimeInstalled → installFromArchive → fetchReleaseAsset (SHA256SUMS) fails → the Effect.catchIf in runUpdate.

Change

Check the response status on the HTTP client error instead of searching its text. Other failures now keep the install error they already had: Pinned runtime install failed while downloading the t3 release checksums.

Scope and approval

A two-line fix to an obvious bug, so no prior issue. One condition changes; a real 404 still gives the same message as before.

Verification

Added two tests in apps/server/src/cli/update.test.ts that run the real runUpdate and ensurePinnedRuntimeInstalled against a stubbed HTTP client, using the version 1.2.3-nightly.20260404.1:

  • reports a missing release when the checksums return 404: guards the real case, which had no test. Passes before and after.
  • does not call a failed download a missing release: a transport reset and a 503. Fails before, passes after.

Before, with the fix removed:

× does not call a failed download a missing release
AssertionError: expected 'No release archive was published for …' to equal 'Pinned runtime install failed while d…'
Tests  1 failed | 1 passed | 16 skipped (18)

After:

✓ t3 update download failures > reports a missing release when the checksums return 404
✓ t3 update download failures > does not call a failed download a missing release
Tests  18 passed (18)

vp lint and vp fmt --check on apps/server/src/cli, and tsc --noEmit in apps/server, are clean.

Not checked: a real t3 update against GitHub. The failures are stubbed at the HTTP client.

Claude Opus 5.5 (Claude Code desktop app)

🤖 Generated with Claude Code

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:S 10-29 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d6d0ac5

Macroscope's review found this PR approvable — The CLI now uses an actual HTTP 404 response to identify a missing release rather than matching error text. This narrowly changes failure diagnostics without changing update defaults or installation safeguards.

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

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The update command now identifies a missing release archive from an HTTP 404 response, rather than matching "404" in an error string. Tests cover HTTP 404, HTTP 503, and transport failures.

Changes: Checksum error handling

Layer / File(s) Summary
Classify checksum download failures
apps/server/src/cli/update.ts, apps/server/src/cli/update.test.ts
The update command checks the HTTP error type and response status to identify a missing archive. Tests verify that HTTP 503 and transport failures report a checksum download failure instead.

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

Change: Bug fix

Suggested reviewers: exotic209093, juliusmarminge

Merge Risk: Minimal · up to d6d0a

Missing releases are distinguished from other download failures. No merge-blocking risk was identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the change: t3 update no longer reports a failed download as a missing release. The raw summary confirms the change to runUpdate's missing-release check, which now uses the HTTP respon…
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.
Description check Passed The description is complete and matches the template. It has a Problem section that explains the bug and the call path. It has a Change section that describes the fix. It has a Scope and approval sect…
Full details: Title check

Explanation

The title names the change: t3 update no longer reports a failed download as a missing release. The raw summary confirms the change to runUpdate's missing-release check, which now uses the HTTP response status instead of matching "404" in the error text. The title is specific and matches the main fix.

Full details: Description check

Explanation

The description is complete and matches the template. It has a Problem section that explains the bug and the call path. It has a Change section that describes the fix. It has a Scope and approval section that explains why no prior issue is needed for this small fix. It has a Verification section with the test names, before and after results, lint and type-check results, and what was not checked. The author also notes that #29911 already landed the same fix on main, so the remaining diff is only the test file. The template's model-and-harness attribution is present. The description also has an update note at the top that explains the current scope. Those points are consistent with the raw summary, which describes the update.ts change and the update.test.ts additions.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c8a601c and d6d0ac5.

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

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

@maria-rcks

maria-rcks commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

Verified unchanged head d6d0ac529384aa263b81e01951d88fa599929a17 on a disposable Linux VM. Reproduced the bug on github/main at 5815442128 in a separate worktree, then installed and ran the exact PR head. Only one task dev server ran at a time.

Used the real CLI entry point and Fetch HTTP client, without mocks, fixtures, or demo data. An unavailable release mirror at http://127.0.0.1:4040 produces a real connection refusal:

T3CODE_RELEASE_BASE_URL=http://127.0.0.1:4040 node apps/server/src/bin.ts update 1.2.3-nightly.20260404.1 --base-dir <isolated-home> --allow-downgrade --yes
  • Main: CliUpdateError: No release archive was published for t3@1.2.3-nightly.20260404.1.
  • PR: PinnedRuntimeInstallError: Pinned runtime install failed while downloading the t3 release checksums. Its cause correctly retains connect ECONNREFUSED 127.0.0.1:4040.
  • A real GitHub request for v99.404.999/SHA256SUMS returned HTTP 404. Both main and PR report No release archive was published for t3@99.404.999.
  • PR: updating to the current version 0.1.6 exits 0 with t3 is already on 0.1.6 (stable).
  • PR: update definitely-invalid still rejects the invalid exact version.

Also drove the actual dev web client with repo Playwright and /usr/bin/google-chrome. Added the real git checkout as a project through the UI, selected Codex gpt-6.1-sol, and submitted a real provider turn that ran the first three commands above. The completed turn displayed the expected exit codes (1, 1, 0) and diagnostics. A follow-up turn also streamed and completed. Main's real Codex turn worked before switching servers.

real codex turn reports the three update outcomes

real codex follow-up interaction in the dev web client

Inspected the captures and checked uploaded video playback and seeking in T3's player. Blacksmith passed all 18 update tests and vp run check:changed d833184115ec823825c2cc6f612443cd3a1c1b2c: format, lint, knip, GraphQL, typechecks for 3 packages, and 19 tests across 2 files. No source changes were needed.

Not verified

A real HTTP 503 or connection reset was not induced; the real transport case was ECONNREFUSED. The existing tests cover 503 and reset. Successful archive installation, background-service restart, and launcher replacement were not exercised. Native desktop/mobile and macOS/Windows behavior were not exercised on this Linux VM. This PR changes CLI failure classification, with no UI changes.

pingdotgg#29911 landed the same status check through isUnpublishedRelease, so main's update.ts is taken whole.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MarcoDotIO

Copy link
Copy Markdown
Contributor Author

Closing: #29911 landed the same fix on main, checking the response status through isUnpublishedRelease. Nothing of the fix is left here, only extra test coverage.

Claude Opus 5.5 (Claude Code desktop app)

@MarcoDotIO MarcoDotIO closed this Nov 3, 2026
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Nov 3, 2026
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.

2 participants