Skip to content

fix(connect): identify rejected environment links and allow explicit relinking - #359

Merged
lukemaj merged 3 commits into
mainfrom
fix/358-rejected-connect-link
Oct 15, 2026
Merged

lukemaj merged 3 commits into
mainfrom
fix/358-rejected-connect-link

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

What: The server and CLI identify rejected T3 Connect links and support explicit relinking.
Why: A deleted bestia tunnel triggers replacement requests, but the relay answers 401 not_authorized while saved status still says provisioned.
So what: Merge the reviewed server repair; live bestia access still needs fresh account authorization before a supported relink.

Port the server, CLI and optional API-state portion of upstream #19379, merged as 8e018ff91f64d6caedc029e498eca446ff0fad20. Registration and running-host recovery mark a rejected credential without deleting it or automatically relinking. Explicit t3 connect link drops the rejected link while retaining sign-in/preferences. Accepted registration, recovery, configuration and unlink clear the mark. A late rejection cannot mark a concurrently replaced credential.

This is the headless server repair for #358. Upstream's optional UI changes are deferred to normal upstream absorption; existing clients tolerate the optional API field. No browser, provider, tunnel quota or relay deployment changes. Package versions remain upstream-owned.

Proof: 57 focused tests pass, including registration/recovery 401, retained credentials, concurrent relink and mark clearing. Five lifecycle regressions fail against the original production implementation, then pass with this patch. Server and contracts typechecks, targeted lint/format and whitespace checks pass. Additional nearby tunnel/link tests are recorded on #358.

Live limitation: bestia's account OAuth refresh independently returns HTTP 400 invalid_grant; its environment recovery receives relay HTTP 401. No live unlink or restart was attempted. This PR makes the state actionable, and does not claim restored phone access. The operational Issue stays open until outside-client proof exists.

Requirements and who asked: User requested diagnosis, supported recovery and an independently reviewed product repair where applicable.
Deleted: No manual token editing, automatic reauthorization, quota workaround, extra retries, UI work or service restart.
Bottleneck: Relay will not accept the stored environment credential; the account refresh grant is also invalid.
Checked myself: Live failure traces and clocks, recovery/auth/quota code, upstream complete diff, baseline regressions and focused checks.

Website impact: none, server/CLI behavior and existing troubleshooting guide only. No SEO or URL changes.

Model and harness: GPT-6.1-Sol, medium effort, Codex through T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 15, 2026
@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Scope clarification from deeper tracing: the relay shares 401 not_authorized across bearer, proof and link-consistency checks. We cannot infer the exact invalidation event from host traces alone. This is why the port only marks a rejected link and preserves its state, and why no automatic relink/auth bypass is added. The separate account OAuth refresh is confirmed HTTP 400 invalid_grant.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: No blocking findings on candidate 4fbbff530f2b5616957907572fd52d41e42d8769 against base 503282b9117ac946f637e5a6995e870e04549bba.
Why: The inspected rejection paths retain credentials, expose the rejected state, and preserve explicit CLI relinking with credential replacement protection.
So what: This exact candidate passes independent Sol review; the parent must obtain review for the newer PR head and wait for required CI before merging.

Reviewed Issue #358, the candidate's complete diff, upstream merged PR pingdotgg/t3code#19379 (8e018ff91f64d6caedc029e498eca446ff0fad20), and relevant callers in server.ts, cloud/http.ts, ManagedEndpointRuntime.ts, CliTokenManager.ts, and the connect commands. This verdict covers only the requested server/CLI/API repair, troubleshooting guidance and map ref. UI adoption and automatic reauthorization are outside its acceptance criteria.

Confirmed by source inspection:

  • Registration 401 becomes revoked, bypassing startup recovery/reconciliation decisions; running-host recovery marks the link and still fails with the original unauthorized error.
  • Marking compares the rejected credential to the current credential under the same link-state lock used by server relinking and unlinking.
  • Explicit connect link authorizes first, then drops only marked link state. Its removal list retains the separate OAuth credential, desired link mode and preferences.
  • Accepted configuration, registration, recovery and unlink paths clear the mark. linkRevoked is optional on the wire, preserving existing clients.

