fix(connect): explain DPoP connection failures - #8351
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (46)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
UI consistency review of the changed web surfaces. The new multi-line DPoP hint is handled correctly where it is rendered (whitespace-pre-line break-words instead of truncate, and ConnectionStatusDot already sets max-w-80 whitespace-pre-wrap for the tooltip). Two consistency points on the wrap-vs-truncate treatment are noted inline.
Posted via Macroscope — UI Consistency
| className={cn( | ||
| "mt-1 truncate text-xs", | ||
| "mt-1 text-xs", | ||
| statusHasHint ? "whitespace-pre-line break-words" : "truncate", |
There was a problem hiding this comment.
The text.includes("\n") ? "whitespace-pre-line break-words" : "truncate" decision is now duplicated in three web call sites (here, settings/ProviderSettingsPanel.tsx ~line 267, settings/ConnectionsSettings.tsx ~line 1470), while CommandPaletteResults.tsx and T3ConnectUserProfilePage.tsx apply the wrap classes unconditionally. The hint format (\n\n separator) is owned by client-runtime, so a change there has to be re-found in every row.
Consider a small named helper next to the status presentation code — e.g. statusTextWrapClassName(statusText) in components/cloud/cloudEnvironmentConnectionPresentation.ts returning "whitespace-pre-line break-words" or "truncate" — and calling it from each row, keeping contextual color/spacing classes at the call site.
Posted via Macroscope — UI Consistency
| <span | ||
| className={cn( | ||
| "min-w-0", | ||
| connectionStatusText(environment.connection).includes("\n") | ||
| ? "whitespace-pre-line break-words" | ||
| : "truncate", | ||
| )} | ||
| > | ||
| {connectionStatusText(environment.connection)} | ||
| </span> |
There was a problem hiding this comment.
connectionStatusText(environment.connection) is evaluated twice in this row, and statusTooltip (line 1376) already holds the same string. Reusing it keeps the wrap decision and the rendered text from drifting apart, and matches the sibling rows that hoist statusText/statusHasHint before the JSX.
| <span | |
| className={cn( | |
| "min-w-0", | |
| connectionStatusText(environment.connection).includes("\n") | |
| ? "whitespace-pre-line break-words" | |
| : "truncate", | |
| )} | |
| > | |
| {connectionStatusText(environment.connection)} | |
| </span> | |
| <span | |
| className={cn( | |
| "min-w-0", | |
| statusTooltip.includes("\n") ? "whitespace-pre-line break-words" : "truncate", | |
| )} | |
| > | |
| {statusTooltip} | |
| </span> |
Posted via Macroscope — UI Consistency
UI Consistency: 2 findingsThis PR makes several status/error strings multi-line ( 1.
- <p className="mt-0.5 text-xs text-muted-foreground">{devicesState.error}</p>
+ <p className="mt-0.5 whitespace-pre-line break-words text-xs text-muted-foreground">
+ {devicesState.error}
+ </p>2.
Neither change alters layout for single-line status text. Posted via Macroscope — UI Consistency |
There was a problem hiding this comment.
Reviewed the web UI changes that make status/error text render the new multi-line DPoP hint. The gated whitespace-pre-line break-words vs truncate treatment looks correct for the rows that were updated (no fixed-height or virtualized rows are involved, and ConnectionStatusDot's tooltip already sets whitespace-pre-wrap).
Two sibling render sites that display the same now-multi-line messages were not updated, so the hint collapses onto one line there. Details inline.
Posted via Macroscope — UI Consistency
| <p className="mt-1 whitespace-pre-line break-words text-xs text-muted-foreground"> | ||
| {environmentsState.error} | ||
| </p> |
There was a problem hiding this comment.
The sibling mobile-clients page renders the same relay snapshot error string and was not updated, so the new hint collapses to one run-on line there. useManagedRelayDevices and useManagedRelayEnvironments both go through readManagedRelaySnapshotState, which now returns relayProtectedErrorMessage(...) (which can contain \n\n), but MobileClientsUserProfilePage.tsx:133 still renders it with default whitespace handling.
Suggest applying the same treatment there so the two T3 Connect profile pages present the same message identically:
- <p className="mt-0.5 text-xs text-muted-foreground">{devicesState.error}</p>
+ <p className="mt-0.5 whitespace-pre-line break-words text-xs text-muted-foreground">
+ {devicesState.error}
+ </p>Posted via Macroscope — UI Consistency
| <span | ||
| className={cn( | ||
| "min-w-0", | ||
| connectionStatusText(environment.connection).includes("\n") | ||
| ? "whitespace-pre-line break-words" | ||
| : "truncate", | ||
| )} | ||
| > | ||
| {connectionStatusText(environment.connection)} | ||
| </span> |
There was a problem hiding this comment.
The T3 Connect row further down this file (status={operationError ?? primaryCloudLinkState.error}, around line 1687) can now receive a message built by relayProtectedErrorMessage (link/challenge relay calls failing with invalid_dpop), but SettingsRow wraps status in a plain <div className="pt-0.5 text-xs text-muted-foreground"> with no whitespace handling, so the \n\n hint collapses into the sentence instead of reading as a hint line.
Smallest fix is to pass the status as pre-line text at that call site:
- status={operationError ?? primaryCloudLinkState.error}
+ status={
+ (operationError ?? primaryCloudLinkState.error) ? (
+ <span className="whitespace-pre-line break-words">
+ {operationError ?? primaryCloudLinkState.error}
+ </span>
+ ) : null
+ }Given this newline-sniffing pattern is now repeated at four web call sites (plus the mobile rows), a small shared helper (e.g. a hintAwareTextClassName(text) in ~/lib/utils) would keep the treatment from drifting between surfaces.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One remaining web renderer of the new multi-line message still collapses the hint. Details inline.
Posted via Macroscope — UI Consistency
| import { request, runStream } from "@t3tools/client-runtime/rpc"; | ||
| import { makeEnvironmentHttpApiClient } from "@t3tools/client-runtime/rpc"; | ||
| import { ManagedRelay } from "@t3tools/client-runtime/relay"; | ||
| import { ManagedRelay, relayProtectedErrorMessage } from "@t3tools/client-runtime/relay"; |
There was a problem hiding this comment.
decodedRelayClientError (line 135) now builds "<action>: Relay rejected the DPoP proof.\n\nHint: …" from relayProtectedErrorMessage, and that string reaches the error toast in apps/web/src/cloud/useCloudLinkController.ts:61 (description: message). Toast.Description is rendered with min-w-0 select-text wrap-break-word … and no whitespace-pre-line, so the hint collapses into the sentence there — unlike the row renderers updated in this PR and unlike mobile.
Smallest fix that matches the pattern used for EnvironmentUnavailableRow is to pass the description as a pre-line node at the call site:
- description: message,
+ description: message.includes("\n") ? (
+ <span className="whitespace-pre-line">{message}</span>
+ ) : (
+ message
+ ),(or add whitespace-pre-line to descriptionClassName in components/ui/toast.tsx if every toast should preserve author-supplied line breaks).
Posted via Macroscope — UI Consistency
UI Consistency: 2 findingsThe renderers flagged in earlier runs (toast description, 1.
Smallest fix is at the call site (mirrors the title: `${activeEnvironmentUnavailableState.label}: ${connectionStatusTitle(unavailableConnection)}`,
- description:
- unavailableConnection.error ??
- "Reconnect this environment before sending messages or running actions.",
+ description: unavailableConnection.error ? (
+ <span className="whitespace-pre-line break-words">{unavailableConnection.error}</span>
+ ) : (
+ "Reconnect this environment before sending messages or running actions."
+ ),2.
- {operationError ? <p className="text-xs text-destructive">{operationError}</p> : null}
+ {operationError ? (
+ <p className="whitespace-pre-line break-words text-xs text-destructive">{operationError}</p>
+ ) : null}Posted via Macroscope — UI Consistency |
|
7fcedaf to
d479fc6
Compare
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes authentication-path DPoP verification and error contracts across shared code, relay, server, client runtime, web, and mobile, making it a broad cross-cutting runtime change rather than a contained presentation fix. Sensitive authentication directories and verifier behavior are involved, while the remaining UI comments are comparatively minor or already addressed in the current diff. You can add or adjust custom eligibility rules. Learn more. |
|
@coderabbitai review Please run one final review against the current head commit. Verification was performed by GPT-5.6-Luna on behalf of extoci via T3 Code. |
|
Action performedReview triggered.
|
1 participant
Problem
DPoP proof failures often appear as a generic connection error. A device clock mismatch is the most common cause, but a client can also be talking to an older T3 Connect server or relay.
What changed
Compatibility
The new response field is optional. Updated clients understand responses from older servers and relays, and older clients can ignore the additive field from newer servers. Because clock skew is the most common DPoP failure, clients use the date/time hint when dpopFailureReason is absent. This is the safest first diagnostic for older or otherwise unknown responses; newer responses use a neutral retry hint for known non-clock failures.
Validation
git diff --checkpasses.Focused verification is complete here. Please run the repository CI suite before merge.
Note
Add
dpopFailureReasonto auth errors and surface DPoP failure hints in clientsDpopFailureReasonin baseSchemas.ts and adds it as an optional field toEnvironmentAuthInvalidErrorandRelayAuthInvalidError; server and relay now populate it with categorized codes derived from DPoP verifier failures.verifyDpopProofin dpop.ts returns structuredDpopVerificationFailureCodevalues instead of generic errors; relayverifyAndConsumeraises the newDpopProofRejectedinstead ofHttpApiError.Unauthorized.dpopFailureHintandrelayProtectedErrorMessagein errorPresentation.ts to translate failure reasons into user-facing hints (clock hint fortime_window, retry hint for others).whitespace-pre-line/break-wordsrendering when error text contains newlines, so multi-line hints display without truncation.verifyDpopProofnow checks signature before time window, so an out-of-window but invalidly signed proof reportsinvalid_signatureinstead oftime_window; relay callers ofverifyAndConsumemust handleDpopProofRejectedinstead ofUnauthorized.Macroscope summarized d479fc6.
Note
Medium Risk
Touches DPoP verification order and auth error payloads across environment server, relay, and all Connect clients; changes are additive and tested but still sit on the authentication path.
Overview
DPoP auth failures now carry an optional safe
dpopFailureReasonon environment and relayauth_invalidresponses, backed by a sharedDpopFailureReasonschema and structured failurecodes fromverifyDpopProof(signature is validated before the time window so bad signatures no longer masquerade as clock skew).Servers and relay map low-level verifier codes into those categories, attach them to API errors, and record
environment.dpop.failure_code/relay.dpop.failure_codeon spans. Relay DPoP verification returnsDpopProofRejectedwith explicit codes instead of a generic unauthorized.Clients centralize messaging in
errorPresentation(dpopFailureHint,relayProtectedErrorMessage): missing ortime_windowreasons get a date/time hint; other known reasons get a neutral retry/trace hint. Mobile and web connection/status UI stop truncating messages that contain newlines so multi-line hints stay readable.Tests and docs cover backward compatibility (older peers without the field still get the clock hint) and user-facing troubleshooting.
Reviewed by Cursor Bugbot for commit d479fc6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
User Interface
Documentation