[Narayan201120]

Split from #27423 per one-problem-per-PR. Test-only change; no production code is touched.

cancelUnsettled runs inside the commit transaction, so awaitSettled waiters must resolve only via the post-commit signalCancellations path. An earlier revision of that PR signalled inside the cancel itself; review showed a rollback after the signal would leave waiters resolved while rows stayed unsettled, so the signal stays post-commit.

This test pins the boundary: a waiter registered before the cancel stays pending through the cancel alone and resolves after the post-commit signal.

Verification

• FoundationPersistence tests: 47 passed, including the new boundary test (failed before the correct boundary was established)
• Formatting and lint passed for the changed file
• Knip passes passed
• Typecheck passed for apps/server

Made with GPT-6-Luna in Codex, running in T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at e25d16f

Macroscope's review found this PR approvable — This test-only change pins the post-commit outbox cancellation boundary. It changes neither production behavior nor product defaults and introduces no diagnostic suppressions.

Notes:

• No code objects were reviewed. Approvability was decided on eligibility alone.

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

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 32ecd71
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 466e8fdc-83c3-4f58-a627-6b2f1a2a4405

Commits

Reviewing files that changed from the base of the PR and between 32ecd71 and e25d16f.

Files selected for processing: 1

• apps/server/src/orchestration-v2/FoundationPersistence.test.ts

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

Walkthrough

This change adds a test for awaitSettled during cancellation. The test checks that the waiter remains pending after cancelUnsettled and resolves after signalCancellations.

Changes: Cancellation settlement

Layer / File(s)	Summary
Waiter settlement test apps/server/src/orchestration-v2/FoundationPersistence.test.ts	The test enqueues padding effects, cancels a provider-turn effect, and verifies that the waiter remains pending until signalCancellations is called.

Priority: Low

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

Change: Other

Suggested reviewers: juliusmarminge, macroscopeapp

Merge Risk: Minimal · up to e25d1

The cancellation-settlement test has no identified merge-blocking issue. Merge after normal checks.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description covers the problem and the change clearly. It states that the PR is test-only and that it is split from #27423 under the one-problem-per-PR rule. It explains why the signal must stay p…	Add a Scope and approval section. Link the triaged issue or the maintainer-approved discussion, including the approval comment. If no prior issue exists, explain why this test-only change qualifies without one. Correct 'Knip passes passed' …

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title names the change accurately. The PR adds one test that pins the post-commit settle boundary for outbox waiters. The title follows the conventional commit format and names the test scope (ser…
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: Title check

Explanation

The title names the change accurately. The PR adds one test that pins the post-commit settle boundary for outbox waiters. The title follows the conventional commit format and names the test scope (server) and the behavior under test. It is specific enough for a teammate scanning history to understand the change.

Full details: Description check

Explanation

The description covers the problem and the change clearly. It states that the PR is test-only and that it is split from #27423 under the one-problem-per-PR rule. It explains why the signal must stay post-commit. The Verification section gives concrete results: 47 tests passed, the new test failed before the boundary was established, and formatting, lint, Knip, and typecheck passed. The template also asks for a Scope and approval section. The description does not include one. It links no triaged issue and no maintainer approval comment. It also does not explain why this focused test-only change qualifies without prior approval. The description has a typo, 'Knip passes passed'. The closing line names the model and harness, as the template requests. The Problem heading is not labeled as such, but the first paragraph covers the context.

Resolution

Add a Scope and approval section. Link the triaged issue or the maintainer-approved discussion, including the approval comment. If no prior issue exists, explain why this test-only change qualifies without one. Correct 'Knip passes passed' to 'Knip passed'. Consider a short Problem heading that states the gap the test closes, which is the lack of a test that pins the post-commit settle boundary.

Finishing Touches

🧪 Generate unit tests (beta)

• 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.}

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified the test-only change at e25d16f995b441b845fccd431d25530fe35ec19a with real codex gpt-6.1-sol turns through the local proxy. no code changes or pushes were needed.

first ran github/main (5fe18d79ac835e0f193ad61325e5b7d69a317bcf) in a separate worktree, then the exact pr head, installing each with vp i. servers ran in the background, one at a time, with isolated state. used the repository playwright dependency and /usr/bin/google-chrome. added a project through the app pointing at a real git repository and created its threads and turns through the client.

on both revisions:

• a real codex turn read README.md and returned BASELINE_READY.
• sent thread.stop over the authenticated client websocket immediately after message.dispatch succeeded, to reach the narrow pre-start cancellation path. the client showed "run interrupted before provider start" and the persisted provider-turn.start effect was cancelled. the next turn returned AFTER_CANCEL_OK.
• clicked stop while a real codex terminal command was running. the client showed "stopped by user", the interrupt effect succeeded, and another turn returned AFTER_STOP_OK.

read-only database checks found zero pending/running outbox effects afterward on both revisions. runtime transcript.

blacksmith: vp test run apps/server/src/orchestration-v2/FoundationPersistence.test.ts
47 passed
premature-signal mutation: added test failed at waiterFiber.pollUnsafe()
mutation restored: 47 passed again

main and the pr have identical EffectOutbox.ts and EventSink.ts. no original application failure reproduced on main; this pr adds coverage rather than changing application behavior. the temporary mutation ran only on the remote check worker, never in either dev app.

main cancellation:

pr cancellation:

Not verified

• the internal waiter/transaction boundary is covered by the focused test and mutation check, not directly observable through the client. the new test does not open a transaction or exercise rollback; its padding enqueues are a scheduling heuristic. no SQL rollback was injected into the running app.
• native ios/android, electron, macos/windows, other providers, remote/relay/tunnel connections, and restart-continuation waiters were not exercised on this linux vm.
• the first new-thread reply on this historical head persisted successfully but needed a reload to appear. subsequent replies, cancellation, and stop updated live without reload. that separate rendering observation was not fixed by this test-only pass.

inspected the captures and checked the uploaded video plays to the end and seeks successfully.