[saphid]

Problem

When an agent watches a pull request and the branch conflicts with its base, the watch wakes the agent once: "The branch now conflicts with main." The agent rebases and pushes. If that push does not fix the conflict, the watch stays silent. It only speaks again after the branch has been clean and then conflicts again.

So the agent ends its turn expecting to be woken on the next change, and never learns that its fix didn't work. Failed checks already behave the way the agent expects: a push resets them, and a check that fails again on the new head wakes the agent.

Change

evaluatePullRequestWatch now treats the conflict it already reported as belonging to one head, the same way failedChecks does. When the head moves, a conflict the host still reports is news again. While the host recomputes after the push, "unknown" mergeability still keeps the reported state, now for the current head only. Re-watching the same head still restores the ended watch's headSha and conflicting, so it does not repeat a conflict the thread was already told about.

A detail with no headSha now keeps the watch's last known head instead of counting as a push. Azure DevOps omits the head on purpose when its iteration read fails (#20977). Without this, a conflicting Azure PR would re-wake the agent with "conflicting" on each failed read and again when the head came back. The reactor's watch-life telemetry (pushed) keeps the last head the same way.

Scope and approval

Very small, focused fix for an obvious bug, so no prior issue: one wake transition is missing (a push that leaves the branch conflicting), and the sibling check-failure path already handles the same case. 6 non-test lines plus one extended test case. The missing-head part is included because this PR is what makes the Azure re-wake reachable.

