Skip to content

fix(web): Unlink this PC clears a T3 Connect link from any state - #177

Merged
ImZoomBoy merged 1 commit into
mainfrom
fix/t3-connect-unlink-control
Oct 11, 2026
Merged

ImZoomBoy merged 1 commit into
mainfrom
fix/t3-connect-unlink-control

Conversation

@ImZoomBoy

Copy link
Copy Markdown
Owner

Problem

A PC whose server holds a T3 Connect link made under one account could not be unlinked after signing in as another account. Every attempt to switch T3 Connect on failed with "This environment is already linked to a different cloud account. Unlink it before switching accounts." No click reached the unlink call.

The only route to environment.cloud.unlink was reconcileCloudState when neither a managed tunnel nor publishing is wanted. That runs only when a switch that shows ON is turned OFF. A stored link with no tunnel and publishing off shows both switches OFF. Turning either one ON links as the signed-in account and hits the conflict.

Upstream has not fixed this. upstream/main (2cf0ff0643) has the same single route in useCloudLinkController.ts, so there was no upstream shape to follow.

Fix

  • Settings > Connections shows an Unlink this PC row whenever a link is stored, on web and desktop. It sits under the two switches and does not depend on what they show.
  • The row calls the local unlink. It needs no relay token and no sign-in to the account that made the link. The relay revoke is attempted only when the signed-in account is not known to differ from the linked one.
  • The conflict error now reads: This PC is already linked to a different T3 Connect account. Use "Unlink this PC" in Settings > Connections, then try again.
  • When the stored cloudUserId differs from the signed-in Clerk user id, the row says the PC is linked to a different account. The relay sets cloudUserId from the token subject, which is the Clerk user id. When either id is unknown the row shows the plain description.
  • No server change. validateLinkedCloudUser is untouched and unlink still needs the relay:write scope. Without that scope the button is disabled with the same permission tooltip the switches use.

The logic lives in two fork files, forkUnlinkThisPc.ts and ForkUnlinkThisPcRow.tsx. The two upstream files gain 13 and 14 lines. fork-features.json has a new t3-connect-unlink-this-pc entry with the keep rule. docs/user/remote-access.md gains one paragraph.

Reproduction

Throwaway dev server on an empty home under %TEMP%/fm-unlink-control, made-up Clerk key and relay URL, no sign-in. The link was made through the real endpoint with a made-up account id:

GET  /api/connect/link-state   -> 200 {"linked":false,"cloudUserId":null,...}
POST /api/connect/relay-config -> 200 {"ok":true,"endpointRuntimeStatus":{"status":"disabled"}}     (cloudUserId user_madeup_account_A)
GET  /api/connect/link-state   -> 200 {"linked":true,"cloudUserId":"user_madeup_account_A",...,"managedTunnelActive":false,"publishAgentActivity":false}
POST /api/connect/relay-config -> 409 {"_tag":"EnvironmentHttpConflictError","message":"This environment is already linked to a different cloud account. Unlink it before switching accounts."}     (cloudUserId user_madeup_account_B)

States that could not reach unlink on main:

  • Linked, no tunnel, publishing off: both switches show OFF, so there is no ON switch to turn OFF.
  • Linked under a different account, any switch state: turning a switch ON links as the signed-in account and gets the 409. Turning T3 Connect OFF while publishing is ON relinks as publish-only and gets the 409.
  • Linked and signed out of T3 Connect: both switches are disabled.

On main the page in that state has no unlink control:

main: linked, both switches off, no way to unlink

Real run with the fix

Same server, same stuck state. The headless browser clicked Unlink:

before-click {"Enable T3 Connect":"checked=null disabled=true","Publish agent activity to mobile clients":"checked=null disabled=true","Unlink this PC row":"present, button disabled=false"}
after-click  {"Enable T3 Connect":"checked=null disabled=true","Publish agent activity to mobile clients":"checked=null disabled=true","Unlink this PC row":"absent"}
connect requests: ["GET /api/connect/link-state -> 200","POST /api/connect/unlink -> 200","GET /api/connect/link-state -> 200"]
GET  /api/connect/link-state   -> 200 {"linked":false,"cloudUserId":null,"relayUrl":null,"relayIssuer":null,"managedTunnelActive":false,"publishAgentActivity":false}
POST /api/connect/relay-config -> 200 {"ok":true,"endpointRuntimeStatus":{"status":"disabled"}}     (cloudUserId user_madeup_account_B, 409 before the unlink)
GET  /api/connect/link-state   -> 200 {"linked":true,"cloudUserId":"user_madeup_account_B",...}

