fix(ai): close pre-INSERT abort window + tighten abort mark predicates (#2028)#2062
Conversation
#2028) Item 1 — Pre-INSERT pending-abort intent: - New `ai_pending_abort_intents` table + migration (0206) - `pending-abort-intents.ts`: recordPendingAbort, consumePendingAbort, clearPendingAbort - `abort-stream-anywhere.ts`: records a pending-abort intent when Stop finds nothing in flight - `createStreamLifecycle`: consumes pending-abort at INSERT time; if present, writes the row as 'aborted' directly and returns a pre-finished handle (preAborted: true) - Both chat routes (chat/route.ts, global/[id]/messages/route.ts): abort the controller when preAborted is true, so streamText never starts Item 4a — False `unconfirmed` when locally aborted: - `abort-stream-anywhere.ts`: if markAbortRequested fails but locallyAborted is non-empty, return ABORTED (streams are provably dead in-process) instead of UNCONFIRMED Item 4b — clearAbortMarks missing status predicate: - `stream-abort-mark.ts`: adds `status='streaming'` filter to clearAbortMarks UPDATE so a mark written against a non-streaming row is not erased Item 4c — Takeover reconcile missing abortRequestedAt null: - `stream-takeover.ts`: adds `abortRequestedAt: null` to the inline reconcile UPDATE, matching reconcileDeadStreamRows' shape Tests: 13 new tests across pending-abort-intents, abort-stream-anywhere-pending, and stream-lifecycle (pre-aborted path). All existing tests pass (113 total). Closes #2028 items 1, 4a, 4b, 4c. Items 2 and 3 are separate changes with their own migration risk. Refs: #2022, #2028
📝 WalkthroughWalkthroughThe change adds durable pending-abort intents, consumes them during stream lifecycle creation, prevents pre-aborted generations from starting, and updates abort outcome handling, mark cleanup, takeover reconciliation, database schema, route wiring, expiry sweeping, and tests. ChangesPending abort flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant abortStreamAnywhere
participant PendingAbortIntents
participant ChatRoute
participant createStreamLifecycle
participant aiStreamSessions
participant streamText
Client->>abortStreamAnywhere: request stop
abortStreamAnywhere->>PendingAbortIntents: record intent when no stream exists
ChatRoute->>createStreamLifecycle: create lifecycle
createStreamLifecycle->>PendingAbortIntents: consume intent
createStreamLifecycle->>aiStreamSessions: write aborted session
createStreamLifecycle-->>ChatRoute: return preAborted lifecycle
ChatRoute->>streamText: abort controller before generation
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
) Closes the remaining Stop gaps from #2028: 1. Pre-INSERT preflight window (item 1, critical) - New `ai_pending_abort_intents` table with migration 0206 - `abortStreamAnywhere` records a durable pending-abort intent when it finds nothing in flight (Stop pressed during 0.5-3s preflight) - `createStreamLifecycle` consumes the intent at INSERT time; if present, writes the row as 'aborted' directly and returns preAborted=true so the route aborts the controller before streamText - Both chat routes handle preAborted by aborting + removing stream 4a. False unconfirmed when locally aborted (item 4a) - `abortStreamAnywhere` now returns ABORTED when mark fails but locally-aborted streams are stopped, instead of UNCONFIRMED 4b. clearAbortMarks missing status predicate (item 4b) - Added `eq(status, 'streaming')` to prevent clearing marks on non-streaming rows 4c. Takeover reconcile missing abortRequestedAt null (item 4c) - Inline reconcile in stream-takeover.ts now sets abortRequestedAt: null to match reconcileDeadStreamRows behavior
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9f4dfc22a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // | ||
| // We INSERT the row as 'aborted' directly, skip multicast registration and the heartbeat, and | ||
| // return a pre-finished handle. The caller aborts the controller so streamText never starts. | ||
| const preAborted = await consumePendingAbort({ conversationId, userId }); |
There was a problem hiding this comment.
When Stop lands after this one-shot consume returns false but before the ai_stream_sessions INSERT completes, the conversation-only abort path still cannot find a row and records a pending intent. This lifecycle never checks again, so the current generation starts anyway, and the just-recorded intent remains to pre-abort the user's next send within the 30s TTL. Please coordinate the consume with the insert, or re-check/update the row after insertion, so this narrow race does not both miss the requested Stop and poison the next request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4407c4d. Rechecked consumePendingAbort() immediately after the aiStreamSessions INSERT resolves (before the heartbeat/broadcast start), not just before it. A Stop landing in the gap you flagged now gets caught by this second check: the just-inserted row is UPDATEd to status='aborted', the multicast registry entry is evicted, and the handle returns preAborted: true — same contract as the pre-INSERT path, just one INSERT later. This closes the window down to the (now synchronous) gap between the recheck and streamText starting, rather than the full route preflight. Added 6 tests in stream-lifecycle.test.ts covering the late-intent path (UPDATE not INSERT, registry eviction, no broadcast, no-op handle, UPDATE-failure resilience). Leaving this thread open for verification.
| "idx": 206, | ||
| "version": "7", | ||
| "when": 1784053313000, | ||
| "tag": "0206_pending_abort_intents", | ||
| "breakpoints": true |
There was a problem hiding this comment.
This journal entry adds migration 0206 and the schema now declares aiPendingAbortIntents, but the commit does not include packages/db/drizzle/meta/0206_snapshot.json (the meta directory still stops at 0205_snapshot.json). The snapshots are the baseline for future generated migrations in this repo, so the next schema diff will not have ai_pending_abort_intents in the latest snapshot and can re-emit or fail around the same CREATE TABLE. Please add the generated 0206 snapshot with this migration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4407c4d. The committed 0206_pending_abort_intents_snapshot.json turned out to be doubly wrong, not just missing/misnamed: its prevId didn't chain from 0205_snapshot.json's id, and it didn't even declare the new ai_pending_abort_intents table — so it wasn't a valid snapshot for this migration at all. Regenerated the real snapshot by running drizzle-kit generate against a scratch out-folder truncated at 0205 (same schema, same config), confirmed the resulting CREATE TABLE SQL matches the already-committed 0206_pending_abort_intents.sql byte-for-byte on the meaningful parts (columns, PK), confirmed the diff adds exactly one table (ai_pending_abort_intents, nothing else drifted), and replaced the file as 0206_snapshot.json — matching the plain <idx>_snapshot.json naming every other entry in this repo's meta/ folder uses (drizzle-kit's own writeResult always names it that way; the tagged name was hand-added, not generated). _journal.json and the SQL migration were left untouched. Leaving this thread open for verification.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/lib/ai/core/stream-lifecycle.ts (1)
112-241: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftClose the consume/insert race in
createStreamLifecycle(apps/web/src/lib/ai/core/stream-lifecycle.ts:120-241):consumePendingAbort()runs before theaiStreamSessionsrow exists, so a Stop that lands in that gap gets recorded as a pending intent after this call has already missed it. That drops the abort for the current generation and leaves an orphan intent that the next send on the same(conversationId, userId)can consume instead. Make the consume+insert path atomic or re-check before starting the stream.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/core/stream-lifecycle.ts` around lines 112 - 241, Close the race in createStreamLifecycle between consumePendingAbort and creation of the aiStreamSessions row: ensure a Stop arriving during this window is applied to the current stream generation rather than left for the next send. Make the pending-abort consumption and session insertion atomic, or re-check pending abort immediately before stream startup and mark the inserted row aborted when found, while preserving the existing preAborted no-op behavior.
🧹 Nitpick comments (4)
packages/db/src/schema/ai-streams.ts (1)
86-111: 🚀 Performance & Scalability | 🔵 TrivialConsider a periodic sweep for orphaned intents.
An intent that is never consumed (user never sends again on that conversation) has no other cleanup path than the TTL check inside
consumePendingAbort, which only runs for that same(conversation_id, user_id)pair. The docstring itself notes a cron "would clean it up," but none exists in this change. Given the PK bounds this to at most one row per conversation+user, growth is not unbounded, but a scheduled sweep (DELETE WHERE created_at < now() - interval) would keep the table tidy without relying on that specific pair being touched again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/schema/ai-streams.ts` around lines 86 - 111, The aiPendingAbortIntents table lacks cleanup for intents that are never consumed. Add a scheduled periodic sweep that deletes rows whose createdAt exceeds the existing TTL, reusing the project’s established job or scheduler mechanism; keep consumePendingAbort’s pair-specific TTL behavior unchanged.apps/web/src/lib/ai/core/__tests__/abort-stream-anywhere-pending.test.ts (1)
77-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the return code too, not just the side effect.
This test only checks that
recordPendingAbortwasn't called; it doesn't verify whatresult.codeactually is for this specific "marked but finished here" branch.♻️ Suggested addition
const result = await abortStreamAnywhere({ conversationId: 'conv1', userId: 'user1', }); expect(recordPendingAbort).not.toHaveBeenCalled(); + expect(result.code).toBe('not_found'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/core/__tests__/abort-stream-anywhere-pending.test.ts` around lines 77 - 88, Update the “does NOT record a pending-abort when rows were marked” test for abortStreamAnywhere to also assert the returned result.code for the marked-but-recently-finished branch. Keep the existing recordPendingAbort assertion and verify the branch’s expected success code.apps/web/src/app/api/ai/chat/route.ts (1)
1233-1240: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPre-aborted requests still run the whole generation setup instead of short-circuiting before
createUIMessageStream.In both routes, when
lifecycle.preAbortedis true, the code aborts the controller and removes the stream from the registry, but then falls through unconditionally into buildingcreateUIMessageStream/execute(), which still writes command-plan indicator parts, callsgetModelCapabilities, and builds/callsrunAgentWithRetry→streamTextwith the now-aborted signal. This should be safe in practice — providers forwardabortSignalto the underlyingfetch, and per the Fetch spec an already-aborted signal makesfetch()reject immediately without any network call — but the code relies on that implicit guarantee rather than a provable early exit, and still performs avoidable work (writer writes, tool-capability resolution, message/tool payload assembly) for a request that can never actually stream.
apps/web/src/app/api/ai/chat/route.ts#L1233-L1240: after settingpreAborted, return the (empty/aborted) UI message stream response immediately rather than entering thetry { createUIMessageStream(...) }block, or otherwise skip straight toexecute's no-op path.apps/web/src/app/api/ai/global/[id]/messages/route.ts#L1104-L1111: apply the same early-exit change here for consistency with the chat route.Also worth adding a route-level test (not present in this batch) asserting the model provider is never actually invoked when
lifecycle.preAbortedis true — this is the property the whole feature exists to guarantee, and today it's only exercised down to thestream-lifecycle.tsboundary, not through the route.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/ai/chat/route.ts` around lines 1233 - 1240, Pre-aborted requests must exit before entering the stream-generation setup. In apps/web/src/app/api/ai/chat/route.ts lines 1233-1240, after handling lifecycle.preAborted, immediately return the empty/aborted UI message stream response; apply the same change in apps/web/src/app/api/ai/global/[id]/messages/route.ts lines 1104-1111 so createUIMessageStream, execute, and model invocation are skipped.packages/db/drizzle/0206_pending_abort_intents.sql (1)
1-6: 🚀 Performance & Scalability | 🔵 TrivialConsider an expiry sweep for orphaned intents.
Rows here are only cleaned up lazily, inside
consumePendingAbort, when a latercreateStreamLifecyclecall for the same(conversation_id, user_id)happens to run. If a user presses Stop during preflight and never sends another message on that conversation, the row lingers indefinitely (it's small, but unbounded across abandoned conversations). Worth considering a periodic sweep (similar to other reconcile/cron jobs in this codebase) that deletes rows older than the TTL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/drizzle/0206_pending_abort_intents.sql` around lines 1 - 6, Introduce a periodic expiry sweep for ai_pending_abort_intents that deletes rows older than the configured pending-abort TTL, rather than relying only on consumePendingAbort. Integrate it with the existing reconcile or cron-job mechanism and reuse the established table and TTL symbols; preserve the current lazy cleanup behavior as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/web/src/lib/ai/core/stream-lifecycle.ts`:
- Around line 112-241: Close the race in createStreamLifecycle between
consumePendingAbort and creation of the aiStreamSessions row: ensure a Stop
arriving during this window is applied to the current stream generation rather
than left for the next send. Make the pending-abort consumption and session
insertion atomic, or re-check pending abort immediately before stream startup
and mark the inserted row aborted when found, while preserving the existing
preAborted no-op behavior.
---
Nitpick comments:
In `@apps/web/src/app/api/ai/chat/route.ts`:
- Around line 1233-1240: Pre-aborted requests must exit before entering the
stream-generation setup. In apps/web/src/app/api/ai/chat/route.ts lines
1233-1240, after handling lifecycle.preAborted, immediately return the
empty/aborted UI message stream response; apply the same change in
apps/web/src/app/api/ai/global/[id]/messages/route.ts lines 1104-1111 so
createUIMessageStream, execute, and model invocation are skipped.
In `@apps/web/src/lib/ai/core/__tests__/abort-stream-anywhere-pending.test.ts`:
- Around line 77-88: Update the “does NOT record a pending-abort when rows were
marked” test for abortStreamAnywhere to also assert the returned result.code for
the marked-but-recently-finished branch. Keep the existing recordPendingAbort
assertion and verify the branch’s expected success code.
In `@packages/db/drizzle/0206_pending_abort_intents.sql`:
- Around line 1-6: Introduce a periodic expiry sweep for
ai_pending_abort_intents that deletes rows older than the configured
pending-abort TTL, rather than relying only on consumePendingAbort. Integrate it
with the existing reconcile or cron-job mechanism and reuse the established
table and TTL symbols; preserve the current lazy cleanup behavior as
appropriate.
In `@packages/db/src/schema/ai-streams.ts`:
- Around line 86-111: The aiPendingAbortIntents table lacks cleanup for intents
that are never consumed. Add a scheduled periodic sweep that deletes rows whose
createdAt exceeds the existing TTL, reusing the project’s established job or
scheduler mechanism; keep consumePendingAbort’s pair-specific TTL behavior
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa67f803-f440-4324-a7f6-f5bd9c529bbc
📒 Files selected for processing (13)
apps/web/src/app/api/ai/chat/route.tsapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/lib/ai/core/__tests__/abort-stream-anywhere-pending.test.tsapps/web/src/lib/ai/core/__tests__/pending-abort-intents.test.tsapps/web/src/lib/ai/core/__tests__/stream-lifecycle.test.tsapps/web/src/lib/ai/core/abort-stream-anywhere.tsapps/web/src/lib/ai/core/pending-abort-intents.tsapps/web/src/lib/ai/core/stream-abort-mark.tsapps/web/src/lib/ai/core/stream-lifecycle.tsapps/web/src/lib/ai/core/stream-takeover.tspackages/db/drizzle/0206_pending_abort_intents.sqlpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/ai-streams.ts
…ng-abort intents Addresses PR #2062 review feedback (Codex + CodeRabbit): - stream-lifecycle.ts: recheck consumePendingAbort() immediately after the aiStreamSessions INSERT, not just before it. A Stop landing in the gap between the pre-INSERT check and the INSERT resolving previously missed the current generation AND left an orphaned intent to wrongly pre-abort the next send within the 30s TTL. - packages/db/drizzle/meta/0206_snapshot.json: the committed snapshot was misnamed (0206_pending_abort_intents_snapshot.json) and internally wrong — its prevId didn't chain from 0205 and it didn't even declare the new table. Regenerated via drizzle-kit against a scratch out-folder truncated at 0205, verified the resulting SQL matches the already-committed 0206_pending_abort_intents.sql, and replaced the file under the convention every other snapshot in this repo follows. - chat/route.ts, global/[id]/messages/route.ts: short-circuit inside execute() when lifecycle.preAborted, skipping command-plan writes, model-capability resolution, and the agent loop instead of relying on the already-aborted signal to short-circuit streamText's fetch. - pending-abort-intents.ts: add sweepExpiredPendingAbortIntents(), wired into the existing /api/cron/sweep-expired route alongside the other append-with-TTL table sweeps. Closes the gap where an intent from a conversation that's never resumed would sit past its TTL indefinitely. - abort-stream-anywhere-pending.test.ts: assert result.code for the "marked but finished here" branch, not just the recordPendingAbort side effect. 65 stream-lifecycle/pending-abort tests + 18 sweep-route tests pass; full ai/core + chat + global test directories (1062 tests) green; typecheck and lint clean.
|
Pushed 4407c4d addressing review feedback so far:
All CI checks were green before this push (typecheck, lint, unit tests, security suite); re-running now. 1062 tests pass locally across the touched directories. |
…heck Self-review (8 independent finder agents) surfaced that the two sequential consumePendingAbort() calls added in 4407c4d (one before the aiStreamSessions INSERT, one after) were: - Duplicative: both branches wrote near-identical 'aborted' state and the same noop-handle literal, just via INSERT vs UPDATE. - Wasteful: every non-preaborted stream start (the overwhelming majority) paid for 2 DB round-trips to check an intent that almost never exists, on a path this whole feature calls TTFB-sensitive. Collapsed to ONE check, placed after the INSERT: since nothing else consumes a pending-abort intent in between, checking once there covers both the original pre-INSERT-window case and the narrower insert-race case the prior commit added, at 1 fewer DB round-trip per stream start and ~95 fewer lines. Also tightened the check's comment to disclose the still-open (but narrow, TTL-bounded, self-healing) commit-ordering race between this consume and a concurrent recordPendingAbort — matching this codebase's own KNOWN RACE disclosure convention rather than implying the window is fully closed. And de-duplicated the identical 5-line comment copied into both routes' execute() guards down to a 3-line pointer at StreamLifecycleHandle.preAborted's existing JSDoc. 48 stream-lifecycle tests (down from 52, net fewer due to the merge) + full ai/core + chat + global + cron test dirs (1058 tests) pass; typecheck and lint clean.
|
Pushed b48cf13 — a self-review pass (8 independent finder agents: line-scan, removed-behavior audit, cross-file tracer, reuse, simplification, efficiency, altitude, CLAUDE.md conventions) on top of 4407c4d surfaced that my own consume/insert-race fix was itself improvable:
Fix: collapsed to one check, placed after the INSERT. Since nothing else consumes a pending-abort intent in the interim, checking once there covers both the original pre-INSERT-window case (#2028 item 1) and the insert-race case — 1 fewer DB round-trip per stream start, ~95 fewer lines, same guarantee. Also acted on:
Not acted on (documented, not silently dropped):
CI: all required checks green (CodeQL, CodeQL Security Analysis, Dependency Audit, Security Test Suite, Secret Scanning, Static Security Analysis, Lint & TypeScript, CodeRabbit re-review clean) as of the prior push; re-running now against b48cf13. 1058 tests pass locally across the touched directories, typecheck/lint clean. |
1 participant
Summary
Closes the cardinal Stop gap from #2028: a Stop pressed during the route's preflight window (0.5-3s TTFB) was a silent no-op that let the generation run to completion. Also fixes three smaller known issues (items 4a, 4b, 4c).
Does not touch items 2 (cross-instance multicast join) or 3 (takeover TOCTOU race) — those are separate changes with their own migration risk.
Item 1 — Pre-INSERT pending-abort intent 🔴
The bug.
createStreamLifecycleruns at the END of the route's preflight — after auth, permissions, message persistence, and context assembly (0.5-3s). A Stop pressed during that window:abortConversationStreamsSELECTsstatus='streaming'rows → 0 rowsmarkAbortRequestedUPDATEs by conversation → 0 rows markednot_found→ UI stays SILENT by designThe fix. A durable pending-abort intent table (
ai_pending_abort_intents), keyed by(conversation_id, user_id):abortStreamAnywhererecords a pending-abort intent when Stop finds nothing in flightcreateStreamLifecycleconsumes it right after theaiStreamSessionsINSERT (registration + insert always happen first, so the same check also catches a Stop landing in the narrow gap between entering the function and the INSERT resolving — see Post-review convergence below). If present, the row is flipped tostatus='aborted', the registry entry is evicted, and the handle returnspreAborted: truechat/route.ts,global/[id]/messages/route.ts) abort the controller and skip straight toonFinishwhenpreAbortedis true, sostreamTextnever startsSecurity. Same model as
ai_stream_sessions.abort_requested_at. The composite PK(conversation_id, user_id)IS the authorization — a row can only be written by a Stop that has already authenticated asuser_id, and only consumed bycreateStreamLifecycle, which receivesuserIdfrom the authenticated route. No forgeable payload.TTL. 30s. The preflight it targets is 0.5-3s. A stale intent is consumed but not honoured.
Item 4a — False
unconfirmedwhen locally abortedabortStreamAnywherereturnedUNCONFIRMEDwhenmarkAbortRequestedfailed, even if streams were demonstrably killed in-process (locallyAborted.length > 0). Now returnsABORTEDin that case — a false "may still be running" about streams that are provably dead.Item 4b —
clearAbortMarksmissing status predicateAdded
status='streaming'to theclearAbortMarksUPDATE WHERE clause, so a mark written against a row that has since terminated (or been taken over) is not cleared.Item 4c — Takeover reconcile missing
abortRequestedAt: nullThe inline reconcile in
stream-takeover.tsnow nullsabortRequestedAt(matchingreconcileDeadStreamRows). Harmless today, but the two reconcile paths now look the same.Changes
packages/db/src/schema/ai-streams.tsaiPendingAbortIntentstable definitionpackages/db/drizzle/0206_pending_abort_intents.sqlai_pending_abort_intentsapps/web/src/lib/ai/core/pending-abort-intents.tsrecordPendingAbort,consumePendingAbort,clearPendingAbort,sweepExpiredPendingAbortIntentsapps/web/src/lib/ai/core/stream-lifecycle.tspreAbortedon handleapps/web/src/lib/ai/core/abort-stream-anywhere.tsapps/web/src/lib/ai/core/stream-abort-mark.tsclearAbortMarksapps/web/src/lib/ai/core/stream-takeover.tsabortRequestedAtin reconcileapps/web/src/app/api/ai/chat/route.tsonFinishwhenpreAbortedapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/app/api/cron/sweep-expired/route.tsai_pending_abort_intentsrows alongside the other append-with-TTL tablesTests
21 new/updated tests across 5 files. Full
ai/core+chat+global+cron/sweep-expiredtest directories (1058 tests) pass; typecheck and lint clean.pending-abort-intents.test.ts(11 tests): record/consume/clear with TTL, error swallowing, sweepabort-stream-anywhere-pending.test.ts(6 tests): intent recording, item 4a fix, result-code assertionstream-lifecycle.test.ts(48 tests): pre-aborted path — handle, registry eviction, no broadcast, INSERT-then-UPDATE, no-op finish/pushPartcron/sweep-expired/route.test.ts: extended to cover the new sweep alongside the existing threeInvariants preserved
request.signalnever reachesstreamText(disconnect-immunity test unchanged)Post-review convergence
Addressed automated review (Codex + CodeRabbit) and a follow-up self-review pass (8 independent finder agents: line-scan, removed-behavior audit, cross-file tracer, reuse, simplification, efficiency, altitude, CLAUDE.md conventions):
recordPendingAbort, matching this codebase's ownKNOWN RACEdisclosure convention rather than implying full closure.0206snapshot didn't chain from0205and didn't declare the new table. Regenerated correctly viadrizzle-kitagainst a scratch out-folder, verified the SQL matches the already-committed migration, and renamed to match this repo's<idx>_snapshot.jsonconvention.returnat the top ofexecute()whenpreAborted, skipping command-plan writes, model-capability resolution, and the agent loop —onFinishstill runs unmodified so credit-hold release is untouched.sweepExpiredPendingAbortIntents(), wired into the existing/api/cron/sweep-expiredroute (already running every 15 min) alongside the other append-with-TTL sweeps.abort-stream-anywhere-pending.test.tsnow assertsresult.codefor the "marked but finished here" branch, not just the side effect.Closes #2028 (items 1, 4a, 4b, 4c).
Refs: #2022
Summary by CodeRabbit
New Features
Bug Fixes
Tests