Overlap search: local PR index and REST file diffs for the open PR-watch PRs (#28894, #28200, #28020, #27953, #26310, #26271, #25539, #25181, #24981, #24599, #24568, #24141, #23522, #21636, #21239). None change how the reported conflict carries over when the head moves. #23522 baselines existing conflicts at watch start, and #24141 adds merge-queue removal. One gh search prs query returned nothing.

Verification

• Red on main: extended reports a conflict once, until the branch is clean again in pullRequestWatch.test.ts. It fails with AssertionError: expected [] to deeply equal [ { kind: 'conflicting' } ].
• Red for the missing head (before the second commit): a read with no headSha after a reported conflict fails with AssertionError: expected [ { kind: 'conflicting' } ] to deeply equal [].
• Green: vp test run src/orchestration-v2/pullRequestWatch.test.ts src/orchestration-v2/runtimeLayer.test.ts → 132 passed.
• vp run check:changed: format, lint, knip, graphql, typecheck, tests all pass.
• After the second commit: vp test run src/orchestration-v2/pullRequestWatch.test.ts → 20 passed; vp run check:changed passes again (tests: 2 files, 21 passed).
• Not checked: a live watch against a real host's conflict after a force-push, or a live Azure DevOps iteration-read failure.

Reviewed with sol-loop: 2 rounds with GPT-6.1 Sol on high (the second on the full diff with the missing-head fix). Sol verdict: approved.

Created with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at 6809d98

Macroscope's review found this PR approvable — PR watches will report a conflict again when a new push leaves it unresolved, while missing host head SHAs no longer look like pushes. The change stays within existing watch and wake behavior without changing product defaults or permissions.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 61c3794
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: dcd30324-43c7-4f49-91e8-dd5c072bcf4a

Commits

Reviewing files that changed from the base of the PR and between 6809d98 and 65ba6a2.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The pull request watch retains the saved head SHA when the host omits it. Conflict reporting and saved conflict state now account for whether the head has changed. Tests cover reads without a head SHA and conflict recomputation after a head change.

Changes: Conflict tracking

Layer / File(s)	Summary
Handle conflict state across head changes apps/server/src/orchestration-v2/pullRequestWatch.ts, apps/server/src/orchestration-v2/PullRequestWatchReactor.ts, apps/server/src/orchestration-v2/pullRequestWatch.test.ts	The watch retains the saved head SHA when the host omits one. Conflict reporting and preservation of conflict state now depend on whether the head has changed. Tests cover an omitted head SHA and a new head that first has unknown, then conflicting, mergeability.

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

Change: Bug fix

Possibly related issues

• [Bug]: On github.com, PR sync rewrites every open linked PR twice after each base-branch push, because GitHub briefly reports mergeable UNKNOWN #23860: The issue describes repeated updates during a transient unknown mergeability result after a push. This change covers that host state in the watch evaluator, but the issue concerns the sync snapshot path.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · previous review 6809d

The reported conflict replay issue is addressed, and no additional actionable merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: PR watches wake again when a push leaves the branch conflicting.
Description check		The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the bug, implementation, rationale, approval basis, test results, and untested live scenar…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the bug, implementation, rationale, approval basis, test results, and untested live scenarios.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 5b9731c
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6a92b1eb-d7e1-42d7-919b-0f77ade2d66c

Commits

Reviewing files that changed from the base of the PR and between 4db326b and 6809d98.

Files selected for processing: 3

• apps/server/src/orchestration-v2/PullRequestWatchReactor.ts
• apps/server/src/orchestration-v2/pullRequestWatch.test.ts
• apps/server/src/orchestration-v2/pullRequestWatch.ts

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

[coderabbitai]

• Fix comments: Fix CodeRabbit comments on this PR

Dismissing prior approval to re-evaluate 6809d98

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

partial verification at 65ba6a236612db957203a22a358e1e661306c465 on a linux boat vm. this does not verify the conflict-after-push fix.

ran github/main (61c3794c32) in a separate worktree first, then the exact pr head, with vp i, isolated t3 homes and one background dev server at a time. used playwright from the repo with /usr/bin/google-chrome, real projects, and real codex gpt-6.1-sol turns through the local proxy. no mocks, fixture state or demo provider.

• the real watch_pull_request call registered the watch. the scheduled reactor poll delivered "fix(server): PR watches wake again when a push leaves the branch conflicting #29767: checks passed", resumed codex, and its reply appeared in the web client.
• clicking the web client's stop-watching control removed the waiting state while retaining the saved head and passed-check state.
• requesting rewatch through the web composer called the provider's mcp tool and restored that state. the next scheduled poll produced no duplicate wake. reactor telemetry: first watch reads=1, wakes=1; restarted watch reads=1, wakes=0.
• blacksmith ran the existing pullRequestWatch.test.ts and runtimeLayer.test.ts: 132 tests passed. github ci is green at this head. no source changes or pushes.

30-second interaction recording, with idle time between stop and rewatch removed:

command and runtime transcript

Not verified

the original bug on main and the changed conflict → new push → still-conflicting wake remain unverified. the only permitted pr is currently MERGEABLE. reproducing that sequence requires a controlled conflicting pr or deliberately introducing a conflict here; neither was authorized under the restriction against touching other prs. clean-pr watch results and green tests do not prove this fix.

also unverified: transient unknown mergeability after a push, a real azure devops iteration-read failure omitting the head, and native desktop/ios/android clients. verification here covered the linux web client and codex provider path.

[saphid]

Note

🤖 Claude Opus 5.5 responding on behalf of saphid

Thanks. Agreed that a clean PR can't exercise this. At the evaluator layer the sequence is pinned exactly. In pullRequestWatch.test.ts ("evaluatePullRequestWatch"), the watch sees a conflicting PR and wakes. Then a push arrives with a new head and mergeability: "unknown", then the same new head comes back still conflicting. With main's pullRequestWatch.ts swapped in, that last step fails:

AssertionError: expected [] to deeply equal [ { kind: 'conflicting' } ]
 ❯ src/orchestration-v2/pullRequestWatch.test.ts:365:12
Tests  1 failed | 19 passed (20)

On this head, 20/20 pass. The same test also covers a poll that omits the head (as the Azure DevOps iteration-read failure does), which must not wake. If you want a live run, I can set up a deliberately conflicting scratch PR on the saphid fork, so no upstream PR is touched, and capture main against this head. Just say if that's worth doing.