The link attempt as a second account gets past the conflict check after the unlink. In the page the switches stay disabled because nobody is signed in, which is the user's step.

Stuck state with the fix After clicking Unlink
fix: Unlink this PC row fix: after unlink

Plain web (no desktop bridge), same state:

fix on web

Limits of this run:

  • The T3 Connect row renders only when window.desktopBridge exists. The run did not launch the built desktop app. The script set a stand-in window.desktopBridge in the headless page after boot: every member is a function, subscriptions return an unsubscribe function, getLocalEnvironmentBearerToken returns the throwaway server's own dev token, and every other call never settles. No product code changed for this. The plain web screenshot uses no stand-in.
  • Nobody signed in, so the "linked to a different account" wording and the reworded conflict error were not seen in the page. The tests below cover both.
  • The machine name in the screenshots is covered with "MADE-UP-PC".

Tests

apps/web/src/cloud/forkUnlinkThisPc.test.tsx mounts useCloudLinkController with the commands mocked, the same shape as upstream's useCloudLinkController.test.tsx. It covers: unlink from the stuck state with clerkToken: null and no token read, unlink while signed out, relay revoke when the same account is signed in, local unlink when the token read fails, a failed unlink, the conflict error naming the control, the error clearing after unlink, and other errors left unchanged.

Fails on main: I removed the two new source files and restored main's useCloudLinkController.ts and ConnectionsSettings.tsx, leaving only the test file, then ran it:

$ vp test run src/cloud/forkUnlinkThisPc.test.tsx
 × unlinks a PC linked under another account with both switches off, without a relay token
   AssertionError: expected undefined to be true // Object.is equality
 × unlinks while signed out of T3 Connect
   AssertionError: expected undefined to be false // Object.is equality
 × also revokes at the relay when the signed-in account made the link
   AssertionError: expected undefined to be false // Object.is equality
 × still unlinks locally when the relay token cannot be read
   TypeError: mounted(...).unlinkThisPc is not a function
 × reports a failed unlink and leaves the link in place
   TypeError: mounted(...).unlinkThisPc is not a function
 × points the different-account conflict at the unlink control
   AssertionError: expected 'Could not configure environment relay…' to contain 'Use "Unlink this PC" in Settings > Co…'
 × clears a shown conflict once the PC is unlinked
   TypeError: mounted(...).unlinkThisPc is not a function
 Test Files  1 failed (1)
      Tests  7 failed | 1 passed (8)

The one that passes on main checks that other link errors keep their wording.

With the fix:

$ vp test run src/cloud/forkUnlinkThisPc.test.tsx
 Test Files  1 passed (1)
      Tests  8 passed (8)

Checks

$ vp fmt --check
All matched files use the correct format.
Finished in 1095ms on 4166 files using 24 threads.

$ vp lint --report-unused-disable-directives
exit 0 (warnings only, none in the new files or lines)

$ cd apps/web && vp run typecheck        (tsc --noEmit)
exit 0, 0 "error TS" lines

$ vp run knip:check
exit 0

$ cd apps/web && vp run test
 Test Files  412 passed (412)
      Tests  5484 passed (5484)

$ node scripts/check-fork-features.ts
 FAIL  scripts/build-desktop-artifact.test.ts  (11 tests)
 Test Files  1 failed | 35 passed (36)
      Tests  11 failed | 613 passed (624)

The fork features check exits 1 on this Windows machine because of the 11 scripts/build-desktop-artifact.test.ts failures. They fail the same way on main (fddd2c2):

$ git checkout --detach origin/main && vp test run scripts/build-desktop-artifact.test.ts
 Test Files  1 failed (1)
      Tests  11 failed | 60 passed (71)

