fix(threads): a merged PR settles its thread only once - #7454
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 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.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — Bug fix ensuring merged PRs settle threads only once by tracking PR timestamps against thread activity. The author originally created this settling logic, and the fix includes comprehensive tests for edge cases (inherited old PRs, revived threads, missing timestamps). You can add or adjust custom eligibility rules. Learn more. |
Dismissing prior approval to re-evaluate 187e7b0
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 187e7b0. Configure here.
## What's Changed * fix(desktop): throttle hidden preview rendering by @t3-code[bot] in pingdotgg/t3code#7445 * fix(server): stop probing Grok, Cursor, and OpenCode unless turned on by @t3dotgg in pingdotgg/t3code#7459 * fix(desktop): boot the main window unthrottled so cold start paints at full speed by @t3dotgg in pingdotgg/t3code#7460 * fix(threads): a merged PR settles its thread only once by @t3dotgg in pingdotgg/t3code#7454 * feat(cli): npx t3 triage hands broken installs to your own coding agent by @t3dotgg in pingdotgg/t3code#6563 **Full Changelog**: pingdotgg/t3code@v0.0.34-nightly.20260819.1129...v0.0.34-nightly.20260819.1130 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.34-nightly.20260819.1130
pingdotgg#7437, pingdotgg#7459, pingdotgg#7460, pingdotgg#7445, pingdotgg#7122, pingdotgg#7317, pingdotgg#7381, pingdotgg#6563) Where upstream reworked something the fork had already built, upstream's version wins and the fork's extras ride on top: - Browser tab mute (pingdotgg#7252): upstream's rollback-on-refusal setAudioMuted and tabMuteMenuItem replace the fork's earlier copies; the fork's guest viewport override (setViewport/automationSetViewport) stays. - Passkey autofill (pingdotgg#7437): upstream's `passkeys` const replaces the fork's duplicate manualOnlyPasskeys. - Settle-once-on-merge (pingdotgg#7454): the sidebar now reports the whole change request (state + updatedAt) instead of a bare state, so the new rules can tell a fresh merge from inherited branch history. The fork's per-row reporting stays; upstream's semantics win. - Merge action labels (pingdotgg#7381): upstream's confirmation state object, extended with the fork's merge-stack action. Also: Clerk v6 moved `layout` into `options`, so the appearance override moves with it, and the connect-providers helper joins the web tsconfig include list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sync a4cc136..5ea5a80 (20 commits). Upstream's once-only merged-PR settle rule (pingdotgg#7454) reshaped changeRequestAutoSettles/effectiveSettled to take a change-request object plus thread context; the fork's autoSettleEnabled master gate is kept ahead of it at every call site, including the fork-only command-palette one upstream could not migrate. Two integration fixes a clean textual merge would not have caught: the failover-target list now resolves enablement through resolveProviderInstanceEnabled, so a driver upstream just made default-off is no longer offered as a target the server refuses; and t3-thread-background's test was asserting a liveness quirk pingdotgg#7172 fixed (the tool replays through the real registry, so it inherits the fix).
(cherry picked from commit f21b47e)
* test(web): remove duplicate lookup assertion (pingdotgg#7364) * fix(mobile): show structured input option descriptions (pingdotgg#7321) * fix(orchestration): do not revive idle tasks from status-free progress (pingdotgg#7172) * refactor(server): simplify error transformation with Effect.mapError in GitHubPullRequestCli (pingdotgg#7385) Signed-off-by: aoright <102943475+aoright@users.noreply.github.com> * fix(preview): open local environment ports on localhost (pingdotgg#7300) * fix(desktop): prevent quit shortcut spillover (pingdotgg#7397) * fix(desktop): stop overwriting a custom dock icon on launch (pingdotgg#7125) * feat(web): show project location in new thread picker (pingdotgg#7392) Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com> * fix(packaging): install AUR launcher icons where icon themes look (pingdotgg#7421) * fix(web): label pull request merge actions (pingdotgg#7381) * fix(server): avoid PRs inherited from default upstreams (pingdotgg#7317) * fix(desktop): stop the passkey dialog from popping as soon as sign-in opens (pingdotgg#7437) * feat(desktop): mute a browser tab (pingdotgg#7252) * fix(web): improve disconnected composer placeholder (pingdotgg#7122) Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com> Co-authored-by: maria <254055478+maria-rcks@users.noreply.github.com> * fix(desktop): throttle hidden preview rendering (pingdotgg#7445) Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com> Co-authored-by: maria <254055478+maria-rcks@users.noreply.github.com> * fix(server): stop probing Grok, Cursor, and OpenCode unless turned on (pingdotgg#7459) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(desktop): boot the main window unthrottled so cold start paints at full speed (pingdotgg#7460) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(threads): a merged PR settles its thread only once (pingdotgg#7454) * feat(cli): npx t3 triage hands broken installs to your own coding agent (pingdotgg#6563) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(marketing): Safari gets the arm64 Mac download (pingdotgg#7473) * feat(web): add shortcuts to the surface dropdown (pingdotgg#7318) * fix(marketing): never serve the Intel build to Apple Silicon Macs (pingdotgg#7477) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(web): animate command palette when closing (pingdotgg#5169) * fix(desktop): upgrade Clerk OAuth transport (pingdotgg#7479) * feat(server): run the background service on macOS via launchd (pingdotgg#6286) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(web): align sidebar statuses with project names (pingdotgg#7491) Co-authored-by: GPT-5.6 <noreply@openai.com> * fix(desktop): close the window before quit cleanup (pingdotgg#6562) * fix(desktop): stop automatic passkey prompts (pingdotgg#7522) * docs(user): document phoenix triage and macOS background service The 2026-08-19 upstream sync added the triage command with no docs/user entry, and the docs index still called the background service Linux-only after launchd support landed. Found by PR #61 code review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Signed-off-by: aoright <102943475+aoright@users.noreply.github.com> Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com> Co-authored-by: Nick Anisimov <n.anisimov.23@gmail.com> Co-authored-by: Maslin Edwin <maslinje@gmail.com> Co-authored-by: aoright <102943475+aoright@users.noreply.github.com> Co-authored-by: Guilherme Barros <gbarros1095@gmail.com> Co-authored-by: Bilal Bakr <62337003+Bil0000@users.noreply.github.com> Co-authored-by: Rishet11 <154429365+Rishet11@users.noreply.github.com> Co-authored-by: Wout Stiens <71498452+StiensWout@users.noreply.github.com> Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com> Co-authored-by: Augie <augie@luebbers.email> Co-authored-by: Taras <Taras.Fomin@gmail.com> Co-authored-by: Gianmarco <gianmarcosimone89@gmail.com> Co-authored-by: Theo Browne <me@t3.gg> Co-authored-by: Chris Deeming <chris@xenforo.com> Co-authored-by: Inaya Yousfi <zied.essaber@gmail.com> Co-authored-by: maria <254055478+maria-rcks@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Rakshith Bhat <88523594+RakshithBhat03@users.noreply.github.com> Co-authored-by: GPT-5.6 <noreply@openai.com>
Range: 3b5d476..f2d5fc9 (21 upstream commits, merge-base 3b5d476). Shape: 23 files touched by both sides, 63 upstream-only, 525 fork-only. Zero conflicts — git found no textual overlap, so the audit below is the only thing that could have caught a bad merge. Conflicts: none. Every upstream-only file is byte-identical to origin/main and every fork-only file byte-identical to personal (verified by blob hash, not by inspection); only the 23 shared files were actually merged. Notable upstream changes landing on fork-rewritten files: - pingdotgg#7454 changed changeRequestAutoSettles from (state, autoSettleOnMerge) to (pr, {autoSettleOnMerge, thread}) so a merged PR settles its thread once. Both call sites live in fork-rewritten files (Sidebar.tsx +467 vs upstream +8, ChatView.tsx +550 vs upstream +36) and took the new shape. - pingdotgg#7318/pingdotgg#7252 rewrote RightPanelTabs.tsx (upstream 231/58 against the fork's 13/1). The fork's two "trustedFile" cases survived inside both surface switches — verified by reading the merged switches, since a line-level sweep cannot tell a surviving line from a reachable one. - pingdotgg#7459 turns Grok/Cursor/OpenCode probing off by default and folds the legacy in-config "enabled" flag into the envelope. Orthogonal to the fork's probe-cache pinning; unknown (fork) drivers default to enabled. - pingdotgg#7122 moved the disconnected composer placeholder into a shared constant. Its text is neutral about sending, so it does not contradict the fork's offline outbox. - pingdotgg#6286 adds launchd management under the label com.t3tools.t3code.service, distinct from this machine's hand-rolled com.t3code.server, so it cannot disturb the running install. - Clerk bump (pingdotgg#7522/pingdotgg#7479/pingdotgg#7437) rewrote the catalog and minimumReleaseAgeExclude blocks but not patchedDependencies. Invariants (docs/fork/README.md), all re-probed against the merged tree: 1. Migrations: 45 entries, ids unique and monotonic, max 46, id 34 still burned, filename numbers 033/037/038/039 still duplicated. PASS. 2. Fork-owned @effect/platform-node patch still pinned in patchedDependencies with its FORK-ONLY comment; patch file intact and carried in the lockfile by patch_hash. No effect bump in this range. PASS. 3. Sidebar default unflipped: AppSidebarLayout renders ThreadSidebar unless legacySidebarEnabled. Fork edits landed in Sidebar.tsx, the rendered one. PASS. 4. No fork-deleted line came back (sweep resurrected=0). PASS. 5. Upstream's "steers a running turn" ClaudeAdapter test still absent, with its explanatory comment. PASS. 6. Both project entry points still on the row: ellipsis opening the fork's project-actions dialog and gear navigating to /projects/$projectKey. PASS. Sweep (all three directions, 23 shared files): resurrected 0, dropped 0, fork-loss 0. Install: full pnpm install, 13 added / 7 removed, pnpm-lock.yaml unchanged afterwards — the lockfile committed here is the one the gate verified.

Two ways a merged PR wrongly hid a thread:
Both are the same mistake: the settle rule treated any merged/closed PR as "this thread's work is done", forever. Settling on a merge should happen once. The status PR now carries the provider's
updatedAt, and a terminal PR only auto-settles a thread while it is the thread's latest event. A new thread created after the merge, or any message sent after it, outranks the merge for good. Inactivity auto-settle and manual settle still work as before.updatedAton the status change request in contracts; old servers and clients interop unchanged.client-runtimeanchors on the thread's latest activity (falling back to creation time). Applied on web (sidebar partition, Woke pill, parked banner, header menu) and mobile (both thread lists).Tests: age-gate and revival cases in
threadSettled.test.ts, updatedGitManager, web, and mobile suites. All targeted typechecks, lint, and format pass.Changes by Claude Fable 5 running in Claude Code.
Note
Medium Risk
Changes when threads appear in settled lists across web and mobile; behavior is gated on new optional
updatedAtwith safe fallbacks when absent.Overview
Fixes threads being auto-settled (or re-settled) when the branch’s merged/closed PR predates the conversation—e.g. new threads on an already-merged branch, or revived threads after a merge.
changeRequestAutoSettlesnow takesChangeRequestSettleSource(state + optionalupdatedAt) and only treats a terminal PR as settling the thread whenupdatedAt≥ the thread’s latest user-initiated activity (messages / turn requests, else creation). Missing or bad timestamps keep prior always-settle behavior for compatibility.Contracts and
GitManagerexpose optional PRupdatedAtfromgh; web and mobile thread lists, sidebar partition, Woke pill, parked banner, and action menus pass the full change request (and thread shell) intoeffectiveSettledinstead of bare PR state.Reviewed by Cursor Bugbot for commit 26af072. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix thread auto-settlement to only trigger when a merged PR postdates thread user activity
ChangeRequestSettleSource(state +updatedAt) throughout the stack, replacing plain PR state strings in settlement logic.changeRequestAutoSettlesnow compares the PR'supdatedAtagainst the thread's user-activity anchor (createdAt,latestUserMessageAt,latestTurn.requestedAt), skipping settlement when the terminal PR predates user engagement.toStatusPrand theVcsStatusChangeRequestcontract schema are extended to includeupdatedAt, propagated from the VCS provider.Macroscope summarized 26af072.