fix(server): preserve cancelled-run history when switching providers #34045 · pingdotgg/t3code

MmacroscopeappBotapproved these changes7h ago

Approvability

Verdict: Approved at 0bc4f8f

Macroscope's review found this PR approvable — Preserves history from started, cancelled runs when switching providers, while excluding runs cancelled before starting. Existing thread boundaries and provider-handoff policy checks remain unchanged.

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

CcoderabbitaiBot7h ago

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Started cancelled runs now qualify as provider handoff sources when they have a start timestamp. Integration tests cover immediate and queued handoffs, and verify that a request cancelled before starting is excluded from delivered history.

Changes: Cancelled-run handoff history

Layer / File(s)	Summary
Handoff eligibility and integration coverage `apps/server/src/orchestration-v2/Orchestrator.ts`, `apps/server/src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts`	The handoff predicate accepts cancelled runs with a start timestamp. Integration tests cover immediate and queued handoffs and check that a queued request cancelled before starting is absent from delivered history.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

pingdotgg/t3code#34016: Both PRs change isHandoffSourceRun for started cancelled runs and test handoff history.

Suggested labels: bugfix, size:S

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to 0bc4f

No material merge-blocking risk was identified. Targeted tests were reported passing, but the full changed-check run remains incomplete.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title clearly summarizes the main change: preserving cancelled-run history when switching providers.
Description check	Passed	The description explains the problem and change, links the issue, states why the fix is focused, and gives detailed verification results and limitations. It uses “Summary” and “Evidence” instead of th…
Linked Issues check	Passed	Issue `#34009` is closed and completed, so it supplies historical context only. No active directly linked issue imposes coding requirements on this pull request.
Out of Scope Changes check	Passed	The change is limited to provider-handoff eligibility for cancelled runs and regression tests for that behavior. The implementation includes cancelled runs only when startedAt is non-null, and the tes…

Full details: Description check

Explanation

The description explains the problem and change, links the issue, states why the fix is focused, and gives detailed verification results and limitations. It uses “Summary” and “Evidence” instead of the template’s section headings, but the required information is mostly present.

Full details: Out of Scope Changes check

Explanation

The change is limited to provider-handoff eligibility for cancelled runs and regression tests for that behavior. The implementation includes cancelled runs only when startedAt is non-null, and the tests cover exclusion of a queued request cancelled before starting. These changes support the pull request's stated history-preservation intent; no unrelated changes are identified.

Finishing Touches

Generate unit tests (beta)

Create a new PR

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

External image Recent review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: main
File: .coderabbit.config.ts
Commit: d756fed
Layers: Repository TS config
Review profile: CHILL
Plan: Advanced
Run ID: `94d33896-a858-48d7-b52c-db3060694c83`

External image Commits

Reviewing files that changed from the base of the PR and between d756fed7aaac06134bc68cbe0047e6f14b0e9aa2 and 0bc4f8f463c37734c86e8a89d36952aea689e64f.

External image Files selected for processing: 2

`apps/server/src/orchestration-v2/Orchestrator.ts`
`apps/server/src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts`

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

JJulius Marminge7h ago

Thanks for this. Main has the change since #34016: Compared the complete diff of #34045 with merged main: its only production change is the identical isHandoffSourceRun condition accepting cancelled runs when startedAt is non-null, covering immediate switches, queued promotion and returning-provider delivery tracking; remaining differences add regression coverage rather than behavior.

Closing as superseded.

JJulius Marminge7h ago

Thanks for this. Main already has the change, as of d756fed7aaac06134bc68cbe0047e6f14b0e9aa2: Main already contains this fix in d756fed7aa, landed through #34016. The complete diff's only production change is the identical condition accepting cancelled runs when startedAt is non-null. That shared predicate covers immediate switches, queued promotion, and returning-provider delivery tracking. Remaining differences extend tests rather than behavior. This is the only mainline landing touching the two changed files since the comparison base; there are no merge commits in that range. The linked #34009 is already completed, and the PR is already closed as superseded. All 37 pages of open PRs were scanned.

Closing as superseded.

fix(server): preserve cancelled-run history when switching providers#34045

Summary

Evidence

Merge Danger

Checks

Approvability

1	vp test run src/orchestration-v2/testkit/ProviderSwitch.integration.test.ts src/orchestration-v2/ContextHandoffService.test.ts --silent
2	vp run typecheck