Every other fork feature test passes, including the new one.

Surfaces

  • Entry points: the row is in ConfiguredCloudLinkRow, which all three places that render the T3 Connect rows share. The first-run Connect dialog uses the same controller and gets the reworded conflict error.
  • Clients: web and desktop. Mobile has no local environment to link, so nothing changes there.
  • Contracts and server: unchanged.
  • Reverse state: after an unlink the row goes away and the switches link again as normal.
  • Docs: one paragraph in docs/user/remote-access.md.

A PC linked under one T3 Connect account could not be unlinked once both
switches showed OFF, or once another account was signed in. The only path
to the unlink call ran when a switch that showed ON was turned OFF. Every
attempt to link then failed with the different-account conflict.

Settings > Connections now shows an Unlink this PC row whenever a link is
stored. It calls the local unlink and needs no relay token and no sign-in
to the account that made the link. The relay:write scope is still needed.
The conflict error names the row, and the row says when the stored link
belongs to another account than the one signed in.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −3 B (−0.0%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +7 B (+0.1%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.4 KiB 6.4 KiB −10 B (−0.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.1 KiB 56.1 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 7 7 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB −4 B (−0.0%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +1 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB −5 B (−0.1%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 56.9 KiB 56.9 KiB 0 B (0.0%) 66.4 KiB ✅
Claude Live turn messages 7 7 0 (0.0%) 21 ✅

Baseline: fddd2c2 · PR result: d14e674 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@ImZoomBoy

Copy link
Copy Markdown
Owner Author

Review - standards and spec

Two independent read-only reviews of d14e674. Verdict: no blocking finding. Security checks hold: no file under apps/server changed, so the account check on linking and the relay:write scope on unlink are untouched; the local unlink sends and reads no token when signed out or when the accounts differ; the relay revoke runs after the local unlink and its failure is caught. Double click, failed unlink, toggle race and stale error behave correctly.

Owed in a follow-up pull request, not blocking this one:

  • The cause of "both switches off did not unlink" is not found or fixed, and reconcileCloudState has no new regression test. This pull request adds the way out only.
  • The row is hidden when the link state fails to load.
  • The conflict hint matches server prose by substring; a typed conflict in packages/contracts would not drift.
  • After an unlink under a different account, the old account keeps its relay entry and nothing tells the user.

Not acted on, by standing fork decisions: the wording "Unlink this PC" was asked for by name; evidence images on a pr-assets branch are this fork's practice; the body does not name what wrote it.

Full reports follow.


Spec review - PR 177

The core fix meets the spec, with one gap: the root cause of the stuck state is not found or fixed.

Confirmed by reading the code:

  • No file under apps/server changed, so validateLinkedCloudUser and the relay:write check on unlink are untouched.
  • The local unlink sends no token and reads none when signed out or when the accounts differ.
  • The relay revoke runs after the local unlink and its failure is caught.

(a) Missing or partial

  1. Root cause not fixed. Supervisor ask: find and fix why "both switches off did not unlink", with a regression test. The diff adds a way out only. It does not say how "linked, no tunnel, publishing off" arises, and reconcileCloudState is unchanged and has no new test. Confidence high, severity medium.
  2. Row hidden when link state fails to load. Spec: "reachable in every linked state". linked falls back to false when there is no data, so the row disappears. Confidence medium, severity low.
  3. Row component untested. Spec: "the unlink control is offered in the stuck state". Tests cover the hook only. The hide rule and the disabled rule in ForkUnlinkThisPcRow.tsx have no test. Confidence high, severity low.
  4. Checks not all passing or pasted. Spec: "the fork features check ... pass" and "pastes the real commands and what they printed". The fork features check exits 1, with the same 11 failures shown on main. Lint and typecheck output is summarised. Confidence high, severity low.
  5. Wording never seen in a real client. The different-account text and the reworded error were not shown in the real run. The body says so. Confidence high, severity low.

(b) Not asked for

  1. A success toast and a docs paragraph. Both are small, and the paragraph is true. Confidence high, severity low.

(c) Implemented but questionable

  1. Stray relay entry. Skipping the revoke when accounts differ or when signed out is right, because the token could not revoke it. But the old account keeps a relay entry and nothing tells the user. Confidence medium, severity low.
  2. Token read can block the unlink. For the same account, getToken is awaited before the local unlink. A rejected read is handled, but a hung one blocks it. Confidence low, severity low.
  3. Brittle string match. Spec: "the error shown should point at that control". The match is on "linked to a different cloud account". The test uses its own copy of the server text, so a server rewording drops the pointer silently. Confidence high, severity low.
  4. Id comparison unverified. It assumes the relay's cloudUserId equals the Clerk userId. The relay source is not in this repository. If the forms differ, the same account shows as "different" and the revoke is skipped. Unknown ids are handled correctly. Confidence low, severity low.

Double click, failed unlink, toggle race and stale error all behave correctly.

Next action: decide whether finding 1 blocks the merge.


Standards review - PR 177

No high-severity breach. Four documented-standard findings and six baseline smells.

Documented standards

  1. Vocabulary drift: "PC" for environment. Hard violation. docs/agents/domain.md: use the glossary term, no synonyms. The AGENTS.md glossary says "environment". Sibling rows say "This environment"; the rest of the file says "this machine". Affects ForkUnlinkThisPcRow.tsx, forkUnlinkThisPc.ts (UNLINK_THIS_PC_LABEL, the rewritten error), docs/user/remote-access.md. "PC" is also wrong on macOS and Linux hosts. Confidence high, severity medium.

  2. Server error matched by substring in the client. Judgement call. AGENTS.md Contracts: "Anything crossing the wire is typed in packages/contracts". LINKED_ACCOUNT_CONFLICT = "linked to a different cloud account" couples the web client to server prose. A server rewording silently drops the hint. Confidence medium, severity medium.

  3. PR evidence committed to a branch. Hard violation. AGENTS.md Pull requests: "Upload PR evidence to GitHub. Never commit PR-only screenshots or assets". The images are served from a pr-assets branch (blob/pr-assets/t3-connect-unlink/...). Confidence medium, severity low.

  4. Tests assert call wiring. Judgement call. AGENTS.md Verifying: no tests that "merely assert callback wiring or mirror the implementation". forkUnlinkThisPc.test.tsx mocks seven modules. Several assertions are not.toHaveBeenCalled on mocks. The clerkToken: null cases do test real behavior. Confidence low, severity low.

  5. Explicit return annotations. Judgement call. AGENTS.md Taste: "Inferred types over annotations". : string, : boolean, : Promise<boolean> in forkUnlinkThisPc.ts. Upstream's reconcileCloudState does the same. Confidence low, severity low.

Baseline smells

All judgement calls.

  • Duplicated Code. unlinkThisPc repeats the unlink branch of reconcileCloudState: target guard, "Local environment is not ready yet.", token settle, failure report, refresh. Both hooks also create the same two useAtomCommand handles. The keep-line rule favours small upstream patches, which partly excuses it. Confidence high, severity low.
  • Duplicated Code. The permission tooltip string duplicates disabledReason in ConnectionsSettings.tsx. Confidence high, severity low.
  • Duplicated state. isUnlinking lives in the row and again in the parent through onBusyChange. One owner would do. Confidence medium, severity low.
  • Data Clumps. clearFailure and reportFailure travel together into useForkUnlinkThisPc. Confidence low, severity low.
  • Divergent Change. forkUnlinkThisPc.ts holds the hook, the UI label, and error rewording. Confidence low, severity low.
  • Mysterious Name. forkUnlinkThisPc.test.tsx mounts and tests useCloudLinkController, not the module it is named for. Confidence medium, severity low.

Checked and clean

  • fork-features.json entry matches the schema in scripts/check-fork-features.ts, with a keep line.
  • Docs paragraph is task-shaped and in product voice.
  • Commit title follows the conventional format. One concern.
  • No components/ui restyling. The inline-flex span wrapper matches existing use.
  • Surfaces walk is present in the PR body.

@ImZoomBoy
ImZoomBoy merged commit f690eff into main Oct 11, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant