Skip to content

t3x: resolve upstream sync (run 30235353352) - #22

Merged
radroid merged 0 commit into
mainfrom
t3x/sync-30235353352
Jul 27, 2026
Merged

t3x: resolve upstream sync (run 30235353352)#22
radroid merged 0 commit into
mainfrom
t3x/sync-30235353352

Conversation

@github-actions

Copy link
Copy Markdown

Summary

Rebases fork main onto pingdotgg/t3code@89c5a192f (36 upstream commits, feat(dev): keep worktree dev state isolated on T3 Code dev servers through fix(dev): skip browser-blocked ports (#4608)), resolving the conflicts that blocked the daily automated sync.

Conflicts resolved

  • docs/t3x/SEAMS.md (2 hunks): a self-conflict from replaying the fork's own commit history in a different order than originally authored — an earlier-replayed fork commit already carried the fuller table content that a later-replayed (but chronologically earlier) commit was about to reintroduce a narrower version of. Resolved by keeping the fuller/final table content (both rows), matching the doc's state on pre-rebase main.
  • apps/web/src/components/settings/SettingsPanels.tsx (1 hunk): both upstream (sidebarProjectGroupingMode) and fork (notifyOnNeedsInput) added an entry to the same useMemo dependency array around the same line. Resolved by keeping both entries.

No patches went empty/dropped.

Seam review (docs/t3x/SEAMS.md)

Checked every upstream commit in the merged range that touched a seam file, even where it didn't textually conflict:

  • apps/server/src/orchestration/decider.ts — touched by 202e5609f (thread snoozing). The hasOpenBlockingRequest function that apps/server/src/t3x/autoResume/guards.ts mirrors is byte-for-byte unchanged (only a new doc comment was added above it). No mirror drift.
  • apps/server/src/http.ts — touched by 23b550221 (Tailscale dev sharing). The authenticateRawRouteWithScope function that apps/server/src/t3x/autoResume/http.ts mirrors is byte-for-byte unchanged. No drift.
  • apps/web/src/routes/_chat.$environmentId.$threadId.tsx — touched by 91dfe60a9 (thread loading flash fix), which replaced the routeThreadExists boolean with a renderState enum. The fork's <AutoResumeOverlay /> mount point (sibling of <ChatView>, gated by renderState !== "ready") still applies correctly — rebased cleanly with no conflict and no behavior change needed.
  • apps/server/src/server.ts (the T3xLayerLive/T3xRoutesLive mount seam) — untouched by this upstream range.

One incidental observation, not a seam-review action item: upstream's new notification-coordinator PR added <NotificationCoordinator /> to apps/web/src/routes/__root.tsx, which isn't listed in SEAMS.md — it rebased clean with no fork involvement, so nothing to reconcile, but the doc could note it for completeness in a future pass.

New upstream dependency

Upstream added a packages/tailscale workspace package (Tailscale dev-share feature). Required vp install to link it — vp run typecheck failed with Cannot find module '@t3tools/tailscale' until then. No fork changes needed here, just a fresh install.

Verify fixes (both pre-existing, reproduced identically on pre-rebase main, not introduced by this rebase)

  • vp run typecheck was red on apps/server due to a multipleEffectProvide warning (fork-owned apps/server/src/t3x/autoResume/http.test.ts) — the effect language service fails the check on any warning-or-above diagnostic, not just error. Fixed by merging two chained Effect.provide calls into one Effect.provide(Layer.merge(...)).
  • vp run lint was red due to a t3code/no-manual-effect-runtime-in-tests violation (fork-owned apps/server/src/orchestration/Layers/CrashRecoveryReconciler.test.ts, using ManagedRuntime.make). Fixed with a scoped oxlint-disable-next-line + justification comment, following the existing precedent in apps/desktop/src/backend/DesktopBackendConfiguration.test.ts, rather than adding the file to the rule's LEGACY_BASELINE map — that map lives in an upstream-owned file (oxlint-plugin-t3code/rules/no-manual-effect-runtime-in-tests.ts), and editing it would open a new, undocumented seam.

Results

  • vp run typecheckgreen (15/15 packages; only pre-existing informational suggestion-level diagnostics remain on unmodified upstream code)
  • vp run lintgreen (0 errors; only pre-existing warnings on unmodified files remain)
  • vp run testgreen (1705 passed, 7 skipped, 0 failed, across all 14 test tasks)

🤖 Generated with Claude Code

@radroid

radroid commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Automated review (Claude, on Raj's behalf) — resolver sync PR

TL;DR: the rebase and conflict resolution are high quality — I couldn't find anything wrong with the code. All 37 fork patches are intact, the real conflicts were resolved correctly, the resolver's test fixups don't weaken anything, and there's no seam drift. What needs your attention before merging is how to land it and the lack of an independent CI gate — not the resolution.


⚠️ Needs attention before merging

1. 🚫 Do NOT use the Merge / Squash / Rebase button — it can't work here (by design).
This branch is the fork's patch series rebased onto new upstream, so origin/main is not an ancestor of it. GitHub reports the PR CONFLICTING / DIRTY (234 files) with add/add conflicts on the fork's own files (CrashRecoveryReconciler.test.ts, http.test.ts, SettingsPanels.tsx, auto-build-runbook.md). The button will never go green — this branch is meant to replace main's history, not extend it.

Correct way to land (make main become the reviewed tip 1acb55187):

git fetch origin
git push --force-with-lease origin origin/t3x/sync-30235353352:main

There is no branch protection on main, so this is allowed — but also unguarded; your only rollback is the t3x/last-good-* recovery tag. Then close this PR manually (it won't auto-mark as merged after a force-update):

gh pr close 22 --comment "Landed by force-updating main to reviewed tip 1acb55187"

2. Four in-flight worktree branches will be stranded. After main jumps forward ~36 commits, these all sit on pre-sync history and must be rebased onto the new main before their PRs can land: t3x/issue-21-ws-keepalive-reconnect-heap, t3code/chat-input-notifications, t3code/web-push-notifications, t3x/mobile-keyboard-queue. Your local main is also stale (5 behind) — reset it to the new tip.

3. No independent CI gate on this PR. ci.yml runs on blacksmith-* runners the fork can't access, so the PR shows "no checks." The only evidence that typecheck/lint/test pass is the resolver's own job log (I checked — no continue-on-error, no test filtering, no forced exit-0). If you want a real gate, run this on the tip before landing:

git switch --detach origin/t3x/sync-30235353352 && vp run typecheck && vp run lint && vp run test

4. Doc gap (follow-up, not blocking). Both the runbook rewrite (step 6) and the design spec say "a human merges the PR" — but a rebased sync branch can't be landed via the button. Worth adding a "How to land a sync PR" section documenting the force-update step + the worktree fallout above.


✅ What I verified is solid

  • All 37 fork patches present, none dropped or emptied. Only 3 needed adaptation; the 12 old PR-merge commits were correctly flattened. One new resolver commit (1acb55187), nothing stray.
  • SettingsPanels.tsx (the one real code conflict): both upstream's sidebarProjectGroupingMode and the fork's notifyOnNeedsInput are preserved in all four places (restore labels, useMemo deps, restore defaults, UI rows). The dependency array is complete — no stale-closure bug.
  • Resolver's test fixups (1acb55187) do not weaken tests. http.test.ts merges two chained Effect.provides into one Layer.merge — provably equivalent here (the two layers are independent, no cross-dependency). CrashRecoveryReconciler.test.ts gets one scoped, justified oxlint-disable-next-line matching existing precedent (DesktopBackendConfiguration.test.ts). No assertions changed; both fixed reds were pre-existing on main.
  • No seam drift — even though upstream rewrote decider.ts by +203/−58, the fork's mirrored hasOpenBlockingRequest predicate in guards.ts is byte-identical to upstream's current logic (same activity kinds + stale-detail strings). The resolver's "none found" was a real check, not a rubber stamp.
  • SEAMS.md conflict was whitespace / table-formatting only.

Net: the resolution is trustworthy — get the landing mechanics right (item 1) and mind the worktree fallout (item 2), and this is good to ship.

🤖 Generated with Claude Code — automated review run while you were away.

@radroid
radroid merged commit 6dc983f into main Jul 27, 2026
@radroid
radroid deleted the t3x/sync-30235353352 branch July 27, 2026 13:22
radroid added a commit that referenced this pull request Jul 28, 2026
…tton)

Closes the review gap on PR #22: the runbook and design spec said "a human
merges the PR" but a resolver branch is the fork's series rebased onto new
upstream, so main is not its ancestor and GitHub's Merge/Squash/Rebase button
reports CONFLICTING and cannot land it.

Add a "Landing a sync PR" section to the runbook (force-update main via
--force-with-lease, close the PR manually, no branch protection = unguarded,
no independent CI on fork PRs so re-gate locally, and rebase the other
in-flight worktree branches onto the new main). Point the two "a human merges"
references and the design-spec flow at it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant