Skip to content

Keep queued chat follow-ups behind pending approvals - #8146

Open
3mdistal wants to merge 6 commits into
mainfrom
t3/fix-standalone-chat-run-slot-race
Open

3mdistal wants to merge 6 commits into
mainfrom
t3/fix-standalone-chat-run-slot-race

Conversation

@3mdistal

@3mdistal 3mdistal commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Queued follow-ups could claim the thread's run slot while its current turn waited for tool approval. Clicking Approve then failed with "The current agent run still owns this thread." This caused the standalone Chat check failures on #7981 and #8092.

Check current-turn approvals inside locked slot admission for both fresh sends and queued promotions. Explicit interruptions cancel a completed approval pause in the same transaction that admits the replacement. A previously resolved approval cannot resume the interrupted turn afterward. Expired, stopped, or superseded approvals do not block later work. Stop persists its turn marker even after an approval-paused execution has completed, including safe-boundary Stop, and schedules an immediate inbox drain. The drain also runs when Stop reports an error after saving state; its admission checks keep work queued if the stop did not persist.

Verification:

  • Deterministic regressions fail before the corresponding fixes, covering approval persistence during admission, completed-pause Stop, safe-boundary Stop, and error paths. An interruption regression resolves the old approval first, checks rollback when receipt submission fails, admits and completes the replacement, then verifies that the cached approval cannot claim the old turn. A fresh-send regression uses the production handler and real database: an unresolved approval queues the send, while a stopped turn allows a completed run without creating a queue receipt.
  • 802 tests across six affected suites pass, along with Core typecheck and changed-file lint/format checks. All 93 repository guards pass. All 9 required CI checks pass on 987a189.
  • Execution-lease model and mutation checks pass; coverage metadata is updated.

The standalone browser smoke could not scaffold on this Windows PC (spawnSync npm.cmd EINVAL); that separate issue is recorded privately. The Linux standalone Chat check passes on current head 987a1891f8b2.

builder-io-integration[bot]

This comment was marked as outdated.

@github-actions

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

builder-io-integration[bot]

This comment was marked as outdated.

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builder reviewed your changes — looks good ✅

Review Details

Incremental review of PR #8146 at the latest HEAD. The update addresses the previously reported interrupt-admission issue by making tryClaimRunSlot identify a pending current-turn approval and, for an explicit interrupt, insert the old turn's abort markers in the same locked transaction as the replacement run. If admission fails after insertion, the transaction rolls the markers back; the added regression also verifies that a late approval cannot resume the interrupted turn. The two prior review threads are resolved. Four independent code-review passes found no new actionable issues in the interrupt, Stop, or ordinary approval-gating paths. Overall, the concurrency approach is sound for this standard-risk change. 🧪 Browser testing: Skipped — the browser-test-planner agent is unavailable in this environment.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant