fix(runner): reject a second turn before touching the sandbox - #6500
Merged
mmabrouk merged 8 commits intoSep 4, 2026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (76)
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe change adds single-turn session admission, durable Redis record recovery, release-gate session-control validation, client turn correlation, persisted build-kit state, design documentation, utility fixes, tests, and version updates. ChangesSession control and release validation
Record durability
Design and supporting updates
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟠 High · up to An oversized session record can cause later valid history records to be discarded, creating a material data-loss risk. Malformed trigger payloads may also escape validation, and some refusal envelopes can lose their stable client-visible code. These issues should be fixed before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 29.41% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 204 functions across 44 files. (31 skipped: 31 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
mmabrouk
changed the title
mmabrouk
marked this pull request as ready for review
September 3, 2026 11:15
Contributor
📘 Docs preview
This comment updates in place on every push. |
Contributor
Railway Preview Environment
|
mmabrouk
added
lgtm
This was referenced Sep 4, 2026
A second user message on a session with a turn in flight killed both turns and left the session locked until the 30-minute lease expired (#6417, #5539, #5538). The platform's arbiter was always correct; the runner acted before reading its answer. The runner starts a turn's alive watchdog before it touches any sandbox, and that watchdog's first heartbeat is an atomic `nx` acquire of the session's `alive` lock in the API. When a second turn lost that acquire the API already answered `is_current_turn: false`. The runner read it only as "abort later", then walked into the keepalive pool, found the first turn's environment busy, and destroyed it. Turn one lost its sandbox mid-answer and turn two aborted on its own watchdog signal. Read the answer before acting: - `startAliveWatchdog` now reports `admitted`, the first beat's answer only. A later `is_current_turn: false` stays a cancel and keeps travelling the `onInterrupted` -> abort path. A network or HTTP failure still fails open. - `server.ts` stops a refused turn at the edge, before the interaction sweep, before the persisting emitter, and before `run()`. Nothing is persisted, so the refused message never enters the session's history and the client can keep the user's text. The refusal streams as an `error` event carrying the new `session_turn_in_use` code plus a failed terminal result. - The keepalive coordinator no longer evicts a `busy` entry. That branch was the destruction half of the bug. It now refuses, which is the backstop for the window admission leaves open when the API is unreachable. A `destroyed` entry still evicts and cold-starts, because nothing is in flight on it. Queue and steer are out of scope: both need a durable pending-input store, while refusing needs none. This is the `on_busy: reject` policy only. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
… turn The runner now refuses a message sent while another turn is already running on the same session (#6417, #5539, #5538). A naive refusal is worse than the bug for the person typing: the composer clears synchronously on submit, so the text they wrote is gone and there is no way to get it back. - `useAgentChatQueue` remembers the message it handed to `sendQueued`, both on the immediate path and on a queue release, and hands it back once through `takeLastSent`. A queued message never needed this; it is already in the queue and rendered by the dock. An immediately-sent one had nowhere to live. It is deliberately NOT re-queued: the queue releases on a settled "error" status, which for a refusal would re-send and be refused again in a loop. - `AgentConversation` puts that text back into the composer when the stream error is the refusal. The rAF mirrors the edit-stash restore beside it, because the editor clears itself after `onSubmit` returns. - The bubble says "Message not sent" instead of "The agent run failed", and offers no retry: nothing failed, and the text is already back in the box. - `parseAgentRunError` carries the stable class for the refusal, so the code reaches the bubble whether it arrives on the message part or the error. The refusal message is the contract with the runner. It is produced once, in `services/runner/src/sessions/admission.ts`, and reaches the browser verbatim: the SDK keeps a clean one-line runner error unchanged and the Vercel egress passes it through as `errorText`. Both constants must stay byte-identical. Mobile shares the queue hook and the error model but has its own composer and error effect, so it gets the refusal class without the text restore. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
The runner's edge now acts on the first heartbeat's `is_current_turn`, so the three answers that decide a session's behaviour need their own coverage. The API code is unchanged; these lock the contract the runner reads. - A second turn arriving while a DIFFERENT turn holds `running` is refused, and the running turn's alive lock is untouched. - A refused turn's end beat (its watchdog release) cannot clear the live turn's `running`, because the release is owner-scoped. - An approval resume IS admitted while the previous turn is parked. `alive` alone cannot tell a park from a live turn; the absent `running` owner is what distinguishes them, and getting this wrong would stop every approval in the product from resuming. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
What happens today with `path:line` evidence, what the three commits change and why, the live test protocol and its results, what queue and steer still need, and five open questions. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…ecution
No first-party client can send `expected_execution_id` on the public Cancel
today, because the runner mints the turn id per execution and never tells
anyone. A Stop can therefore only mean "whatever is running now", never "the
turn I was watching".
The `start` frame cannot carry it. It is built and sent by the SDK's Vercel
egress before the runner replies at all (`vercel/stream.py`, the `start` yield
is the first statement of the projection), so a runner-minted id does not
exist yet at that point. Putting it there would mean moving the mint out of
the runner and threading a new correlation id through the normalizer, the
response models, and the routing layer for every workflow, not just agents.
Use the earliest frame that CAN carry it instead:
- The runner emits `{type: "turn", turnId}` as the first event of a
session-owned run, immediately after admission. It goes through `liveEmit`,
never the persisting emitter, because it is transport correlation and must
not become a session record.
- The Vercel egress forwards it unchanged as `data-agent-turn`, in both the
live and dev-twin projections. A missing, empty or non-string id emits no
part, so a client is never handed a guard value that names nothing.
A refused turn emits none: it runs nothing, so there is nothing to stop.
Verified live: the frame arrives third, after `start` and `start-step` and
before any content, and its id is the one holding the session's alive lock.
Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Adds the section on why the `start` frame cannot carry a runner-minted turn id and what carries it instead, with the live evidence. Updates the test table and notes the keepalive test that was passing for the wrong reason. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Follow-up to ce0f1e1, which put the runner's admitted turn id on a `data-agent-turn` part. Message metadata is the better carrier and the one asked for: a client reads `message.metadata.turnId`, beside the `sessionId` the `start` frame already sets and the `traceId`/`usage` the `finish` frame adds, instead of scanning parts for it. The `start` frame still cannot carry it. That frame is emitted before the runner replies at all, so a runner-minted id does not exist yet. A `message-metadata` chunk is the same channel one frame later, and it is a first-class chunk in the pinned `ai@6.0.0-beta.150`. Safe because the AI SDK MERGES metadata rather than replacing it (`mergeObjects`), so the `finish` frame's own metadata lands beside the turn id rather than over it. A test pins that: the two carry disjoint keys and the turn id is written first. If the SDK ever changed to replace, a client would lose the id exactly when a late Stop needs it. Replaces the `data-agent-turn` part rather than adding to it. One fact should travel one channel, and nothing consumes the part yet. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Records why the `start` frame cannot carry a runner-minted id, why a `message-metadata` chunk one frame later can, and the merge behaviour that makes it survive the `finish` frame. Live evidence updated to the new frame. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
mmabrouk
force-pushed
the
feat/session-single-turn-admission
branch
from
September 4, 2026 16:41
141ebb7 to
e4fb78e
Compare
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-chat/src/model/error.ts (1)
101-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve string envelope codes.
When
status.codeis"session_turn_in_use", Line 101 drops it. Return valid string codes too.Proposed fix
- return {message, code: typeof status?.code === "number" ? status.code : undefined} + return { + message, + code: + typeof status?.code === "number" || typeof status?.code === "string" + ? status.code + : undefined, + }
🧹 Nitpick comments (2)
web/packages/agenta-entities/src/workflow/state/store.ts (1)
1469-1486: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the multi-line comments in the chat queue, runner-error, and build-kit storage test files.
web/AGENTS.mdapplies to allweb/packagesfiles. Keep comments to one short line, or at most two sentences for a genuinely surprising constraint. These blocks exceed that limit; retain only the essential rationale.web/mobile/src/features/chat/LiveConversation.tsx (1)
150-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten the added code comments.
web/AGENTS.mdapplies to all listedweb/**/*.tsandweb/**/*.tsxfiles. Keep each comment to one short line; retain only genuinely surprising constraints, limited to one or two sentences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 353b2aff-cfa9-4b00-896f-eda2f4f5ac9a
⛔ Files ignored due to path filters (4)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (76)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/path_triggers.py.agents/skills/agent-release-gate/resources/qa_product.py.agents/skills/agent-release-gate/resources/session_control.py.agents/skills/agent-release-gate/resources/test_qa_product_concurrency.py.agents/skills/agent-release-gate/resources/test_session_control.py.all-contributorsrcREADME.mdapi/ee/src/dbs/postgres/sessions/records/dao.pyapi/entrypoints/worker_streams.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/routers/user_profile.pyapi/oss/src/services/db_manager.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/tasks/asyncio/shared/consumer.pyapi/oss/src/tasks/taskiq/triggers/worker.pyapi/oss/src/utils/caching.pyapi/oss/src/utils/crypting.pyapi/oss/src/utils/env.pyapi/oss/src/utils/exceptions.pyapi/oss/src/utils/helpers.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_records_worker_durability.pyapi/oss/tests/pytest/unit/sessions/test_watch_publish.pyapi/oss/tests/pytest/unit/triggers/test_triggers_worker_lifecycle.pyapi/oss/tests/pytest/unit/utils/test_caching.pyapi/pyproject.tomlclients/python/pyproject.tomldocs/design/agent-workflows/documentation/adapters/agenta.mddocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mddocs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.mddocs/design/session-control-and-live-events/README.mddocs/design/session-control-and-live-events/context.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/plan.mddocs/design/session-control-and-live-events/records-invariants.mddocs/design/session-control-and-live-events/requirements.mddocs/design/session-control-and-live-events/research.mddocs/design/session-control-and-live-events/rfc.mddocs/design/session-control-and-live-events/slice-admission.mddocs/design/session-control-and-live-events/slice-records-ack.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/kubernetes/helm/Chart.yamlsdks/python/agenta/sdk/agents/adapters/vercel/stream.pysdks/python/oss/tests/pytest/unit/agents/adapters/test_vercel_stream_conformance.pysdks/python/pyproject.tomlservices/pyproject.tomlservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/lifecycle/session-coordinator.tsservices/runner/src/protocol.tsservices/runner/src/server.tsservices/runner/src/sessions/admission.tsservices/runner/src/sessions/alive.tsservices/runner/tests/unit/session-admission.test.tsservices/runner/tests/unit/session-alive-interrupt.test.tsservices/runner/tests/unit/session-keepalive-dispatch.test.tsservices/runner/tests/unit/session-steer-mount-loss.test.tsweb/ee/package.jsonweb/mobile/package.jsonweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/package.jsonweb/packages/agenta-api-client/package.jsonweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
Contributor
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 (4)
web/packages/agenta-chat/src/model/error.ts (1)
101-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve string envelope codes.
When
status.codeis"session_turn_in_use", Line 101 drops it. Return valid string codes too.Proposed fix
- return {message, code: typeof status?.code === "number" ? status.code : undefined} + return { + message, + code: + typeof status?.code === "number" || typeof status?.code === "string" + ? status.code + : undefined, + }api/oss/src/tasks/asyncio/sessions/records_worker.py (1)
254-255: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftProcess oversized entries before reclaiming later records.
When an individual payload exceeds
max_batch_mb, thisbreakleaves it and all later entries pending.StreamConsumer.reclaim_batchcallsXCLAIMbeforeprocess_batch, which increments every claimed entry’s delivery count. With a healthy write path, the reclaim pass can drop later valid records atmax_deliverieswithout an append attempt. Handle the oversized entry as a singleton or dead-letter it, reset the byte accounting, and continue with later entries.api/oss/src/tasks/taskiq/triggers/worker.py (1)
109-111: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNormalize
project_idbefore UUID parsing.If a task payload provides a non-string
project_id,UUID(project_id)can raiseAttributeErrororTypeError, whichexcept ValueErrordoes not catch. ParseUUID(str(project_id))and add a non-string payload test.docs/design/session-control-and-live-events/rfc.md (1)
238-242: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal · Exploitability: Difficult
Require encrypted control delivery.
Require authenticated encryption on every runner-to-API hop. Use HTTPS with certificate validation or mTLS. Reject HTTP endpoints. If redirects are supported, validate the destination and do not forward credentials to another authority.
🧹 Nitpick comments (2)
web/packages/agenta-entities/src/workflow/state/store.ts (1)
1469-1486: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the multi-line comments in the chat queue, runner-error, and build-kit storage test files.
web/AGENTS.mdapplies to allweb/packagesfiles. Keep comments to one short line, or at most two sentences for a genuinely surprising constraint. These blocks exceed that limit; retain only the essential rationale.web/mobile/src/features/chat/LiveConversation.tsx (1)
150-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten the added code comments.
web/AGENTS.mdapplies to all listedweb/**/*.tsandweb/**/*.tsxfiles. Keep each comment to one short line; retain only genuinely surprising constraints, limited to one or two sentences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 353b2aff-cfa9-4b00-896f-eda2f4f5ac9a
⛔ Files ignored due to path filters (4)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (76)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/path_triggers.py.agents/skills/agent-release-gate/resources/qa_product.py.agents/skills/agent-release-gate/resources/session_control.py.agents/skills/agent-release-gate/resources/test_qa_product_concurrency.py.agents/skills/agent-release-gate/resources/test_session_control.py.all-contributorsrcREADME.mdapi/ee/src/dbs/postgres/sessions/records/dao.pyapi/entrypoints/worker_streams.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/routers/user_profile.pyapi/oss/src/services/db_manager.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/tasks/asyncio/shared/consumer.pyapi/oss/src/tasks/taskiq/triggers/worker.pyapi/oss/src/utils/caching.pyapi/oss/src/utils/crypting.pyapi/oss/src/utils/env.pyapi/oss/src/utils/exceptions.pyapi/oss/src/utils/helpers.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_records_worker_durability.pyapi/oss/tests/pytest/unit/sessions/test_watch_publish.pyapi/oss/tests/pytest/unit/triggers/test_triggers_worker_lifecycle.pyapi/oss/tests/pytest/unit/utils/test_caching.pyapi/pyproject.tomlclients/python/pyproject.tomldocs/design/agent-workflows/documentation/adapters/agenta.mddocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mddocs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.mddocs/design/session-control-and-live-events/README.mddocs/design/session-control-and-live-events/context.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/plan.mddocs/design/session-control-and-live-events/records-invariants.mddocs/design/session-control-and-live-events/requirements.mddocs/design/session-control-and-live-events/research.mddocs/design/session-control-and-live-events/rfc.mddocs/design/session-control-and-live-events/slice-admission.mddocs/design/session-control-and-live-events/slice-records-ack.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/kubernetes/helm/Chart.yamlsdks/python/agenta/sdk/agents/adapters/vercel/stream.pysdks/python/oss/tests/pytest/unit/agents/adapters/test_vercel_stream_conformance.pysdks/python/pyproject.tomlservices/pyproject.tomlservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/lifecycle/session-coordinator.tsservices/runner/src/protocol.tsservices/runner/src/server.tsservices/runner/src/sessions/admission.tsservices/runner/src/sessions/alive.tsservices/runner/tests/unit/session-admission.test.tsservices/runner/tests/unit/session-alive-interrupt.test.tsservices/runner/tests/unit/session-keepalive-dispatch.test.tsservices/runner/tests/unit/session-steer-mount-loss.test.tsweb/ee/package.jsonweb/mobile/package.jsonweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/package.jsonweb/packages/agenta-api-client/package.jsonweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sending a second message while a session is running can make both turns terminate and leave the session unusable.
The runner now reads the first heartbeat admission result before resolving or changing the session environment. A rejected turn stops at the boundary. The active turn keeps its sandbox. The browser keeps the rejected text so the user can resend it.
Issue coverage
Closes the concurrent-send failure described by #6417, #5539, and #5538 for the reject policy. It does not implement Queue or Steer.
Dependencies
The runtime fix is independent of warm Stop and durable commands. The pull request is based on the RFC branch and will need a clean release base before merge.
Tests
A live local-provider test refused the second send in 0.18 seconds while the first turn completed and retained its sandbox.
How to review