Independent focused check: ./node_modules/.bin/vp test run apps/server/src/cloud/CliState.test.ts apps/server/src/cloud/CloudLink.lifecycle.test.ts apps/server/src/cloud/managedTunnelStartup.test.ts apps/server/src/cli/connect.test.ts packages/contracts/src/environmentHttp.test.ts completed with 5 files, 57 tests passed, exit 0. pnpm was absent on PATH, so the installed executable was used. Diff whitespace check passed. No source edits, Git writes, browser use, live server/data writes or restarts were performed.

Identity limitation: the PR and workspace advanced during review to 036e2f1790d6ebf0344b2db5379fc16dc3428aea. I inspected the comparison: only docs/fork-features.md and scripts/fork-upstream-edits.txt were added to the diff, so test inputs and production source remain identical to the requested candidate. This review/status does not cover that newer commit. The author's base-failure and typecheck reports were read, not independently rerun. Full CI was still running when inspected. Live bestia restoration remains blocked on user sign-in and is not established by this review.

Requirements and who asked: Parent requested read-only, exact-SHA independent review of Issue #358's existing-component repair.
Deleted: No UI expansion, automatic reauthorization, live restoration claim, baseline source swapping or repository mutation.
Bottleneck: Validate rejected-link transitions and stale rejection handling; live account authorization is a separate blocker.
Checked myself: Complete candidate/upstream diffs, service and CLI callers, locking and secret ownership, optional contract, 57 focused tests, and candidate-to-current-head comparison.

Reviewer: GPT-6.1-Sol, high effort, Codex harness through T3 Code. No merge performed.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: No blocking findings on final candidate 036e2f1790d6ebf0344b2db5379fc16dc3428aea against base 503282b9117ac946f637e5a6995e870e04549bba.
Why: The complete cumulative diff and relevant callers preserve rejected credentials, expose actionable state, and protect replacement credentials from late rejection.
So what: This exact candidate passes independent Sol review; the parent must still obtain the other required exact-head review and green CI before merging, while live restoration remains blocked on user sign-in.

Read Issue #358 and its comments, PR #359 and its comments, the entire final cumulative diff, the complete merged upstream #19379 diff (8e018ff91f64d6caedc029e498eca446ff0fad20), and the relevant startup, runtime recovery, HTTP, CLI authorization, link-state locking and secret ownership paths. Compared the added/removed lines for all nine changed server/contracts paths with upstream: identical. The omitted UI/client presentation changes are explicitly outside this repair's acceptance criteria.

Confirmed by source inspection:

  • Registration 401 returns revoked without deleting credentials or scheduling startup relinking/recovery. Running-host recovery marks rejection and retains its original failure. The public 401 does not establish which relay authorization check failed.
  • Rejection marking compares the rejected credential with the current credential under the same link-state semaphore used by server configuration/relink and unlink.
  • Explicit CLI linking authorizes before dropping marked link state. The removal list leaves the separate OAuth credential, desired mode and preferences intact. Accepted configuration, registration, recovery and unlink clear the marker.
  • linkRevoked is optional on the wire. No UI, provider, relay deployment or automatic reauthorization changes were introduced.
  • The only delta from previously reviewed 4fbbff530f2b5616957907572fd52d41e42d8769 is docs/fork-features.md and scripts/fork-upstream-edits.txt. All eight carried upstream paths have a primary owner; the existing contracts path is also watched. Inspected the validator's ownership rules and the final-head Fork Stack Model CI success.

Proof disposition: reused the prior independent Sol run of 5 files / 57 focused tests recorded at #359 (comment) because the final delta changes neither production code nor test inputs. No tests or broad checks were duplicated in this round. Independently reran the cumulative diff whitespace check: exit 0. The reported five failing-before lifecycle regressions, 55 nearby tests and targeted typechecks remain author-provided evidence, not new executions by this reviewer. Full CI completion is a separate merge gate.

The workspace remained clean at the requested final SHA. No source/file/Git mutations, browser use, servers, live-data writes, restarts or merges were performed. This review does not claim that bestia access is restored.

Requirements and who asked: Parent requested read-only independent Sol review, high effort, of the exact final candidate for Issue #358's bounded existing-component repair.
Deleted: No UI expansion, automatic reauthorization, live restoration claim, source swapping or duplicate broad checks.
Bottleneck: Establish correctness and identity of the rejected-link transitions and final inventory; account sign-in separately blocks live restoration.
Checked myself: Full Issue/PR/upstream/final diffs, relevant callers and semaphore ownership, credential retention and marker clearing, optional contract, metadata validation rules, prior-proof input identity, whitespace and final head/workspace identity.

Reviewer: GPT-6.1-Sol, high effort, Codex harness through T3 Code. Verdict applies only to 036e2f1790d6ebf0344b2db5379fc16dc3428aea. No merge performed.

@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 4.9 KiB 5.0 KiB +30 B (+0.6%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB +30 B (+2.5%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.8 KiB 20.9 KiB +41 B (+0.2%) 29.3 KiB ✅
Codex Live turn messages 1 2 +1 (+100.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 503282b · PR result: 036e2f1 · 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: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

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

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: 4fbbff530f2b5616957907572fd52d41e42d8769 fails Fork Stack Model because eight upstream files change with no allowlist entry and no feature owner.
Why: Those files match upstream at base 503282b9117ac946f637e5a6995e870e04549bba and at upstream merge-base 8d11fe1261f0dfd74f58a1b1ae8ad7e400905fcb, and this commit is what first changes them.
So what: This SHA is not mergeable. The rejected-link behavior itself has no blocking finding. A later commit on the branch adds the inventory; this status does not cover that commit.

Blocking

scripts/fork-check.sh compares the whole stack with upstream/main. .github/workflows/fork.yml runs it on the PR head. At this SHA the check fails for these paths, which are absent from scripts/fork-upstream-edits.txt and have no owner in docs/fork-features.md:

  • apps/server/src/cli/connect.ts
  • apps/server/src/cloud/CliState.test.ts
  • apps/server/src/cloud/CliState.ts
  • apps/server/src/cloud/CloudLink.lifecycle.test.ts
  • apps/server/src/cloud/CloudLink.ts
  • apps/server/src/cloud/config.ts
  • apps/server/src/cloud/managedTunnelStartup.test.ts
  • apps/server/src/cloud/managedTunnelStartup.ts

Evidence: each blob is identical between 8d11fe12 and 503282b, and different at 4fbbff. packages/contracts/src/environmentHttp.ts, docs/map.md, and docs/user/remote-access.md were already allowlisted.

Required fix: list the eight paths in scripts/fork-upstream-edits.txt and give them one owner in docs/fork-features.md.

Rejected-link behavior

No blocking finding in the port. The server, CLI, and contract diff matches upstream 8e018ff91f64d6caedc029e498eca446ff0fad20 aside from existing fork line context. UI and client presentation stayed out.

  • Registration 401 returns revoked and startup takes no relink or recovery action. Credentials stay stored.
  • Running-host recovery marks the same 401 and still fails. shouldRetryCloudLink is false for unauthorized, so that attempt does not retry.
  • markLinkRevoked compares the rejected credential with the stored credential under the link-state lock, so a replacement is left unmarked. The lifecycle test covers that path.
  • t3 connect link authorizes first, then dropRevokedCloudLink removes the marked link only. Sign-in, desired mode, relay URL, and publish preference stay.
  • Accepted configuration, registration, recovery, and unlink clear the mark.
  • linkRevoked is optional on EnvironmentCloudLinkStateResult.
  • Package versions are unchanged.

Cleared: a successful registration removes the mark outside that lock. The acceptance case is the late 401 after replacement, which the lock covers, and that matches the upstream patch.

Independent check, on production blobs identical to this SHA: ./node_modules/.bin/vp test run apps/server/src/cloud/CliState.test.ts apps/server/src/cloud/CloudLink.lifecycle.test.ts apps/server/src/cloud/managedTunnelStartup.test.ts apps/server/src/cli/connect.test.ts packages/contracts/src/environmentHttp.test.ts — 5 files, 57 tests, exit 0. The author's base-regression and typecheck runs were not rerun. Live bestia restoration remains blocked on account sign-in and is outside this verdict.

Identity

During this review the PR head moved to 036e2f1790d6ebf0344b2db5379fc16dc3428aea, which adds docs/fork-features.md and scripts/fork-upstream-edits.txt. The eleven behavioral files are the same blobs as 4fbbff. This verdict and review/independent-grok status apply only to 4fbbff.

Requirements and who asked: Parent requested a read-only Grok review of the exact candidate for Issue #358.
Deleted: No UI expansion, automatic reauthorization, live restoration claim, or repository mutation.
Bottleneck: The candidate omits the fork inventory CI requires; the link-state transitions themselves hold.
Checked myself: Full candidate and upstream diffs, startup and CLI callers, credential lock, allowlist blobs against 8d11fe12 and 503282b, and the 57 focused tests.

Reviewer: Grok, high effort. No merge.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Accepted Grok finding for old head 4fbbff5: eight newly carried upstream files lacked the fork inventory. Commit 036e2f1 already adds their allowlist entries and primary ownership, with the contracts path shared under its existing owner. Current Fork Stack Model and full CI pass on 036e2f1790d6ebf0344b2db5379fc16dc3428aea. No production/test code changed, and no behavioral findings remain from the first Grok round. A new final-head Grok review has been delegated; no merge until its exact-head success.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: No blocking findings on final candidate 036e2f1790d6ebf0344b2db5379fc16dc3428aea against base 503282b9117ac946f637e5a6995e870e04549bba.
Why: Commit 036e2f1 puts the eight carried upstream paths on the fork allowlist and gives them one owner, which is the defect that failed 4fbbff5.
So what: review/independent-grok is success for this SHA. Live bestia restoration stays blocked on account sign-in and is not claimed here.

Inventory fix

The only delta from reviewed 4fbbff530f2b5616957907572fd52d41e42d8769 is docs/fork-features.md and scripts/fork-upstream-edits.txt. git diff --quiet over every other path is empty, so production and test blobs are unchanged.

Each of these paths occurs once in scripts/fork-upstream-edits.txt and once in feature rejected-connect-link upstreamFiles:

  • apps/server/src/cli/connect.ts
  • apps/server/src/cloud/CliState.test.ts
  • apps/server/src/cloud/CliState.ts
  • apps/server/src/cloud/CloudLink.lifecycle.test.ts
  • apps/server/src/cloud/CloudLink.ts
  • apps/server/src/cloud/config.ts
  • apps/server/src/cloud/managedTunnelStartup.test.ts
  • apps/server/src/cloud/managedTunnelStartup.ts

packages/contracts/src/environmentHttp.ts is sharedFiles on this feature. Its single upstreamFiles owner remains the people feature. docs/user/remote-access.md stays allowlisted under paired-session renewal. docs/map.md is absent at upstream merge-base 8d11fe1261f0dfd74f58a1b1ae8ad7e400905fcb, so the stack check treats it as a fork file. Fork Stack Model on this SHA succeeded.

Behavior

Reused the behavioral verdict and the 57-test run from #359 (comment). Those tests ran on these same production and test blobs. This round did not rerun them. Re-read the cumulative server, CLI, and contract diff: registration and recovery 401 mark the stored link without deleting it, startup takes no relink action, markLinkRevoked ignores a replaced credential under the link-state lock, t3 connect link authorizes before dropRevokedCloudLink, and accepted config, registration, recovery, and unlink clear the mark. linkRevoked stays optional. No web, version, or changelog files changed.

CI

Check runs on 036e2f1: 22 success, 4 skipped, none failed or still running. Skipped jobs are EAS Preview, Deploy web preview, Mobile Native Static Analysis, and Sync PR size label definitions. This does not restore bestia access.

Requirements and who asked: Parent requested a read-only Grok review of the exact final candidate for Issue #358.
Deleted: No second broad test run, UI expansion, live restoration claim, or repository mutation.
Bottleneck: The prior SHA failed fork inventory CI. This commit is that inventory, and the link-state code is unchanged.
Checked myself: Worktree HEAD 036e2f1, parent 4fbbff5, cumulative diff, allowlist counts, feature ownership, upstream merge-base classification, and live check-run SHAs.

Reviewer: Grok, high effort. Verdict applies only to 036e2f1790d6ebf0344b2db5379fc16dc3428aea. No merge.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: Both required independent reviews passed on final head 036e2f1790d6ebf0344b2db5379fc16dc3428aea.
Why: Sol found no blockers, and Grok verified that the final inventory commit resolves the old head's only blocking finding without changing production or test blobs.
So what: The authorized code repair can merge after the final live gate check; bestia restoration remains blocked on fresh account authorization.

Exact-head findings and verdicts:

The current head has green CI, including all six server shards and Fork Stack Model. No remaining review finding is unresolved. This relays the reviewers' verdicts; live T3 Connect access has not been restored or verified, and #358 stays open.

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