feat(sessions): deliver durable Stop directly to the runner - #6503
Open
mmabrouk wants to merge 29 commits into
Open
feat(sessions): deliver durable Stop directly to the runner#6503mmabrouk wants to merge 29 commits into
mmabrouk wants to merge 29 commits into
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 (13)
📒 Files selected for processing (172)
🚧 Files skipped from review as they are similar to previous changes (58)
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe change adds durable Stop commands, runner-side cancellation and warm parking, single-turn admission, cancellation records, commit-aware record redelivery, adaptive liveness polling, client execution targeting, persisted build-kit state, and release-gate validation. ChangesSession control and runner lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟠 High · up to Merging can cancel the wrong active turn, lose deferred transcript records or rejected-message state, and expose command control across insufficiently scoped runner boundaries. These issues should be fixed before merge. Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionControlRouter
participant PostgreSQL
participant Runner
participant SandboxAgent
Client->>SessionControlRouter: POST /sessions/{session_id}/cancel
SessionControlRouter->>PostgreSQL: Store pending command
SessionControlRouter->>Runner: POST /cancel
Runner->>SandboxAgent: Send session/cancel
SandboxAgent-->>Runner: Confirm or timeout
Runner->>SessionControlRouter: Report outcome
SessionControlRouter->>PostgreSQL: Settle command
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.15% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 372 functions across 50 files. (117 skipped: 38 unsupported, 79 over the file limit.)
✨ 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 |
This was referenced Sep 2, 2026
mmabrouk
changed the title
mmabrouk
added a commit
that referenced
this pull request
Sep 4, 2026
…on id PR #6503 already uses oss000000022 on the same parent. This file becomes oss000000025 so both branches can merge; whichever lands second re-points down_revision to the new head. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
mmabrouk
commented
Sep 4, 2026
mmabrouk
left a comment
mmabrouk
left a comment
Member
Author
There was a problem hiding this comment.
AGENT-GENERATED review, low weight. A read guide and the points a human should check; not an approval.
Read in this order
api/oss/src/core/sessions/commands/service.py— admission, the three late-Stop guards, delivery, settlement.api/oss/databases/.../oss000000022_add_session_commands.py— the table, the partial unique index, the twosession_streamscolumns.api/oss/src/dbs/postgres/sessions/commands/dao.py— every transition is one guarded compare-and-set.api/oss/src/apis/fastapi/sessions/router.py+middlewares/auth.py— the public Stop route and the runner-token outcome route.web/oss/.../useAgentChatSession.ts— the Stop button, now on the durable endpoint.
Top findings
- One coherent change; the DAO compare-and-set tests run against real Postgres and are strong.
- The rollback flag
AGENTA_SESSIONS_DURABLE_STOPis NOT in this PR — the frontend switches to the new endpoint unconditionally and the route is always mounted. The gate arrives in #6501, so #6503 is safe only landed together with it. Confirm against the design's rollback contract. - Auth exemption
/sessions/control/collides with a session literally namedcontrol(see inline). - Migration is additive with a downgrade and chains cleanly on
oss000000021; the outcome route fails closed with a constant-time compare.
mmabrouk
marked this pull request as ready for review
September 4, 2026 11:09
Contributor
📘 Docs preview
This comment updates in place on every push. |
mmabrouk
commented
Sep 4, 2026
mmabrouk
left a comment
mmabrouk
left a comment
Member
Author
There was a problem hiding this comment.
Review summary
- Risky:
services/runner/src/server.ts:753- parked-session ownership ignores the project half of the pool key, so the same session ID in another project can make this runner accept a command it does not hold. - Risky:
api/oss/src/core/sessions/commands/service.py:235- an idempotency-key collision can return an old terminal command while the response names the newly running turn. - Missing tests: cross-project parked sessions with identical session IDs; reuse of one idempotency key after a later turn starts.
Reviewed at e4cbd64ac91a81f172c999667b9d6a3131b4467e. Advisory COMMENT review only.
Contributor
Railway Preview Environment
|
![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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/runner/patches/sandbox-agent@0.4.2.patch (1)
207-207: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore child termination on Windows.
When
process.platform === "win32", usechild.kill(signal)instead ofprocess.kill(-child.pid, signal). The detached child can remain running when the process-group call throws and the emptycatchdiscards the error.
🧹 Nitpick comments (3)
docs/design/session-control-and-live-events/slice-durable-cancel.md (1)
122-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the Fern-generated client for Stop.
Add the cancellation route to Fern, regenerate the sessions client, and replace the raw
axios.postincancelSessionExecution. Preserve the request fields and409handling. Update this paragraph to state that the Fern client is used.api/oss/tests/pytest/unit/sessions/test_session_cancel_admission.py (1)
879-880: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the late claim is attempted.
_ReportsBeforeTheClaimCommits.deliversettles the row before returningaccepted, after whichSessionCommandsService._delivercalls_dao.claim_for_delivery. Line 880 repeats line 877, so the test can pass if that late claim call is removed. Assert thatdao.claimscontains this command and"runner-1"; keep the existing outcome assertion.api/oss/tests/pytest/unit/sessions/test_session_cancel_feature_flag.py (1)
94-94: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPin the durable response body, not only the status code.
Assert the JSON returned by
SessionCancelResponsewith the command ID andpendingstate, plus execution ID andstoppingstate. This test should detect future wire-contract changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 9f7a6b40-593b-4f4a-8d51-dc8df0bfeef9
⛔ Files ignored due to path filters (1)
services/runner/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (81)
api/entrypoints/routers.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000022_add_session_commands.pyapi/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/commands/__init__.pyapi/oss/src/core/sessions/commands/dtos.pyapi/oss/src/core/sessions/commands/interfaces.pyapi/oss/src/core/sessions/commands/service.pyapi/oss/src/core/sessions/commands/types.pyapi/oss/src/core/sessions/interactions/interfaces.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/runner_client.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/dbs/http/__init__.pyapi/oss/src/dbs/http/sessions/__init__.pyapi/oss/src/dbs/http/sessions/control_delivery_direct.pyapi/oss/src/dbs/postgres/sessions/commands/__init__.pyapi/oss/src/dbs/postgres/sessions/commands/dao.pyapi/oss/src/dbs/postgres/sessions/commands/dbas.pyapi/oss/src/dbs/postgres/sessions/commands/dbes.pyapi/oss/src/dbs/postgres/sessions/commands/mappings.pyapi/oss/src/dbs/postgres/sessions/interactions/dao.pyapi/oss/src/dbs/postgres/sessions/streams/dbes.pyapi/oss/src/dbs/postgres/sessions/streams/mappings.pyapi/oss/src/middlewares/auth.pyapi/oss/src/utils/env.pyapi/oss/tests/pytest/unit/middlewares/test_auth_public_endpoints.pyapi/oss/tests/pytest/unit/sessions/test_command_matrix_inputs_data.pyapi/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_admission.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_feature_flag.pyapi/oss/tests/pytest/unit/sessions/test_session_commands_dao.pyapi/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.pyapi/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.pydocs/design/session-control-and-live-events/README.mddocs/design/session-control-and-live-events/api-design.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-durable-cancel.mddocs/design/session-control-and-live-events/spike-a-sandbox-cancel.mddocs/design/session-control-and-live-events/spike-b-durable-commands-design.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/oss/env.oss.dev.exampleservices/runner/patches/sandbox-agent@0.4.2.patchservices/runner/src/engines/sandbox_agent/cancel-turn.tsservices/runner/src/engines/sandbox_agent/engine.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/engines/sandbox_agent/teardown.tsservices/runner/src/lifecycle/session-coordinator.tsservices/runner/src/protocol.tsservices/runner/src/server.tsservices/runner/src/sessions/applied-commands.tsservices/runner/src/sessions/control-channel.tsservices/runner/src/sessions/execution-registry.tsservices/runner/src/sessions/stop-signal.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/control-command-apply.test.tsservices/runner/tests/unit/harness-cancel-park.test.tsservices/runner/tests/unit/session-pool.test.tsservices/runner/tests/unit/teardown.test.tsweb/mobile/src/features/sessions/useActionableInteractions.tsweb/mobile/src/features/sessions/useLivenessPoll.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/liveness.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/liveness.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/tests/unit/session-liveness.test.tsweb/packages/agenta-navigation/src/dynamic/sessionsSource.tsweb/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
mmabrouk
added a commit
that referenced
this pull request
Sep 4, 2026
A user Stop reached the runner only as the absence of a Redis lock, noticed on the next heartbeat up to 30 seconds later. Nothing recorded that a Stop had been asked for, so a Stop against an unreachable runner was simply lost and no execution ever reached a terminal outcome anyone could read. Add session_commands: one row per durable request to change an execution. Two columns that are never merged carry the two questions a caller actually asks. state says where the COMMAND is (pending, claimed, applied, obsolete). outcome says what happened to the EXECUTION (stopped, not_running, superseded_by_newer_turn, failed, lost). A client drawing a Stop button reads the execution; a client retrying safely reads the command id. Every transition is one UPDATE ... WHERE <expected state> RETURNING *, decided by scalar_one_or_none, the same compare-and-set transition_interaction already uses. That is what stops two API replicas both winning a claim or both writing a terminal outcome. Idempotency has two layers: the caller's Idempotency-Key on (project_id, session_id, idempotency_key), and a collapse onto any open command for the same target execution, which is what makes two Stops in a row correct without asking the browser to send a key. session_streams gains two columns. stopping_turn_id names the execution an accepted Stop is waiting on, written in the same transaction as the command insert. turn_started_at records when the row's current turn_id started, because the stale-Stop guard has to compare a Stop's arrival time with the running execution's start time and there was nowhere to read that: updated_at is the heartbeat timestamp and moves every 30 seconds, runner-minted turn ids are uuid4 and carry no time, the Redis lock value is a bare turn id a Lua compare reads whole, and the session_turns append is fire-and-forget so a running turn may have no row. It is stamped only when the id actually changes, so the repeated heartbeats that restamp the same id never move it. Both columns backfill to NULL. A row written before this migration yields no comparison and the guard does not fire, deliberately: a guard that refused every Stop it could not verify would break the common case to protect a rare one. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
… removal Re-measures the live protocol against the rebased runner (82ms to the abort, 126ms to settlement, 993ms to the warm park), records the fourth defect the rebase exposed, and rewrites the census entry: it is removed rather than softened, on the revised design's guidance that the not_held detector is the one worth building and the census is optional. That closes the one open question that was a deviation from the brief. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Settlement released `running` in Redis and stopped there. The row kept `is_running: true`, and the row is the only thing the product's liveness polls read: `query_streams` serves Postgres and never looks at Redis. Nothing else could correct it. Settlement tombstones the stopped execution before it releases `running`, so the runner's own final `is_running=false` heartbeat is refused by the tombstone check and returns before the mirror write at the end of `heartbeat`. The order cannot be swapped: a late beat that found `alive` free would take it straight back under the dead turn's id. The runner reports its outcome as soon as it issues the abort, so the tombstone always wins that race. Measured on the local sandbox with Pi before the fix: the row read `is_running: true` with a pre-Stop `updated_at` for the full 193 s of the sample, while Redis had released `running` within 0.5 s. The tab that pressed Stop therefore showed a "running somewhere else" strip over its own session until the orphan sweep collapsed the row minutes later. After the fix the row reads `is_running: false, is_alive: true` within 0.15 s of the request, and the parked sandbox still resumes warm. `mirror_liveness` re-reads Redis rather than writing a literal `false`, so a newer turn that has already taken `running` is reported and not erased. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Every liveness `refetchInterval` asked "is the alive set non-empty", which is not the question. Stop ends the work and leaves the session alive so the sandbox resumes warm, and an ordinary turn end does the same, so one stopped session held all four polls at 15 s, in every open tab, for the hour that `alive` lock lives. One shared predicate now answers the real question: 15 s while something is RUNNING, 60 s while a session is merely alive, and stop when nothing is alive. The sidebar rail passes a 60 s idle floor instead of stopping, because it must still discover a run it did not start; that baseline was already deliberate and is unchanged. The mobile gates poll keys on running directly, since a running turn is what mints new gates. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…olves Admission resolves the execution to stop from the Redis `running` key with a fallback to `alive`, so a session parked on an approval is reachable. The `expected_execution_id` guard then compared against `running` alone. A parked approval has released `running` and still holds `alive` under the same turn id, so the guard read `running` as none, refused the request with a conflict naming "current: none", and left the gate pending. The identical Stop sent without an expectation was accepted and cancelled the gate. The browser always sends the id it streamed, so pressing Stop on an approval card was refused in the product while the integration approval cell passed, because its driver sent no expectation. The guard fired on the one case it exists to allow. It now compares against the resolved target. The guard still refuses a stale id: a Stop naming a finished turn on a session parked under a newer one is refused, and the conflict now names the turn that would have been stopped instead of none. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…aimed Admission inserts the command `pending`, hands it to the runner, and writes `claimed` on the runner's behalf only after the runner answers. A runner that aborts fast reports its outcome inside that window, while the row still says `pending`. The outcome route guarded on `claimed` alone, so that report was refused with a conflict. Observed on a Stop during model output: the runner aborted and parked correctly, logged `[control] outcome HTTP 409`, and the command sat `claimed` for 2 min 17 s until the sweep settled it `obsolete` with outcome `lost`. The user watched "stopping" for the whole sweep window and a Stop that worked was recorded as lost. The guard is now a set, and it is still one statement, so it is evaluated at the moment of the write. Reading the state first and updating after would reopen the same race: the claim can commit in between. The replica guard is widened only where there is nothing to guard. A `pending` row holds no claim, so a null `claimed_by` passes; a claimed row must still be claimed by the reporter, and a report from any other replica is still refused. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…ndbox A Stop pressed as the answer lands used to tear the sandbox down, so the next message rebuilt cold. Reproduced on the local sandbox with Pi by firing the Stop on the runner's own `prompt stopReason=end_turn` line: [control] aborted command=... turn=c81b783e... [control] outcome reported command=... state=stopped [keepalive] evict key=... reason=no-park:end_turn and the next message took 7.2 s against 1.9 s warm. Two things were wrong, in two places. The execution stays registered through teardown, which writes the transcript, exports the trace and parks the environment, and that takes hundreds of milliseconds. A Stop arriving in that window found a live entry and aborted it. The abort stopped nothing, because the prompt had already settled, but the aborted signal then made `shouldPark` refuse to park a healthy idle environment. The run is now marked settled the instant the harness prompt settles, before teardown begins, and the applier does nothing at all for a settled run. Nothing aborts, so the ordinary park path runs. It reports `obsolete` with `not_running`, because the command stopped nothing. Past that window the runner has dropped the execution and answers `not_held`, and the API judged that on whether the row was beating. A turn that has just ended leaves `alive` set and a fresh beat behind it exactly as a running one does, so every late Stop was settled `lost` and the user was told their Stop failed when the work had simply finished. The discriminator is now `running`: an execution holding it means a process is running this session and it is not the one we called, which is the wrong-replica failure the `lost` outcome exists for. With no `running` owner nothing is executing anywhere, and `not_running` is the honest answer. Verified live on both windows: the command settles `obsolete` / `not_running`, no eviction, and the next message reuses the parked sandbox warm (`hit-continue`, 1.9 s). Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Return whether command insertion won so a replay returns its original target without delivering the command to a newer execution. Cover the replay after the session advances to another turn. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Encode runner credentials before constant-time comparison so non-ASCII input is rejected with 401 instead of raising an internal error. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Treat audit-record persistence as best effort after pending interactions are cancelled. A record-store failure is logged while lifecycle publication and the cancellation result continue. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Accept only object-shaped runner acknowledgements when reading replica identity and make the optional delivery timeout explicit. Malformed successful JSON remains an accepted response without crashing admission. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
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 (1)
api/oss/src/dbs/postgres/sessions/commands/dao.py (1)
177-177: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftIDOR (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Reachability: External · Exploitability: Moderate
Propagate project scope through the runner outcome path.
report_outcomefetches the command and fallback state bycommand_idonly. CarryControlCommand.projectIdin the outcome request, pass it to both DAO reads, and require the project filter.SessionCommandDBEuses(project_id, id)as its key, so the runner token alone must not select or settle another project's command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: ba236d57-74ae-4478-a815-0f65d75e9bb0
⛔ Files ignored due to path filters (8)
web/packages/agenta-api-client/src/generated/api/resources/sessions/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/CancelSessionExecutionRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/SessionStreamCommandRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/index.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionCancelRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStream.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStreamCommandResponse.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/index.tsis excluded by!**/generated/**
📒 Files selected for processing (33)
api/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/commands/interfaces.pyapi/oss/src/core/sessions/commands/service.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/streams/runner_client.pyapi/oss/src/dbs/http/sessions/control_delivery_direct.pyapi/oss/src/dbs/postgres/sessions/commands/dao.pyapi/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.pyapi/oss/tests/pytest/unit/sessions/test_runner_client_kill.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_admission.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_feature_flag.pydocs/design/session-control-and-live-events/api-design.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/server.tsservices/runner/src/sessions/control-channel.tsservices/runner/tests/unit/control-command-apply.test.tsservices/runner/tests/unit/sandbox-agent-orchestration.test.tsservices/runner/tests/unit/server.test.tsweb/mobile/src/features/sessions/useActionableInteractions.tsweb/mobile/src/features/sessions/useLivenessPoll.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/liveness.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/liveness.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/tests/unit/session-cancel-api.test.tsweb/packages/agenta-entities/tests/unit/session-liveness.test.tsweb/packages/agenta-navigation/src/dynamic/sessionsSource.tsweb/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- api/oss/src/core/sessions/streams/runner_client.py
- web/packages/agenta-navigation/src/dynamic/sessionsSource.ts
- web/mobile/src/features/sessions/useLivenessPoll.ts
- api/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.py
- web/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
- web/mobile/src/features/sessions/useActionableInteractions.ts
- web/packages/agenta-entities/tests/unit/session-liveness.test.ts
- docs/design/session-control-and-live-events/api-design.md
- api/oss/tests/pytest/unit/sessions/test_session_cancel_feature_flag.py
- docs/design/session-control-and-live-events/status.md
- api/oss/src/dbs/http/sessions/control_delivery_direct.py
- api/oss/src/core/sessions/interactions/service.py
- services/runner/src/engines/sandbox_agent/run-turn.ts
- web/oss/src/components/AgentChatSlice/state/liveness.ts
- web/packages/agenta-entities/src/session/core/liveness.ts
- docs/design/session-control-and-live-events/decisions.md
- api/oss/src/apis/fastapi/sessions/router.py
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review.
Scope parked-session lookup by project, keep paused approval turns cancellable, clean execution registrations on rejected or failed setup, and refuse redirects when reporting outcomes. Add focused regressions for each boundary. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Regenerate the sessions cancellation route and related stream fields from the branch OpenAPI schema. Replace raw Axios with the typed sessions accessor, preserve project query scope and idempotency headers, and validate successful payloads with Zod. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Fetch the current session stream before aborting the client transport so the Stop command cannot observe and cancel a newer turn submitted during target acquisition. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Subscribe the actionable-interactions hook to the shared liveness query so a newly running turn restarts gate polling after the idle interval was disabled. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep the execution-versus-warm invariants at the affected call sites while moving each changed code comment back to one concise line. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Align the API design, decisions, status, and handoff with the implemented direct control adapter. Mark runner-initiated long polling as deferred behind the same delivery port. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
mmabrouk
force-pushed
the
feat/session-durable-cancel
branch
from
September 4, 2026 17:20
2251b4d to
cd21450
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 7d9ba61a-653e-4a63-b809-75d469ee8856
⛔ Files ignored due to path filters (13)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/runner/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/uv.lockis excluded by!**/*.lockweb/packages/agenta-api-client/src/generated/api/resources/sessions/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/CancelSessionExecutionRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/SessionStreamCommandRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/index.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionCancelRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStream.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStreamCommandResponse.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/index.tsis excluded by!**/generated/**
📒 Files selected for processing (172)
.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/routers.pyapi/entrypoints/worker_streams.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000022_add_session_commands.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/commands/__init__.pyapi/oss/src/core/sessions/commands/dtos.pyapi/oss/src/core/sessions/commands/interfaces.pyapi/oss/src/core/sessions/commands/service.pyapi/oss/src/core/sessions/commands/types.pyapi/oss/src/core/sessions/interactions/interfaces.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/runner_client.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/dbs/http/__init__.pyapi/oss/src/dbs/http/sessions/__init__.pyapi/oss/src/dbs/http/sessions/control_delivery_direct.pyapi/oss/src/dbs/postgres/sessions/commands/__init__.pyapi/oss/src/dbs/postgres/sessions/commands/dao.pyapi/oss/src/dbs/postgres/sessions/commands/dbas.pyapi/oss/src/dbs/postgres/sessions/commands/dbes.pyapi/oss/src/dbs/postgres/sessions/commands/mappings.pyapi/oss/src/dbs/postgres/sessions/interactions/dao.pyapi/oss/src/dbs/postgres/sessions/streams/dbes.pyapi/oss/src/dbs/postgres/sessions/streams/mappings.pyapi/oss/src/middlewares/auth.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/middlewares/test_auth_public_endpoints.pyapi/oss/tests/pytest/unit/sessions/test_command_matrix_inputs_data.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_release_owner.pyapi/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.pyapi/oss/tests/pytest/unit/sessions/test_records_worker_durability.pyapi/oss/tests/pytest/unit/sessions/test_runner_client_kill.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_admission.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_feature_flag.pyapi/oss/tests/pytest/unit/sessions/test_session_commands_dao.pyapi/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.pyapi/oss/tests/pytest/unit/sessions/test_watch_publish.pyapi/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.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/api-design.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-durable-cancel.mddocs/design/session-control-and-live-events/slice-records-ack.mddocs/design/session-control-and-live-events/spike-a-sandbox-cancel.mddocs/design/session-control-and-live-events/spike-b-durable-commands-design.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/oss/env.oss.dev.examplehosting/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/patches/sandbox-agent@0.4.2.patchservices/runner/src/engines/sandbox_agent/agent-mount.tsservices/runner/src/engines/sandbox_agent/cancel-turn.tsservices/runner/src/engines/sandbox_agent/credential-preflight.tsservices/runner/src/engines/sandbox_agent/engine.tsservices/runner/src/engines/sandbox_agent/environment-setup.tsservices/runner/src/engines/sandbox_agent/environment.tsservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/engines/sandbox_agent/mount.tsservices/runner/src/engines/sandbox_agent/provider.tsservices/runner/src/engines/sandbox_agent/reap-exec.tsservices/runner/src/engines/sandbox_agent/reconstruct-history.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/runtime-contracts.tsservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/engines/sandbox_agent/teardown.tsservices/runner/src/environment/abortable-sandbox-provider.tsservices/runner/src/environment/acquire-abort.tsservices/runner/src/environment/harness-session-lifecycle.tsservices/runner/src/environment/mount-lifecycle.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/src/sessions/applied-commands.tsservices/runner/src/sessions/control-channel.tsservices/runner/src/sessions/execution-registry.tsservices/runner/src/sessions/stop-signal.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/acquire-abort.test.tsservices/runner/tests/unit/cancel-continuity.test.tsservices/runner/tests/unit/continuation.test.tsservices/runner/tests/unit/control-command-apply.test.tsservices/runner/tests/unit/credential-preflight.test.tsservices/runner/tests/unit/environment-units.test.tsservices/runner/tests/unit/harness-cancel-park.test.tsservices/runner/tests/unit/reap-exec.test.tsservices/runner/tests/unit/sandbox-agent-mount.test.tsservices/runner/tests/unit/sandbox-agent-orchestration.test.tsservices/runner/tests/unit/sandbox-lifecycle.test.tsservices/runner/tests/unit/server.test.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-ownership-release.test.tsservices/runner/tests/unit/session-pool.test.tsservices/runner/tests/unit/session-reconstruct-history.test.tsservices/runner/tests/unit/session-steer-mount-loss.test.tsservices/runner/tests/unit/teardown.test.tsweb/ee/package.jsonweb/mobile/package.jsonweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/sessions/useActionableInteractions.tsweb/mobile/src/features/sessions/useLivenessPoll.tsweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/liveness.tsweb/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/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/liveness.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.tsweb/packages/agenta-entities/tests/unit/session-cancel-api.test.tsweb/packages/agenta-entities/tests/unit/session-liveness.test.tsweb/packages/agenta-navigation/src/dynamic/sessionsSource.tsweb/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
🚧 Files skipped from review as they are similar to previous changes (58)
- docs/design/session-control-and-live-events/README.md
- web/packages/agenta-entities/src/session/core/schema.ts
- api/oss/src/middlewares/auth.py
- api/oss/tests/pytest/unit/middlewares/test_auth_public_endpoints.py
- services/runner/patches/sandbox-agent@0.4.2.patch
- hosting/docker-compose/oss/env.oss.dev.example
- web/packages/agenta-entities/src/session/api/api.ts
- web/oss/src/components/AgentChatSlice/state/liveness.ts
- api/oss/src/dbs/postgres/sessions/commands/dbas.py
- web/packages/agenta-entities/tests/unit/session-liveness.test.ts
- hosting/docker-compose/ee/env.ee.dev.example
- api/oss/src/dbs/http/sessions/control_delivery_direct.py
- api/oss/src/core/sessions/streams/runner_client.py
- web/packages/agenta-entities/src/session/index.ts
- api/oss/src/core/sessions/commands/types.py
- api/oss/tests/pytest/unit/sessions/test_command_matrix_inputs_data.py
- web/mobile/src/features/sessions/useLivenessPoll.ts
- api/oss/src/core/sessions/interactions/interfaces.py
- services/runner/src/sessions/stop-signal.ts
- api/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.py
- services/runner/src/engines/sandbox_agent/engine.ts
- web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts
- api/oss/src/core/sessions/interactions/service.py
- api/oss/src/dbs/postgres/sessions/commands/mappings.py
- api/oss/src/dbs/postgres/sessions/interactions/dao.py
- web/packages/agenta-entities/tests/unit/session-cancel-api.test.ts
- services/runner/src/tracing/otel.ts
- api/oss/src/core/sessions/streams/dtos.py
- services/runner/tests/unit/session-pool.test.ts
- api/oss/src/apis/fastapi/sessions/models.py
- services/runner/src/sessions/execution-registry.ts
- api/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.py
- api/oss/src/dbs/postgres/sessions/streams/dbes.py
- services/runner/src/sessions/control-channel.ts
- api/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.py
- services/runner/src/server.ts
- web/packages/agenta-entities/src/session/core/liveness.ts
- services/runner/tests/unit/teardown.test.ts
- services/runner/src/sessions/applied-commands.ts
- api/oss/src/core/sessions/commands/interfaces.py
- services/runner/tests/unit/control-command-apply.test.ts
- api/oss/src/core/sessions/commands/dtos.py
- api/oss/src/dbs/postgres/sessions/streams/mappings.py
- api/oss/src/dbs/postgres/sessions/commands/dbes.py
- web/packages/agenta-navigation/src/dynamic/sessionsSource.ts
- api/oss/src/core/sessions/commands/service.py
- api/oss/tests/pytest/unit/sessions/test_session_commands_dao.py
- web/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
- services/runner/tests/unit/harness-cancel-park.test.ts
- services/runner/src/engines/sandbox_agent/teardown.ts
- api/oss/src/core/sessions/streams/types.py
- api/oss/databases/postgres/migrations/core_oss/versions/oss000000022_add_session_commands.py
- api/oss/src/dbs/postgres/sessions/commands/dao.py
- api/oss/tests/pytest/unit/sessions/test_session_cancel_admission.py
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- web/mobile/src/features/sessions/useActionableInteractions.ts
- services/runner/src/engines/sandbox_agent/session-identity.ts
- api/entrypoints/routers.py
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
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 15
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 7d9ba61a-653e-4a63-b809-75d469ee8856
⛔ Files ignored due to path filters (13)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/runner/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlservices/uv.lockis excluded by!**/*.lockweb/packages/agenta-api-client/src/generated/api/resources/sessions/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/CancelSessionExecutionRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/SessionStreamCommandRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/index.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionCancelRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStream.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStreamCommandResponse.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/index.tsis excluded by!**/generated/**
📒 Files selected for processing (172)
.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/routers.pyapi/entrypoints/worker_streams.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000022_add_session_commands.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/commands/__init__.pyapi/oss/src/core/sessions/commands/dtos.pyapi/oss/src/core/sessions/commands/interfaces.pyapi/oss/src/core/sessions/commands/service.pyapi/oss/src/core/sessions/commands/types.pyapi/oss/src/core/sessions/interactions/interfaces.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/runner_client.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/dbs/http/__init__.pyapi/oss/src/dbs/http/sessions/__init__.pyapi/oss/src/dbs/http/sessions/control_delivery_direct.pyapi/oss/src/dbs/postgres/sessions/commands/__init__.pyapi/oss/src/dbs/postgres/sessions/commands/dao.pyapi/oss/src/dbs/postgres/sessions/commands/dbas.pyapi/oss/src/dbs/postgres/sessions/commands/dbes.pyapi/oss/src/dbs/postgres/sessions/commands/mappings.pyapi/oss/src/dbs/postgres/sessions/interactions/dao.pyapi/oss/src/dbs/postgres/sessions/streams/dbes.pyapi/oss/src/dbs/postgres/sessions/streams/mappings.pyapi/oss/src/middlewares/auth.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/middlewares/test_auth_public_endpoints.pyapi/oss/tests/pytest/unit/sessions/test_command_matrix_inputs_data.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_release_owner.pyapi/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.pyapi/oss/tests/pytest/unit/sessions/test_records_worker_durability.pyapi/oss/tests/pytest/unit/sessions/test_runner_client_kill.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_admission.pyapi/oss/tests/pytest/unit/sessions/test_session_cancel_feature_flag.pyapi/oss/tests/pytest/unit/sessions/test_session_commands_dao.pyapi/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.pyapi/oss/tests/pytest/unit/sessions/test_watch_publish.pyapi/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.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/api-design.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-durable-cancel.mddocs/design/session-control-and-live-events/slice-records-ack.mddocs/design/session-control-and-live-events/spike-a-sandbox-cancel.mddocs/design/session-control-and-live-events/spike-b-durable-commands-design.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/oss/env.oss.dev.examplehosting/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/patches/sandbox-agent@0.4.2.patchservices/runner/src/engines/sandbox_agent/agent-mount.tsservices/runner/src/engines/sandbox_agent/cancel-turn.tsservices/runner/src/engines/sandbox_agent/credential-preflight.tsservices/runner/src/engines/sandbox_agent/engine.tsservices/runner/src/engines/sandbox_agent/environment-setup.tsservices/runner/src/engines/sandbox_agent/environment.tsservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/engines/sandbox_agent/mount.tsservices/runner/src/engines/sandbox_agent/provider.tsservices/runner/src/engines/sandbox_agent/reap-exec.tsservices/runner/src/engines/sandbox_agent/reconstruct-history.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/runtime-contracts.tsservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/engines/sandbox_agent/teardown.tsservices/runner/src/environment/abortable-sandbox-provider.tsservices/runner/src/environment/acquire-abort.tsservices/runner/src/environment/harness-session-lifecycle.tsservices/runner/src/environment/mount-lifecycle.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/src/sessions/applied-commands.tsservices/runner/src/sessions/control-channel.tsservices/runner/src/sessions/execution-registry.tsservices/runner/src/sessions/stop-signal.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/acquire-abort.test.tsservices/runner/tests/unit/cancel-continuity.test.tsservices/runner/tests/unit/continuation.test.tsservices/runner/tests/unit/control-command-apply.test.tsservices/runner/tests/unit/credential-preflight.test.tsservices/runner/tests/unit/environment-units.test.tsservices/runner/tests/unit/harness-cancel-park.test.tsservices/runner/tests/unit/reap-exec.test.tsservices/runner/tests/unit/sandbox-agent-mount.test.tsservices/runner/tests/unit/sandbox-agent-orchestration.test.tsservices/runner/tests/unit/sandbox-lifecycle.test.tsservices/runner/tests/unit/server.test.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-ownership-release.test.tsservices/runner/tests/unit/session-pool.test.tsservices/runner/tests/unit/session-reconstruct-history.test.tsservices/runner/tests/unit/session-steer-mount-loss.test.tsservices/runner/tests/unit/teardown.test.tsweb/ee/package.jsonweb/mobile/package.jsonweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/sessions/useActionableInteractions.tsweb/mobile/src/features/sessions/useLivenessPoll.tsweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/liveness.tsweb/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/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/liveness.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.tsweb/packages/agenta-entities/tests/unit/session-cancel-api.test.tsweb/packages/agenta-entities/tests/unit/session-liveness.test.tsweb/packages/agenta-navigation/src/dynamic/sessionsSource.tsweb/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
🚧 Files skipped from review as they are similar to previous changes (58)
- docs/design/session-control-and-live-events/README.md
- web/packages/agenta-entities/src/session/core/schema.ts
- api/oss/src/middlewares/auth.py
- api/oss/tests/pytest/unit/middlewares/test_auth_public_endpoints.py
- services/runner/patches/sandbox-agent@0.4.2.patch
- hosting/docker-compose/oss/env.oss.dev.example
- web/packages/agenta-entities/src/session/api/api.ts
- web/oss/src/components/AgentChatSlice/state/liveness.ts
- api/oss/src/dbs/postgres/sessions/commands/dbas.py
- web/packages/agenta-entities/tests/unit/session-liveness.test.ts
- hosting/docker-compose/ee/env.ee.dev.example
- api/oss/src/dbs/http/sessions/control_delivery_direct.py
- api/oss/src/core/sessions/streams/runner_client.py
- web/packages/agenta-entities/src/session/index.ts
- api/oss/src/core/sessions/commands/types.py
- api/oss/tests/pytest/unit/sessions/test_command_matrix_inputs_data.py
- web/mobile/src/features/sessions/useLivenessPoll.ts
- api/oss/src/core/sessions/interactions/interfaces.py
- services/runner/src/sessions/stop-signal.ts
- api/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.py
- services/runner/src/engines/sandbox_agent/engine.ts
- web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts
- api/oss/src/core/sessions/interactions/service.py
- api/oss/src/dbs/postgres/sessions/commands/mappings.py
- api/oss/src/dbs/postgres/sessions/interactions/dao.py
- web/packages/agenta-entities/tests/unit/session-cancel-api.test.ts
- services/runner/src/tracing/otel.ts
- api/oss/src/core/sessions/streams/dtos.py
- services/runner/tests/unit/session-pool.test.ts
- api/oss/src/apis/fastapi/sessions/models.py
- services/runner/src/sessions/execution-registry.ts
- api/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.py
- api/oss/src/dbs/postgres/sessions/streams/dbes.py
- services/runner/src/sessions/control-channel.ts
- api/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.py
- services/runner/src/server.ts
- web/packages/agenta-entities/src/session/core/liveness.ts
- services/runner/tests/unit/teardown.test.ts
- services/runner/src/sessions/applied-commands.ts
- api/oss/src/core/sessions/commands/interfaces.py
- services/runner/tests/unit/control-command-apply.test.ts
- api/oss/src/core/sessions/commands/dtos.py
- api/oss/src/dbs/postgres/sessions/streams/mappings.py
- api/oss/src/dbs/postgres/sessions/commands/dbes.py
- web/packages/agenta-navigation/src/dynamic/sessionsSource.ts
- api/oss/src/core/sessions/commands/service.py
- api/oss/tests/pytest/unit/sessions/test_session_commands_dao.py
- web/packages/agenta-navigation/tests/unit/sidebarChildren.test.ts
- services/runner/tests/unit/harness-cancel-park.test.ts
- services/runner/src/engines/sandbox_agent/teardown.ts
- api/oss/src/core/sessions/streams/types.py
- api/oss/databases/postgres/migrations/core_oss/versions/oss000000022_add_session_commands.py
- api/oss/src/dbs/postgres/sessions/commands/dao.py
- api/oss/tests/pytest/unit/sessions/test_session_cancel_admission.py
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- web/mobile/src/features/sessions/useActionableInteractions.ts
- services/runner/src/engines/sandbox_agent/session-identity.ts
- api/entrypoints/routers.py
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
🛑 Comments failed to post (15)
.agents/skills/agent-release-gate/resources/path_triggers.py (1)
88-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Trigger session-control evidence for persistence and composition-root changes.
A release limited to
api/oss/src/dbs/postgres/sessions/**, the related session-command migration, or session-control wiring inapi/entrypoints/**does not requiresession_control.py. Those paths can break durable Stop persistence or delivery while the release gate remains green.
.agents/skills/agent-release-gate/resources/path_triggers.py#L88-L91: Add path rules for session persistence, relevant migrations, and session-control composition roots..agents/skills/agent-release-gate/SKILL.md#L195-L199: Document the same expanded mandatory-path scope.📍 Affects 2 files
.agents/skills/agent-release-gate/resources/path_triggers.py#L88-L91(this comment).agents/skills/agent-release-gate/SKILL.md#L195-L199api/oss/src/apis/fastapi/sessions/router.py (1)
1983-1983: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject oversized idempotency keys.
cancel_session_executiontruncates the normalized key beforerequest_cancel. The DAO uses that value for the unique command lookup, so two distinct keys with the same first 255 characters can replay the first command after a later execution starts. If the normalized key exceeds 255 characters, return HTTP 422 instead of truncating it.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.idempotency_key = request.headers.get("Idempotency-Key") if idempotency_key is not None: idempotency_key = idempotency_key.strip() if len(idempotency_key) > _MAX_IDEMPOTENCY_KEY_CHARACTERS: raise HTTPException( status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail="Idempotency-Key is too long.", ) idempotency_key = idempotency_key or Noneapi/oss/src/core/sessions/streams/service.py (1)
212-215: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make expected-turn cancellation atomic.
A new turn can replace the locks after the earlier owner check. This path then tombstones and clears that new turn. Use an atomic compare-and-delete or fencing operation that verifies
expected_turn_idat the lock mutation point.api/oss/src/tasks/asyncio/sessions/records_worker.py (1)
243-248: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learnings /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventionsLength of output: 12646
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- worker outline ---' ast-grep outline api/oss/src/tasks/asyncio/sessions/records_worker.py printf '%s\n' '--- worker focused source ---' sed -n '1,125p' api/oss/src/tasks/asyncio/sessions/records_worker.py sed -n '150,390p' api/oss/src/tasks/asyncio/sessions/records_worker.py printf '%s\n' '--- directly bound reclaim symbols and callers ---' rg -n -C 5 'reclaim_batch|drop_expired|max_deliveries|_last_commit_at|process_batch|quota|entitlement|org_deferred' api/oss/src/tasks/asyncio api/oss/src -g '*.py'Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- consumer size and outline ---' wc -l api/oss/src/tasks/asyncio/shared/consumer.py ast-grep outline api/oss/src/tasks/asyncio/shared/consumer.py printf '%s\n' '--- consumer implementation ---' sed -n '1,420p' api/oss/src/tasks/asyncio/shared/consumer.py printf '%s\n' '--- focused tests and configuration ---' rg -n -C 6 'reclaim_batch|drop_expired|mark_committed|last_commit|max_deliveries|reclaim_pending' api/oss/tests api/oss/src/tasks/asyncio/shared api/oss/src/utils/env.py -g '*.py'Repository: Agenta-AI/agenta
Length of output: 44429
Exclude quota-deferred entries from
StreamConsumer’s delivery budget.When
check_entitlementsfails,RecordsWorker.process_batchleaves the entry pending. An unrelated successful append callsmark_committed(), soreclaim_batchcan treat the path as healthy anddrop_expiredcan acknowledge and delete the deferred entry aftermax_deliveries. Track deferred entries or retry the quota check before expiration.Add a regression with one successful project and one project with repeated entitlement failures. The deferred entry must remain pending beyond
max_deliveries.api/oss/src/utils/env.py (1)
528-532: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject non-positive record recovery settings.
AGENTA_RECORDS_RECLAIM_IDLE_MS=0makesStreamConsumerclaim pending entries on every loop. Negative idle values make Redis reclaim calls invalid.AGENTA_RECORDS_MAX_DELIVERIES<=0can make a healthy consumer ACK/delete a pending record before its next retry. Use_parse_optional_positive_int_envfor both settings.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.reclaim_idle_ms: int = ( _parse_optional_positive_int_env("AGENTA_RECORDS_RECLAIM_IDLE_MS") or 30_000 ) # Deliveries after which a record message is dropped instead of retried forever. A message # Postgres never accepts would otherwise hold every later message in the group. max_deliveries: int = ( _parse_optional_positive_int_env("AGENTA_RECORDS_MAX_DELIVERIES") or 5 )docs/design/session-control-and-live-events/spike-b-durable-commands-design.md (1)
519-521: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -eu file='docs/design/session-control-and-live-events/spike-b-durable-commands-design.md' printf '%s\n' '--- relevant identifiers and authorization terms ---' rg -n -C 4 'AGENTA_RUNNER_TOKEN|replica_id|sessions|lease|owner|claim|settle|consume|authorization|authenticate|project scope' "$file" printf '%s\n' '--- target context ---' sed -n '470,545p' "$file"Repository: Agenta-AI/agenta
Length of output: 50374
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learnings /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventionsLength of output: 23964
🏁 Script executed:
#!/bin/bash set -eu file='docs/design/session-control-and-live-events/spike-b-durable-commands-design.md' printf '%s\n' '--- authentication and claim contract ---' sed -n '494,545p' "$file" printf '%s\n' '--- runner contract and ownership-related terms ---' sed -n '637,670p' "$file" sed -n '918,1015p' "$file" printf '%s\n' '--- bounded ownership checks ---' rg -n -C 3 'not_held|held_sessions|owns|owner|lease|replica_id|runner token|AGENTA_RUNNER_TOKEN|server-side|authorize|authorization' "$file" | tail -n 180Repository: Agenta-AI/agenta
Length of output: 21726
Authorization Bypass (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Reachability: Internal · Exploitability: Difficult
Bind declared sessions to the authenticated runner.
The shared
AGENTA_RUNNER_TOKENdoes not bind a caller to itssessionsorreplica_id. The claim query and settlement guard trust both caller-supplied values, so a token holder can claim commands for another project and settle them. Require a server-side runner lease or owner check for every declared session, or use credentials scoped to the allowed sessions.services/runner/src/environment/mount-lifecycle.ts (1)
211-211: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash set -e file="services/runner/src/environment/mount-lifecycle.ts" wc -l "$file" sed -n '1,290p' "$file" printf '\n--- directly bound symbols ---\n' rg -n --glob '*.ts' 'function (throwIfAcquireAborted|rethrowIfInvariant)|const (throwIfAcquireAborted|rethrowIfInvariant)|throwIfAcquireAborted|rethrowIfInvariant|commitLocalMount|mountStorage|AcquireAbortedError' services/runner/srcRepository: Agenta-AI/agenta
Length of output: 22542
🏁 Script executed:
#!/bin/bash set -e printf '%s\n' '--- acquire-abort.ts ---' cat -n services/runner/src/environment/acquire-abort.ts printf '%s\n' '--- acquire-context.ts: commit and mount state ---' sed -n '90,155p' services/runner/src/environment/acquire-context.ts sed -n '235,305p' services/runner/src/environment/acquire-context.ts printf '%s\n' '--- context implementation: commit and cleanup-related methods ---' sed -n '90,175p' services/runner/src/environment/acquire-context-impl.ts printf '%s\n' '--- mountStorage success/abort path ---' sed -n '300,455p' services/runner/src/engines/sandbox_agent/mount.ts printf '%s\n' '--- callers and teardown references ---' rg -n -C 8 --glob '*.ts' 'mountLocalDurableCwd|mountLocalAgentCwd|mountedCwd|agentMountedPath|unmountStorage\\(' services/runner/src/environment services/runner/src/engines/sandbox_agent/environment.tsRepository: Agenta-AI/agenta
Length of output: 19676
🏁 Script executed:
#!/bin/bash set -e printf '%s\n' '--- mount lifecycle callers and remaining functions ---' sed -n '290,391p' services/runner/src/environment/mount-lifecycle.ts printf '%s\n' '--- environment acquisition and teardown around local mounts ---' sed -n '410,490p' services/runner/src/engines/sandbox_agent/environment.ts sed -n '520,590p' services/runner/src/engines/sandbox_agent/environment.ts sed -n '740,785p' services/runner/src/engines/sandbox_agent/environment.ts printf '%s\n' '--- all direct call sites ---' rg -n -C 6 --glob '*.ts' 'mountLocalDurableCwd|mountLocalAgentCwd' services/runner/srcRepository: Agenta-AI/agenta
Length of output: 34364
Commit each successful local mount before propagating acquisition aborts. Both helpers can receive
truefrommountStorage, then throw beforectx.commitLocalMount; teardown therefore has nomountedCwdoragentMountedPathto unmount. Commit immediately after success, then propagateAcquireAbortedError. InmountLocalAgentCwd,rethrowIfInvariantdoes not rethrowAcquireAbortedError, so the catch logs it as a normal mount failure and returnsfalse.📍 Affects 1 file
services/runner/src/environment/mount-lifecycle.ts#L211-L211(this comment)services/runner/src/environment/mount-lifecycle.ts#L247-L255services/runner/tests/unit/sandbox-agent-orchestration.test.ts (1)
2637-2640: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Assert that the paused execution remains registered
findExecution(... )?.settledisundefinedboth when the entry is present and unsettled and when the entry is absent. The test can therefore pass after the registry entry is removed, leaving the live-turn lookup and cancellation invariant untested.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const execution = findExecution( "11111111-1111-4111-8111-111111111111", sessionId, ); assert.ok(execution, "the paused turn stays registered"); assert.equal(execution.settled, undefined);services/runner/tests/unit/session-alive-interrupt.test.ts (1)
178-187: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the interval heartbeat and assert
onInterrupted.startAliveWatchdogstarts its 30-secondsetIntervalonly after the first beat.flushMicrotasks()waits 0 ms, and this test does not provide anonInterruptedcallback. Therefore it never observes the later interruption, so a broken production path can still pass. Use fake timers to advance one interval, then assertonInterruptedfires whilewatchdog.admittedremainstrue.services/runner/tests/unit/session-steer-mount-loss.test.ts (1)
457-459: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the fake teardown markers in the assertion.
makeHost().teardownemits onlyunmountandrmSync, soincludes("teardown")is always false. This test otherwise allows a refusal regression that acquires and tears down a rival environment to pass because it checks only the original environment'sdestroyedcount.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.assert.ok( !host.trace.some( (line) => line.includes("unmount") || line.includes("rmSync"), ), `a teardown ran during the refusal: ${host.trace.join(" | ")}`,web/mobile/src/features/chat/Composer.tsx (1)
52-53: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reduce the new in-code comments.
Each listed block exceeds one short line. Replace it with one brief reason-only comment, or remove it when the code is self-explanatory.
web/mobile/src/features/chat/Composer.tsx#L52-L53: reduce the placeholder comment to one short line.web/mobile/src/features/chat/LiveConversation.tsx#L150-L151: reduce the pending-task comment to one short line.web/mobile/src/features/chat/LiveConversation.tsx#L307-L311: remove or reduce the JSX comment to one short line.web/oss/src/components/AgentChatSlice/AgentConversation.tsx#L430-L437: reduce the refusal-recovery comment to one short line.web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx#L162-L167: reduce the error-state comment to one short line.As per coding guidelines, “Hard rule. At most ONE short line per comment.”
📍 Affects 4 files
web/mobile/src/features/chat/Composer.tsx#L52-L53(this comment)web/mobile/src/features/chat/LiveConversation.tsx#L150-L151web/mobile/src/features/chat/LiveConversation.tsx#L307-L311web/oss/src/components/AgentChatSlice/AgentConversation.tsx#L430-L437web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx#L162-L167Source: Coding guidelines
web/oss/src/components/AgentChatSlice/AgentConversation.tsx (1)
440-442: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restore consumed attachments on session-busy refusal.
finishSubmitcallsattachments.clearAttachments(consumedUids)immediately aftersubmit. The queue retainsfileParts, but this effect restores onlysent.text, so the staged files are removed and no other path restores them. Retain the consumed staged entries and pass them toattachments.restoreAttachmentsbefore the retry.web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (2)
86-97: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reduce new code comments to one short line each.
The web guideline has a hard one-short-line limit per comment. Move detailed rationale to documentation or encode it in focused tests.
web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts#L86-L97: replace the long recovery rationale with one short comment.web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts#L212-L213: replace the two-line recovery comment with one short comment.web/packages/agenta-chat/src/model/error.ts#L49-L59: replace the admission-contract block with one short comment.web/packages/agenta-chat/src/model/error.ts#L65-L72: replace the matching rationale block with one short comment.web/packages/agenta-chat/src/model/error.ts#L76-L78: remove the parity-history comment or reduce it to one short line.web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts#L367-L371: reduce the scenario explanation to one short line.web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts#L390-L391: reduce the queue-state explanation to one short line.web/packages/agenta-chat/tests/unit/model/error.test.ts#L87-L90: reduce the runner-path explanation to one short line.📍 Affects 4 files
web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts#L86-L97(this comment)web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts#L212-L213web/packages/agenta-chat/src/model/error.ts#L49-L59web/packages/agenta-chat/src/model/error.ts#L65-L72web/packages/agenta-chat/src/model/error.ts#L76-L78web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts#L367-L371web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts#L390-L391web/packages/agenta-chat/tests/unit/model/error.test.ts#L87-L90Source: Coding guidelines
214-214: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash set -eu file='web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts' printf '%s\n' '--- focused source ---' sed -n '1,260p' "$file" printf '%s\n' '--- bound symbols and callers ---' rg -n -C 3 'lastSentRef|takeLastSent|refus|release|send' "$file" web/packages/agenta-chat/src printf '%s\n' '--- focused diff ---' git diff --unified=30 -- "$file"Repository: Agenta-AI/agenta
Length of output: 50373
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learningsLength of output: 4671
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- hook consumers ---' rg -l 'useAgentChatQueue|takeLastSent' web --glob '*.ts' --glob '*.tsx' | sort printf '%s\n' '--- exact consumer context ---' rg -n -C 12 'useAgentChatQueue|takeLastSent' web/packages web/apps --glob '*.ts' --glob '*.tsx' printf '%s\n' '--- refusal classification and handlers ---' rg -n -C 12 'SESSION_TURN_IN_USE|isSessionTurnInUse|takeLastSent|sendQueued|onError' web/packages/agenta-chat web/apps --glob '*.ts' --glob '*.tsx'Repository: Agenta-AI/agenta
Length of output: 31616
🏁 Script executed:
#!/bin/bash set -eu file='web/packages/agenta-chat/src/hooks/useAgentConversation.ts' printf '%s\n' '--- hook outline ---' ast-grep outline "$file" printf '%s\n' '--- queue integration ---' sed -n '100,190p' "$file" sed -n '450,570p' "$file" printf '%s\n' '--- all host reclaim/error references ---' rg -n -C 10 'takeLastSent|sendQueued|isSessionTurnInUse|SESSION_TURN_IN_USE|onError|sendMessage' web/packages/agenta-chat/src/hooks/useAgentConversation.ts web/oss/src/components/AgentChatSlice/AgentConversation.tsxRepository: Agenta-AI/agenta
Length of output: 38981
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- OSS refusal effect and surrounding hook order ---' sed -n '330,455p' web/oss/src/components/AgentChatSlice/AgentConversation.tsx printf '%s\n' '--- release predicate binding ---' rg -n -l 'canReleaseQueuedMessage' web/packages web/oss rg -n -C 18 'canReleaseQueuedMessage' web/packages/agenta-playground web/packages/agenta-* web/oss 2>/dev/null | head -240 printf '%s\n' '--- package test setup and relevant release tests ---' sed -n '360,425p' web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsRepository: Agenta-AI/agenta
Length of output: 34540
Protect refusal recovery before releasing the queue head.
When a refusal exposes
status === "error"with no pending HITL interaction, the queue can release B. Its effect stores B in the single-slotlastSentRefbefore the OSS refusal effect callstakeLastSent, so A can be lost. Preserve recovery by send identity or reclaim A before releasing B.web/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts (1)
1-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reduce these comments to one short line each.
web/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts#L1-L6: replace the reload explanation with one short comment.web/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts#L10-L12: replace the storage-stub explanation with one short comment.web/packages/agenta-entities/src/workflow/state/store.ts#L1469-L1473: replace the persistence overview with one short comment.web/packages/agenta-entities/src/workflow/state/store.ts#L1481-L1486: replace the normalization overview with one short comment.As per coding guidelines: “Hard rule. At most ONE short line per comment.”
📍 Affects 2 files
web/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts#L1-L6(this comment)web/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts#L10-L12web/packages/agenta-entities/src/workflow/state/store.ts#L1469-L1473web/packages/agenta-entities/src/workflow/state/store.ts#L1481-L1486Source: Coding guidelines
mmabrouk
commented
Sep 4, 2026
mmabrouk
left a comment
mmabrouk
left a comment
Member
Author
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
Verdict: CHANGES NEEDED
The rebase keeps the #6496 warm-Stop/Codex-reap work and the #6500 admission checks, but their integration with durable Stop leaves two release-blocking races. I also found one retry-contract defect and confirmed the flag-off desktop response still fails its client schema.
The flag-off transport itself is preserved: desktop reaches the legacy API service through /cancel, mobile continues to use the legacy stream command, and the runner receives the heartbeat-carried interrupt. The remaining client-side issue is that desktop waits for fetchSessionStream before aborting its local stream, so a slow read delays both local feedback and either Stop request.
No focused test was run; the failures follow from the current control flow, and the existing suites are green but do not compose registry replacement with asynchronous admission or abort a turn after PAUSED has won the prompt race.
|
|
||
| // Register only a request that passed synchronous admission validation. | ||
| if (sessionOwned) { | ||
| registerExecution({ |
Member
Author
There was a problem hiding this comment.
[P1] Register only after asynchronous admission succeeds
If turn A is admitted and a second turn B arrives for the same session, this call replaces A in the one-entry registry before B's first heartbeat says admitted: false. A Stop for A during that await sees B, reports the pinned A target as not_running, and never aborts A. B then unregisters itself on refusal, leaving A running with no registry entry, so later Stops return not_held as well. Keep A registered until B actually wins admission (or make registration an admission-aware compare-and-set), and cover Stop during a refused second turn.
| // still make the teardown treat the run as aborted, which DESTROYS the warm environment | ||
| // instead of parking it. Marked here rather than where the caller awaits this function, | ||
| // because that window is precisely what lies between the two. | ||
| if (stopReason !== "paused" && request.sessionId && request.turnId) { |
Member
Author
There was a problem hiding this comment.
[P1] Convert a Stop during pause teardown into cancellation
Leaving a paused turn unset here keeps it discoverable, but it does not keep its abort actionable. Once pause.signal has won Promise.race, stopReason is fixed to paused; a later Stop during waitForEventDrain or tool-call closure aborts a promise nobody awaits. The control channel reports stopped, while this function skips cancelHarnessTurn, returns paused, and approvalToPark deliberately ignores the aborted signal and parks the same prompt. Re-check the Stop signal during pause teardown and take the cancel path (or otherwise make the parked prompt cancellable) before reporting a terminal stopped outcome.
| session_id=session_id, | ||
| ) | ||
|
|
||
| if ( |
Member
Author
There was a problem hiding this comment.
[P2] Replay an idempotency key before validating current execution
An exact retry can no longer replay once the target changes. For example, the first request uses key K plus expected_execution_id=A, A settles, and B starts; retrying the same request now raises a 409 here before the DAO can return K's original command. The existing replay test omits the expected ID even though the browser-facing call supports sending both. Resolve K first (and verify its request identity), then return its stored target/state without consulting B.
| requestOptions, | ||
| ) | ||
| .withRawResponse() | ||
| const validated = safeParseWithLogging( |
Member
Author
There was a problem hiding this comment.
[P2] Parse the legacy response when durable Stop is disabled
With AGENTA_SESSIONS_DURABLE_STOP=false, this same endpoint intentionally returns the legacy {mode, session_id, turn_id, ...} body. Parsing it only as sessionCancelExecutionResponseSchema fails and logs a Zod error on every desktop Stop, even though the API already performed the legacy cancellation. Accept and normalize the legacy schema here, or otherwise avoid treating the documented rollback response as malformed; add the flag-off payload to the frontend test.
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
1 participant
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.
Agent-generated, low weight. Draft. Do not merge before Mahmoud reviews it.
Implements the Spike B design with the direct adapter: the
session_commandstable,POST /sessions/{id}/cancelwith an optionalexpected_execution_id, the control-delivery port, the runnerPOST /cancelbeside/kill, the outcome route, and settlement that releasesrunningunder the owner check and leavesaliveto its TTL. Long polling is designed (PR #6497) and parked in Linear AGE-4253. Heartbeats stay a health and ownership signal. Stacked on Spike A (PR #6496).Evidence
58bec4d382applied/stopped,park-cancelled, next message recalled the codeword58bec4d382expected_execution_id; two Stops; Stop on a parked approvalobsolete200; 409 nothing written; one command; interaction cancelled, environment kept, resume warm58bec4d382is_running: true(the runner's final beat landed 0.88 s after settlement and the tombstone check refused it before the mirror write); the collection query every liveness poll drives is served from Postgres alone76e4b1368bmirror_livenessat settlementis_running: falseat Stop plus 0.15 s,is_alive: true; warm resume 2.01 s inside the park window against a 14.16 s cold rebuild after it9110c08000(integration)runningonly; a parked approval holds the turn inalive); the same Stop without an id cancelled the gate89d7c7c90dpendingtocancelled; a stale id still 409, naming the turn that would have been stopped9110c08000claimeduntil the watchdog settled itlost(one case exactly 120 s)89d7c7c90dpendingorclaimed; the duplicate gets 40989d7c7c90dno-park:end_turn), cold rebuild 7.21 s38cbc92201obsolete/not_running, parked, warmhit-continue1.91 s76e4b1368bdocker restartthe runner, then continueownerlease (120 s) expired: the runner never releases its owner claim at shutdown and a restarted runner has a new replica id; deleting only the owner key admitted the next message at onceTrade-off taken on
38cbc92201: past teardown, the API now discriminateslostfromnot_runningon therunningkey instead of the heartbeat, so in a multi-runner deployment a Stop misrouted to a replica that holds a parked approval settlesnot_runninginstead oflost. Multi-replica direct delivery is not version one; open question 8 in the report.Tests
38cbc92201).cd services/runner && pnpm test: 2671 pass, 160 files (runpnpm run build:extensiononce in a fresh worktree first).@agenta/entities1468,@agenta/navigation46,@agenta/mobile144.uvx ruff@0.15.12 formatandcheck: clean.pnpm lint-fix: clean.Branch head
38cbc92201onfeat/session-durable-cancel, basespike/session-cancel-warm. Reports:slice-durable-cancel.mdon this branch;evidence-2026-09-03/post-stop-mirror.md,post-stop-liveness.md, anddaytona-pi-claude.mdon PR #6505.https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Restack of 2026-09-04
This PR now carries the rollback switch itself, so it is safe on its own:
AGENTA_SESSIONS_DURABLE_STOP(default off) inenv.py; with it off the cancel route returns the exact legacy response and takes the heartbeat-carried Stop, and nosession_commandsrow is written. Commits50bffdc8efande9858e64acmoved down from PR #6501. Also5a66bf0e12narrows the runner-control auth exemption from/sessions/control/to/sessions/control/commands/, so a session whose id is literallycontrolstill needs project auth (a whole-PR review finding). Head5a66bf0e12; API sessions suite 568 passed. Agent-generated, low weight.Cancelled interaction record (2026-09-04, afternoon)
The browser pass showed an approval card that stayed pending after a Stop, because the cancel updated the interaction row and published a notice but wrote no transcript record. Commit
e4cbd64ac9writes one cancelled interaction record per cancelled interaction (deterministic id, identifiers only, published after commit); a later answer gets 409 "Interaction is no longer pending". Reviewed: passes. Known item, non-blocking: the desktop clears the card from the interaction row state, and its record handler ignores the new record'soutcome: cancelledshape; a records-only consumer would need the client handler aligned (client side, PR #6504). The record append is a separate transaction from the cancel commit, as on the normal-answer path.