Skip to content

fix(relay): preserve provisioning failure stage in client errors - #14581

Open
ECuteri wants to merge 1 commit into
pingdotgg:mainfrom
ECuteri:fix/relay-provisioning-stage
Open

ECuteri wants to merge 1 commit into
pingdotgg:mainfrom
ECuteri:fix/relay-provisioning-stage

Conversation

@ECuteri

@ECuteri ECuteri commented Oct 1, 2026

Copy link
Copy Markdown

T3 Connect collapses every ManagedEndpointProvisioningFailed into the same client error, discarding the stage needed to distinguish an allocation failure from a tunnel, DNS, or token failure.

Preserve stage as optional provisioningStage in the existing HTTP 503 error and append it to the shared web/mobile error message. Existing reason and trace ID remain intact. Older relay responses still decode and retain their existing message. Raw provider causes and resource identifiers are not returned.

Closes #14580. Related to #14070.

Evidence

Reproduced on macOS with T3 Code 0.0.44, Settings → Connections → enable T3 Connect. The toggle returned to off with managed_endpoint_provisioning_failed on both attempts:

  • f8f444f3bf4f07fc4ff5aacb9243fcc9
  • 994fe515cbc487d7cf90d24637988a3c

The local relay-client status and link-proof generation succeeded. The hosted provisioning stage/cause is unavailable without relay operator access.

The HTTP regression test failed on the original mapper: its 503 body omitted the injected ensure-tunnel stage. With this patch, the real HTTP API handler serializes the stage, the contract decodes it, and the shared presenter reports it. The exact body assertion verifies that a synthetic provider secret and tunnel identifier do not escape into the response. The provider failure is injected; no live Cloudflare allocation is created by these tests.

Validation

Node 24.13.1, with the frozen upstream lockfile:

  • vp test run packages/contracts/src/relay.test.ts packages/client-runtime/src/relay/errorPresentation.test.ts infra/relay/src/http/Api.test.ts — 43 tests pass.
  • Targeted vp lint and vp fmt --check on the six changed files — pass.
  • pnpm --filter @t3tools/contracts --filter @t3tools/client-runtime --filter t3code-relay run typecheck — pass.
  • git diff --check — pass.

This is a focused fix for observable diagnostic information loss, not a change to provisioning or recovery policy. It follows the small obvious-bug exception in CONTRIBUTING.md. It builds on the structured provider errors already present in the code.

Limit

This does not claim to resolve the hosted incident in #14070. A relay operator still needs to inspect the real traces and remedy their cause. The optional stage requires a relay deployment before clients can receive it; updating a local client alone cannot recover information discarded by the hosted relay. The installed desktop app and active environments were not replaced or restarted.

Model: GPT-6. Harness: Codex desktop.

T3 Connect returned the same managed_endpoint_provisioning_failed message
for allocation, tunnel, DNS, and token failures because the HTTP mapper
discarded ManagedEndpointProvisioningFailed.stage.

Carry an optional provisioningStage through the shared error contract and
web/mobile presenter. Preserve the existing reason, trace ID, and HTTP 503.
Only expose the bounded stage; provider causes and identifiers stay private.

Validation: reproduced the omission before the fix. After the change,
43 focused tests pass, including HTTP serialization, contract decoding,
client presentation, older responses, and exclusion of provider secrets.
Targeted lint, formatting, and contracts/client-runtime/relay typechecks
pass on Node 24.13.1.

This fixes diagnostic information loss, not the unconfirmed production
provisioning cause. Relay operator trace inspection is still required.

Refs: pingdotgg#14580, pingdotgg#14070
@github-actions github-actions Bot added the size:XS 0-9 changed lines (additions + deletions). label Oct 1, 2026
@github-actions github-actions Bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Oct 1, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 1, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at c7723ef

Macroscope's review found this PR approvable — This is a small, backwards-compatible diagnostic fix that carries an existing provisioning stage through the relay’s 503 error and displays it without changing provisioning behavior or exposing provider details. Production behavior is narrowly scoped and covered by HTTP, contract-compatibility, and presentation tests.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

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: 413142d6-a087-4a41-a3fc-c7c8393d9e0e

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc99e1 and c7723ef.

📒 Files selected for processing (6)
  • infra/relay/src/http/Api.test.ts
  • infra/relay/src/http/Api.ts
  • packages/client-runtime/src/relay/errorPresentation.test.ts
  • packages/client-runtime/src/relay/errorPresentation.ts
  • packages/contracts/src/relay.test.ts
  • packages/contracts/src/relay.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.


📝 Walkthrough

Walkthrough

The relay now includes the failed managed-endpoint provisioning stage in the unavailable-link error response. The shared error contract accepts the optional field, and the client error message displays it when present. Tests cover HTTP response sanitization and compatibility with older responses.

Changes

Provisioning error details

Layer / File(s) Summary
Add optional stage to error contract
packages/contracts/src/relay.ts, packages/contracts/src/relay.test.ts
The error schema accepts an optional provisioning stage. Tests check decoding when the field is missing or contains an unrecognized value.
Propagate stage in relay response
infra/relay/src/http/Api.ts, infra/relay/src/http/Api.test.ts
The handler includes the provisioning failure stage in the unavailable-link response. The HTTP test checks the 503 response, client message, and omission of private failure details.
Display stage in client error message
packages/client-runtime/src/relay/errorPresentation.ts, packages/client-runtime/src/relay/errorPresentation.test.ts
The client message includes the stage when present and retains the reason-only message when it is absent.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to c7723

This change adds the failed provisioning stage to link-failure errors and keeps older responses working. I found no merge-blocking risk. Clients only show the stage once the relay is deployed with this change.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to c7723

The change exposes a server-defined stage label in an existing error response without expanding permissions or returning provider secrets or resource identifiers. Older responses remain supported. No material security risk was found in the changed behavior.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added exposure is operational-stage information in an existing user-scoped linking failure response. The changed flow provides no new ability to access another environment, obtain a tunnel token, or control provider resources.

Trust Boundaries and Controls

  • observed — The internal-to-client error boundary remains an explicit field selection rather than serialization of the provider error. Request data is not copied into provisioningStage; inspected provider failure sites assign literal stage labels independently of provider causes and resource identifiers.

Resilience and Maintainability Implications

  • observed — The HTTP regression injects a provisioning failure containing a synthetic private tunnel ID and provider cause, then asserts the exact serialized 503 body and decoded client message. This protects the disclosure boundary against drift, but does not exercise production authentication, live allocation, or recovery.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the primary change: preserving the provisioning failure stage in relay client errors.
Description check ✅ Passed The description explains the problem, change, scope, approval exception, security constraints, verification steps, test results, and known limitations. Although it does not use the template headings e…
Linked Issues check ✅ Passed PR #14581 meets the coding requirements in directly linked issue #14580. Api.ts carries the bounded provisioning stage into provisioningStage. The contract makes this field optional. The client …
Out of Scope Changes check ✅ Passed The changes remain within issue #14580. The source changes implement stage propagation, optional contract decoding, and client presentation. The added tests verify HTTP behavior, compatibility, and sa…
  • Fix all pre-merge checks with AI
✨ 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.

@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 05:26

Dismissing prior approval to re-evaluate c7723ef

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-6). If this change is really important, we recommend rebuilding the PR with a newer model if possible.

@maria-rcks maria-rcks closed this Oct 11, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

@maria-rcks maria-rcks reopened this Oct 11, 2026

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.

[Bug]: T3 Connect discards the failed provisioning stage from client errors

3 participants