Skip to content

fix(server): threads settle right away when a watched PR merges - #26395

Merged
t3dotgg merged 1 commit into
mainfrom
t3/pr-watch-merged-settle
Oct 28, 2026
Merged

t3dotgg merged 1 commit into
mainfrom
t3/pr-watch-merged-settle

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Oct 28, 2026

Copy link
Copy Markdown
Member

A thread with a PR watch did not settle when its PR merged. It sat in "Waiting on Watching pull request #N" for up to 2 minutes. In one case the agent merged #26387 at 10:00:02 and the server synced the merged snapshot at 10:00:03. The turn ended at 10:00:13, but the thread settled only at 10:01:30.

The cause: a watched link counts as awaited background work, and awaited work blocks auto-settle. The watch reactor ends a merged PR's watch without waking the agent, but only on its next 2-minute sweep. So the watch held the thread for nothing.

The fix: a watch whose link snapshot is already merged no longer counts as pending work. This gives:

  • The agent merges (for example with merge with cleanup or full send): the thread settles when the turn ends.
  • You merge it yourself without opening the thread: it settles as soon as the merged snapshot syncs, with no wake and no tokens. Main already did this with no tokens, but up to 2 minutes late.
  • Closed PRs still hold. The reactor wakes the agent for a close, and settling first would drop that message.
  • Awaited post-merge waits (t3_background_wait for CI or deploy) still hold the thread, so the bug from #24681 does not come back.

Proof: the existing watch test table gets a merged row and a closed row. It fails on main and passes here. I replayed the real thread state at 10:00:13 through derivePendingBackgroundWork. On main it returns the watch task, which holds the thread. With this fix it returns nothing. vp run check:changed passes.

Policy conflict: #26271 and #25181 go the other way: a merge wakes the agent, and #25181 also defers settlement while a merged watch is active. This PR assumes merges stay silent. Only one policy should land.

Council review

Rounds: 2

Delegate Round 1 Round 2
Claude Opus 5.5 8/10 9/10
Muse Wonder Full 9/10 9/10
Codex GPT-6.1 Sol 9/10 10/10

Made with Claude Opus 5.5 in Claude Code, run from T3 Code.

🤖 Generated with Claude Code

A PR watch held auto-settle until the watch reactor's next 2-minute sweep,
even after the merged snapshot had synced. The reactor ends merged watches
without a wake, so a merged link's watch no longer holds the thread.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 28, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 0881343

Macroscope's review found this PR approvable — Merged PR watches no longer keep threads pending while watch cleanup catches up. This aligns completion bookkeeping with the existing watch lifecycle and leaves other pending work unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 40d335a and 0881343.

Files selected for processing: 2
  • packages/shared/src/orchestrationV2PendingBackgroundWork.test.ts
  • packages/shared/src/orchestrationV2PendingBackgroundWork.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Walkthrough

The pending-work calculation now uses pull request snapshot state to omit watch tasks for merged pull requests. Tests add merged and closed pull request fixtures and expect closed pull requests to remain in pending-work results.

Changes: Pull request watch filtering

Layer / File(s) Summary
State-aware pull request watch filtering
packages/shared/src/orchestrationV2PendingBackgroundWork.ts, packages/shared/src/orchestrationV2PendingBackgroundWork.test.ts
The pull request type now includes a nullable snapshot state. The pending-work calculation omits watches for merged pull requests. Tests verify that closed pull requests remain in pending-work results.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#25160: This change omits merged pull request watches from pending background work. The issue requests a merge wake for verification.
  • pingdotgg/t3code#26083: This change modifies how merged pull request watches affect pending background work. The issue describes a race that can lose the post-merge wake.

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: Minimal · up to 08813

The change removes completed merged-PR watches without dropping closed-PR or unrelated pending work, so no actionable merge risk remains.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description clearly explains the problem, fix, behavior for merged and closed pull requests, verification, and policy conflict. It does not provide the required Scope and approval information or e… Add a Scope and approval section. Link the triaged issue or maintainer approval with the approval comment, or explain why this focused fix qualifies as an obvious bug without prior approval. Organize the existing content under the required …
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: threads settle immediately when a watched pull request merges.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the problem, fix, behavior for merged and closed pull requests, verification, and policy conflict. It does not provide the required Scope and approval information or explain why the change qualifies for the small, obvious bug exemption.

Resolution

Add a Scope and approval section. Link the triaged issue or maintainer approval with the approval comment, or explain why this focused fix qualifies as an obvious bug without prior approval. Organize the existing content under the required Problem, Change, Scope and approval, and Verification headings.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@t3dotgg
t3dotgg enabled auto-merge (squash) October 28, 2026 10:23
@t3dotgg
t3dotgg merged commit 2a73827 into main Oct 28, 2026
30 checks passed
@t3dotgg
t3dotgg deleted the t3/pr-watch-merged-settle branch October 28, 2026 10:25
@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 5.0 KiB 5.0 KiB 0 B (0.0%) 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 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.6 KiB 21.6 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.1 KiB +6 B (+0.1%) 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.3 KiB 1.3 KiB +6 B (+0.5%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 40d335a · PR result: 0881343 · 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.

voida6 added a commit to voida6/t3code that referenced this pull request Oct 28, 2026
Main now skips merged PRs in pullRequestWatchTasks (pingdotgg#26395). That check
moves into the shared isWatchedLink helper so the monitor row lookup
follows the same rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pipyakas pushed a commit to Pipyakas/t3code that referenced this pull request Oct 28, 2026
…dotgg#26395)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 2a73827)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). 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