[feat] Session control milestone 1: warm Stop, durable Stop, recovery - #6553
Conversation
The records stream worker added every decoded Redis message id to its acknowledged list during deserialization, before it attempted the Postgres write. A failed `append_many` logged an error and continued, and the shared consumer loop then acknowledged and deleted those messages from the stream. Every Postgres failure was therefore permanent, silent record loss, and the worker reported success while doing it (#5496). `append_many` is one statement in one transaction, so one record Postgres rejected also took its whole batch with it, losing up to fifty unrelated records per rejection (#5594). Three changes: - `process_batch` returns a message id only once its rows are committed, or once the worker has decided to drop it on purpose (undecodable, or over quota). A failed entitlements check now defers instead of dropping, because an unreachable meter is transient. - A failed group is rewritten one record at a time, so a rejected record no longer discards the rest of its batch. - `StreamConsumer` gains an opt-in reclaim pass. `read_batch` only ever asks for `>`, so without it an unacknowledged entry is invisible to every later read and "leave it pending" would still lose the record. The pass claims the group's pending entries, and drops one after `max_deliveries` failures with an error log naming the lost record. The drop applies only while other records are committing. The delivery counter cannot tell a rejected record apart from a database that is down, so dropping on the count alone would delete every record in flight once an outage outlasts the budget. A live run against a real Redis found that hole; the guard closes it. The reclaim pass is off for the tracing and events workers, so their behaviour is unchanged. Verified against a real Redis 8: five records published during a twenty second write outage stayed pending, then all landed on recovery with no duplicates and an empty stream; a permanently rejected record let its batch mates through and was dropped loudly once traffic resumed. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
`RecordsRetentionDAO.delete_records_before_cutoff` selected and deleted on `RecordDBE.id`. That attribute does not exist. The records key is `(project_id, record_id)`, so every call to the retention flush raised before it deleted anything and records have never been aged out. Scope added on purpose: this defect is clear, obvious and one line, it sits in the records durability area this branch already touches, and Spike D found it while auditing the same pipeline. It is kept in its own commit so it can be reverted or landed alone. Verified: `hasattr(RecordDBE, "id")` is False, the primary key constraint at `dbes.py:18` is `(project_id, record_id)`, and the corrected statement compiles against the Postgres dialect. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Port the durable-cancel spike's 13-cell driver (refresh_live.py) into the release-gate skill as resources/session_control.py, per qa-audit-2026-09-03.md section 4, so the standing regression check survives outside one evidence folder. Matches the gate's env contract (AGENTA_BASE, AGENTA_ADMIN_KEY, QA_OPENAI_API_KEY, no file fallback), moves the Docker/Postgres-only helpers behind an OperatorHooks interface so six cells run over HTTP against any deployment and the rest SKIP by name without --project, emits the gate's PASS/FAIL/SKIP result shape into a timestamped ~/agenta-qa-evidence/ run folder, and adds --resume so a lost agent costs one cell, not the run. Adds two new cells (repeat-stop, stop-during-completion) from qa-audit section 3, a path_triggers.py rule that makes the suite mandatory for session-code changes, a SKILL.md section naming the command and the model-key locations, and a pytest-and-standalone-runnable unit test for the pure parts (cell registry, hooks skip path, resume, verdict shape, env resolution). Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
The live smoke run against the integration stack showed /sessions/{id}/cancel
returns 202 Accepted (a pending command plus a stopping execution), the
correct async-acceptance status. The verdict checks in stop-warm,
stop-approval, repeat-stop, and stop-during-completion hardcoded 200 and
FAILed every real Stop. Accept 200 or 202 in each.
Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…ontrol.py Add HARNESSES["claude"] (kind claude, model sonnet, provider anthropic, vault connection) so the session-control cells can drive the Claude Code harness, and stock an Anthropic provider key into the bootstrapped account's vault the same way the OpenAI key is stocked, gated on --harness claude so a pi_core/codex-only run does not need ANTHROPIC_API_KEY set. Also widen wait_for_turn/wait_for_tool by a configurable SANDBOX_STARTUP_SLACK_S (25s) when --sandbox daytona is selected, since a Daytona sandbox takes 10 to 20s to start on top of local timings. Record the session's distinct sandbox ids (via /sessions/turns/query, HTTP-only) in every HTTP-only cell's evidence as sandbox_ids / warm_same_sandbox, so a resume that silently rebuilt the sandbox is visible in the result instead of only in the recalled codeword. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
The Daytona smoke run FAILed stop-approval with only "resume did not recall the codeword" and no reply text to check why, so a driver replay bug (the reconstructed output-denied tool part) could not be told apart from a genuine product miss. Add resume_text, resume_frames, and resume_errors to the cell's evidence. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Two invocations started in the same second (e.g. Claude Code and Daytona smoke runs fired in parallel tonight) shared a run folder, since the timestamp alone has 1-second resolution -- the second writer silently overwrote the first one's results.json mid-run and one run's evidence was lost until recovered from its redirected stdout log. Add the PID to the folder name so concurrent invocations never collide. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…imeout
assistant_message() indexed turn["segments"] unconditionally. When the
driver's own wait for a turn times out (handle["out"] stays None,
observed when the runner is unhealthy after a restart), the cell passed
an empty {} dict in and the KeyError masked the real signal, which is a
driver-side timeout rather than a cell result.
Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Add cell_runner_gone, ported from cell_runner_gone in refresh_live.py, as cell "runner-gone" in the session-control driver's registry. It restarts the runner right after a Stop is claimed, then checks that the sweep settles the command as lost (not claimed) in session_commands, the session_streams row reads is_running: false, and a Send sent after that runs. Register it in CELLS and in the registry's stable-names unit test, and update SKILL.md's cell count and Docker-needing cell list. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
…ncurrent-stops Add OperatorHooks.ensure_runner_healthy(), implemented in DockerComposeHooks, that unpauses, restarts, and health-checks the runner as needed. Extract the per-cell execution in main() into run_cell(), which calls it in a finally block after every needs_hooks cell, so a cell that raises before its own restore code runs (as cell_stale_tail did tonight, leaving the runner paused) cannot strand the runner for the next cell. Also wrap cell_stale_tail's pause/unpause and cell_records_outage's stop/start Postgres in their own try/finally, so each cell restores what it touched even on an exception in between. Add cell "concurrent-stops": five sessions started at once with a long turn, Stop sent to all five within about a second, each expected to return HTTP 202, settle exactly one terminal record, and recall its own codeword on a warm resume. HTTP-only, no hooks needed. Add unit tests for run_cell's finally path (NullHooks skips the recovery call without crashing; a stub hooks object confirms the recovery call fires when a cell raises, and is skipped for a cell that does not need hooks) and add "ensure_runner_healthy" to the NullHooks-raises coverage. Update SKILL.md's cell count and lists. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
… both races Split cell_runner_gone into two: the new cell_runner_gone pauses the runner BEFORE sending the Stop, so the command can never be claimed or reported and must settle lost off a deterministic sweep, with an explicit check for the watchdog's execution_lost ending. cell_runner_gone_late keeps the old restart-after-stop timing, which mostly loses that hard race because the runner often reports the Stop's outcome before it actually dies. Both races satisfy the same invariant: exactly one effective terminal outcome, no command left pending or claimed, is_running false, and the next Send succeeds. Factor that shared PASS rule into _judge_runner_gone(), used by cell_runner_gone_late (cell_runner_gone keeps its own stricter assertion since pausing first is meant to force the lost/execution_lost shape every time). Both record which race landed on evidence["race"]. Register runner-gone-late in CELLS and the registry's stable-names test. Update SKILL.md's cell count and Docker-needing cell list. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Keep one guard-clearing wrapper around the aliased chat send and regenerate methods so hook initialization and immediate-Stop fencing both remain correct. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Preserve the backend stopping turn marker through the frontend session schema. Recover matching desktop and mobile stop guards across remounts until settlement clears the marker. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
fix(sessions): guard Stop by execution and cancel pending approvals
Reject invalid reclaim and delivery settings during configuration load. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Fail closed until the first heartbeat confirms ownership and authenticate runner-initiated ownership release. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Bound cancel requests, reject unknown Codex reap outcomes, and isolate all keepalive TTL settings in tests. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Publish successful local mounts to teardown state before observing cancellation. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Guarantee one done record when a session-owned run throws outside the engine. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Retain sends until admission, restore attachments, and avoid overwriting a newer composer draft. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Expose skipped cells as untested, make the offline script fixture-aware, and require the durable late-answer conflict. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document owner routing limits, transport requirements, fail-closed admission, cleanup continuity, and sanitized test evidence. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Treat post-cancel process reaping as best effort without changing the harness-confirmed cancellation outcome. Log cleanup misses for QA and cover failed and unknown reap paths through parking and continuity. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep the recovery slot when a newer composer draft blocks restoration. Consume it only after refused text and staged attachments are placed safely, with regressions for the occupied-composer case. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document that Codex cleanup misses are QA evidence rather than a teardown signal. Keep settled Stop parking and continuity under the 600-second stopped-session window. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Apply the repository Prettier style to the inherited test file so the TypeScript format check passes. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Move a refused send into a conversation-local holding slot before a newer submission can replace the queue recovery value. Restore its text and staged attachments once the composer becomes empty, without submitting it automatically. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Capture the refused send into the conversation-local holding slot before scheduling editor placement. Cover the interleaving where a newer submission replaces the queue recovery value before the frame runs. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
fix: address CodeRabbit review for session control
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.py (1)
64-118: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the hand-rolled account fixture.
The new DAO test uses
project, which creates users, organizations, workspaces, and projects directly. Reuse an account fixture fromapi/oss/tests/pytest/utils/accounts.py.As per coding guidelines, reuse
foo_account/cls_account/mod_accountand do not hand-roll account creation.Source: Coding guidelines
🟠 Major comments (24)
api/oss/src/tasks/asyncio/sessions/records_worker.py-109-109 (1)
109-109: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftPersist permanent rejection state across workers.
Line 109 stores the rejection state only in one process. Any replica can claim the pending entry, and a restart clears this set. A later claimant then treats the same
DataErrororIntegrityErroras retryable, so it never reachesdrop_expiredaftermax_deliveries. Persist the classification by stream message ID, or drop the isolated rejected entry immediately. Add a cross-worker or restart redelivery test.docs/design/session-control-and-live-events/spike-b-durable-commands-design.md-806-808 (1)
806-808: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAlign the parked-session outcome with the state machine.
The state machine defines
claimed -> obsoletefornot_running, but this flow says to settle the command asappliedwithexecution.state = "not_running". Choose one terminal command state and update the transition table, DTO contract, settlement route, and tests together.docs/design/session-control-and-live-events/spike-b-durable-commands-design.md-1005-1010 (1)
1005-1010: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine the claim transition for direct delivery.
The direct sequence inserts a
pendingcommand and then calls the runner. Outcome settlement only updatesstate='claimed' AND claimed_by=<replica_id>, while the direct adapter'sacknowledgeis a no-op. A successful direct call therefore has no defined claim owner and can fail settlement with 409. Claim the command atomically before delivery, or define an equivalent direct-delivery claim path and test it.docs/design/session-control-and-live-events/spike-b-durable-commands-design.md-979-981 (1)
979-981: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal · Exploitability: Moderate
Require encrypted transport for
AGENTA_RUNNER_TOKEN.
AGENTA_RUNNER_INTERNAL_URLacceptshttp://values, and the client sends the bearer token without a transport check. Reject cleartext URLs and require HTTPS with certificate validation or an equivalent authenticated encrypted channel.docs/design/session-control-and-live-events/spike-b-durable-commands-design.md-539-543 (1)
539-543: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBind settlement to a claim-specific lease identity.
claimed_byuses the caller-suppliedreplica_id, but claims expire and can be re-delivered. The settlement predicate has no claim generation. An old outcome can therefore settle a newer claim when the same or a reusedreplica_idholds it. Add a server-issued claim token or generation to the claim and outcome, and include it in the settlement predicate.services/runner/src/environment/mount-lifecycle.ts-227-227 (1)
227-227: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRecord confirmed detachment before checking cancellation.
If
mountStoragereturnsfalseand the signal is aborted, this throws beforectx.markCwdDetachConfirmed(). The failure teardown then skipscleanupWorkspace()because the cwd remains unsafe, although the mount helper confirmed that it is detached. Mark detachment first, then throw for cancellation. Add a regression case wheremountStorageaborts the signal and returnsfalse.Proposed fix
- throwIfAcquireAborted(deps.signal); - ctx.markCwdDetachConfirmed(); + ctx.markCwdDetachConfirmed(); + throwIfAcquireAborted(deps.signal);services/runner/src/engines/sandbox_agent/cancel-turn.ts-116-121 (1)
116-121: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDistinguish a stalled cancel request from a client that cannot cancel.
The request-timeout branch returns the shared
unsettledresult, sorequestedstaysfalse. A stalledcancelSessionmay already have put the notification on the wire, and the harness prompt may still be open.
stopParkedApprovalSessioninservices/runner/src/server.ts(lines 977-990) reads that shape as "no ACP cancel could be SENT", logsreject-only (client has no cancelSession), and reparks the session warm. The comment there states the only cause is an unpatched client. This branch adds a second cause, so a possibly-running turn is now presented as idle instead of failing closed, and the log line names the wrong reason.Report the timeout as its own outcome so the caller can fail closed.
🛠️ Proposed change
/** True when the cancel notification left the runner, whatever the harness did next. */ requested: boolean; + /** True when the cancel request itself never completed inside the budget. */ + requestTimedOut?: boolean;if (requested === TIMED_OUT) { input.log( `stage=harness_cancel sent=false reason=request-timeout budget_ms=${timeoutMs}`, ); - return unsettled; + return { ...unsettled, requestTimedOut: true }; }Then treat
requestTimedOutas unsafe-to-park instopParkedApprovalSession.services/runner/src/sessions/control-channel.ts-271-287 (1)
271-287: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the outcome report with a timeout.
reportOutcomeawaitsfetchwith no deadline.applyCommandawaitsreport, and the/cancelroute awaitsapplyCommand. If the API accepts the connection and never answers, the runner holds the caller's/cancelrequest open for the platform's full socket lifetime, and the Stop looks stuck to the user. The comment above the report says a Stop that worked must not look stuck, so a hung report defeats that intent.Add an
AbortSignal.timeout(...)to the request. A failed report is already safe: the log records it and the API's settlement sweep repairs the command.♻️ Proposed fix
const res = await fetch(url, { method: "POST", redirect: "error", + signal: AbortSignal.timeout(OUTCOME_REPORT_TIMEOUT_MS), headers: {Declare the budget beside the other module constants:
/** A report that cannot land promptly must not hold the `/cancel` caller open. */ const OUTCOME_REPORT_TIMEOUT_MS = 10_000;api/oss/src/dbs/postgres/sessions/streams/dao.py-544-551 (1)
544-551: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe guarded update path diverges from the shared edit mapper. The new
expected_turn_idbranch hand-builds avaluesdict instead of writing the field setmap_stream_dto_to_dbe_editowns. Two fields are lost:tags/meta, and theturn_started_atstamp that fires whenturn_idchanges.
api/oss/src/dbs/postgres/sessions/streams/dao.py#L544-L551: addtagsandmetatovalues, and setturn_started_atto the current UTC time whenstream.turn_iddiffers fromstream.expected_turn_id.api/oss/src/dbs/postgres/sessions/streams/mappings.py#L209-L213: this stamp is the invariant the DBE comment relies on. Keep the two write paths in agreement, or route the guarded path through a shared helper so a third path inherits both rules.api/oss/src/apis/fastapi/sessions/router.py-1988-1994 (1)
1988-1994: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPublish the cancellation response contract.
Because
cancel_session_executionreturnsJSONResponseand the route declares noresponse_modelorresponses, FastAPI does not publish schemas for its successful responses in OpenAPI. RegisterSessionStreamCommandResponsefor the legacy200response andSessionCancelResponsefor the durable200and202responses so generated clients can consume this public API reliably.Source: Coding guidelines
api/oss/src/core/sessions/records/service.py-273-274 (1)
273-274: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake the durable branch default to quarantine.
env.agenta.sessions.late_outputaccepts"quarantine"and"reject". When it is"reject",_handle_by_execution_statecurrently drops late records because it appends only whenaction == "quarantine". This differs from_handle_late_events, which drops only for"reject".- if action == "quarantine": - guarded.append(event.model_copy(update={"quarantined_at": now})) + if action == "reject": + continue + guarded.append(event.model_copy(update={"quarantined_at": now}))api/oss/src/dbs/postgres/sessions/commands/dao.py-303-305 (1)
303-305: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not claim command kinds that this replica cannot map.
claim_commandsmarks every selected row as claimed before this helper drops unknown kinds. The row then remains leased to a replica that cannot execute it, so a compatible replica cannot claim it until lease expiry. Filterselectableto supported kinds before the update, and test the full claim path.api/oss/src/dbs/postgres/sessions/commands/dao.py-221-221 (1)
221-221: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftIDOR (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Reachability: External · Exploitability: Difficult
Bind runner outcome handling to the command’s project.
The outcome endpoint accepts a command UUID under one shared runner token.
fetch_commandthen loads any matching command withoutproject_id, and the service can settle that command. Pass an authenticated project scope or a command-scoped capability through the runner request. Do not rely on the command UUID alone.Source: Coding guidelines
api/oss/src/core/sessions/interactions/service.py-117-117 (1)
117-117: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftReturn a typed result DTO.
cancel_session_pendingreturns a rawint. Return a Pydantic DTO such asSessionPendingCancellationResultand update its callers.As per coding guidelines: “Service methods must return typed DTOs (Pydantic
BaseModelsubclasses), not raw dicts, tuples, orAny.”Source: Coding guidelines
api/oss/src/dbs/postgres/sessions/executions/dao.py-129-147 (1)
129-147: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftScope this DAO read by
project_id.
list_redis_unreconciledreturns rows from every project. Keep this DAO read project-scoped. If reconciliation must run globally, enumerate projects in privileged orchestration and call a project-scoped DAO method.As per coding guidelines: “Always enforce tenant scope (
project_idminimum) in DAO reads and writes.”Source: Coding guidelines
api/oss/src/core/sessions/streams/service.py-347-351 (1)
347-351: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFence lifecycle writes against a newer turn.
After Stop clears the old locks, a new turn can start before either row write completes. Both paths can overwrite the new turn's flags with stale terminal state. Use
expected_turn_idor an equivalent transaction fence, and publishendedonly when that fence succeeds.
api/oss/src/core/sessions/streams/service.py#L347-L351: fence_mark_stream_endedto the cancelled turn before writing terminal flags.api/oss/src/core/sessions/streams/service.py#L1282-L1286: pass the settled turn into the mirror write and reject a write after a newer generation replaces it.api/oss/src/core/sessions/streams/dtos.py-178-193 (1)
178-193: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the duplicate
expected_execution_iddeclaration.This redeclares the field and validator from Lines 155-171. It also drops the first field description. Flake8 reports F811 for the duplicate validator.
Proposed fix
- expected_execution_id: Optional[str] = Field( - default=None, - description=( - "Optional stale-request guard honored only in cancel mode; ignored for send, " - "steer, and attach." - ), - ) - - `@field_validator`("expected_execution_id") - `@classmethod` - def _blank_expected_execution_id_means_absent( - cls, value: Optional[str] - ) -> Optional[str]: - if value is None: - return None - return value.strip() or None - # Cancel guard (RFC D-010). Public name; internally this IS a turn id — the coordinationSources: Coding guidelines, Linters/SAST tools
services/runner/src/sessions/alive.ts-395-395 (1)
395-395: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal · Exploitability: Difficult
Require authenticated transport before sending the runner token.
apiBase()explicitly permits the HTTP URLhttp://api:8000. This request sendsAGENTA_RUNNER_TOKENin cleartext, allowing a network observer to capture the shared runner credential.web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts-152-154 (1)
152-154: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep each new code comment to one short line.
web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts#L152-L154: Replace with one short reason comment.web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts#L270-L271: Replace with one short reason comment.web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts#L280-L281: Replace with one short reason comment.web/oss/src/components/AgentChatSlice/hooks/useSessionRecordsWatch.ts#L46-L50: Replace with one short reason comment.
As per coding guidelines, “Hard rule. At most ONE short line per comment.”Source: Coding guidelines
docs/design/session-control-and-live-events/api-design.md-231-236 (1)
231-236: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal · Exploitability: Moderate
Require encrypted transport for every runner-control hop.
The API sends
AGENTA_RUNNER_TOKENto the runner overAGENTA_RUNNER_INTERNAL_URL, which defaults tohttp://runner:8765. The runner documentation defers TLS and relies only on network isolation and bearer authentication. Require HTTPS with certificate validation for every credential-bearing request, and reject plaintext runner URLs outside explicit development configurations. Keep redirects disabled or prevent any HTTPS downgrade.docs/design/session-control-and-live-events/api-design.md-58-60 (1)
58-60: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftEnsure desktop Stop does not omit
expected_execution_id.The desktop path passes
expectedExecutionIdwhen its in-memory turn ID exists. However,useAgentConversationclears that ID before each send and restores it only after a streamed turn ID appears. A Stop during this gap sends noexpected_execution_id; if a newer execution starts before the request arrives, Stop may cancel the newer execution. Keep the ID available before enabling Stop, or reject the unguarded request.docs/design/session-control-and-live-events/slice-durable-cancel.md-192-200 (1)
192-200: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAdd abandoned-command settlement before enabling durable Stop.
When the runner never reports, the command remains
claimedandstopping_turn_idremains set indefinitely. This blocks the session and violates the Stop recovery objective. Add claim expiry and terminal settlement before enablingAGENTA_SESSIONS_DURABLE_STOP, and make the release gate cover this failure path.Also applies to: 217-218
docs/design/session-control-and-live-events/slice-stop-guard.md-113-116 (1)
113-116: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign the documented cancel-result contract with the clients.
This section documents
cancelSessionStreamas returningcancelled | stale | failed, but the supplied consumers also handleidlefor the no-running case (web/mobile/src/features/chat/LiveConversation.tsx:272-339andweb/mobile/src/features/chat/StopButton.tsx:10-27). Define one result union and update the documentation and consumers together.docs/design/session-control-and-live-events/slice-watchdog.md-77-79 (1)
77-79: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep watchdog rows recoverable until settlement cleanup completes.
The sweep can remove retry eligibility before all effects are durable. If
settled_turnslookup fails, it writes no terminal record but still collapses the row. If it crashes after row collapse and before Redis cleanup, the next pass skips the row whilealiveandrunningkeys remain. Keep the row in a cleanup-pending state, or retry settlement before clearingis_running.Also applies to: 95-96
🟡 Minor comments (12)
services/runner/src/engines/sandbox_agent/reap-exec.ts-42-43 (1)
42-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign this header with the shipped park decision.
The header states that a failed or inconclusive reap prevents a safe park. The only consumer disagrees:
run-turn.ts(lines 1387-1405) callsreapResultHasCleanupMissand logscleanup_miss=true, then keeps the harness-confirmed park. Its own comment states the cleanup is best effort and does not change the park decision.cancel-continuity.test.tspins that behavior for both reap-failure modes.Update this header to describe the log-only outcome, so a later reader does not treat the reap result as a park gate.
services/runner/src/engines/sandbox_agent/sandbox-liveness.ts-270-270 (1)
270-270: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the disable flag value-aware.
Any non-empty value for
AGENTA_RUNNER_SANDBOX_PROBE_DISABLEDdisables the poll, includingfalseand0. An operator who setsAGENTA_RUNNER_SANDBOX_PROBE_DISABLED=falseturns off the exact detection this module adds, and only the informational log inrun-turn.tsreports it.
run-turn.tsline 304 shows the repository's opposite convention for a compose-supplied flag: only the literal"false"opts out. Parse the value here so the two flags cannot be read in opposite directions.🛠️ Proposed change
- if (probe && !process.env[PROBE_DISABLED_ENV]) schedule(); + const disabled = /^(1|true|yes)$/i.test( + (process.env[PROBE_DISABLED_ENV] ?? "").trim(), + ); + if (probe && !disabled) schedule();services/runner/tests/unit/turn-settle.test.ts-212-215 (1)
212-215: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStub the limit env vars to empty before asserting the defaults.
resolveTurnSettleLimits()readsHARD_DEADLINE_ENVandABANDON_GRACE_ENV. This test asserts the exact defaults without neutralizing them first. If either variable is set in the shell or by a loaded dev env file, the test fails for a reason unrelated to the code under test. The same problem was fixed inservices/runner/tests/unit/harness-cancel-park.test.tsby isolating the TTL variables around the default assertions.
vi.stubEnvwithundefineddeletes the variable, and the existingafterEachalready callsvi.unstubAllEnvs().♻️ Proposed fix
it("keeps the hard deadline above the longest legitimate run", () => { // A backstop that fired before the run limits would shorten real runs, which is the // opposite of what users have asked for (issues `#6084`, `#5356`). + vi.stubEnv(HARD_DEADLINE_ENV, undefined); + vi.stubEnv(ABANDON_GRACE_ENV, undefined); expect(DEFAULT_HARD_DEADLINE_MS).toBeGreaterThan(DEFAULT_TOTAL_DEADLINE_MS); expect(resolveTurnSettleLimits()).toEqual({ hardDeadlineMs: DEFAULT_HARD_DEADLINE_MS, abandonGraceMs: DEFAULT_ABANDON_GRACE_MS, }); });api/oss/src/apis/fastapi/sessions/router.py-2040-2040 (1)
2040-2040: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject overlength idempotency keys.
Line 2040 truncates keys instead of rejecting them. Two distinct keys with the same first 255 characters then address one command, causing an unintended replay or a conflict. Reject keys over
_MAX_IDEMPOTENCY_KEY_CHARACTERSbefore normalization.services/runner/src/server.ts-1129-1132 (1)
1129-1132: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject missing or invalid
createdAtwith HTTP 400. WhencreatedAtis absent or not parseable,Date.parsereturnsNaN, so the applier skips the stale-Stop guard and can calllive.abort()on a newer turn. Validate thatcreatedAtis a parseable timestamp before enqueueing the cancel command.web/packages/agenta-chat/src/components/RunningElsewhereStrip.tsx-20-23 (1)
20-23: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReduce this block comment to one short line.
Keep only the non-obvious reason for the state. As per coding guidelines: “At most ONE short line per comment.”
Source: Coding guidelines
api/oss/tests/pytest/unit/sessions/test_orphan_sweep_thresholds.py-341-341 (1)
341-341: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the threshold test independent of environment variables.
ORPHAN_THRESHOLD_SECONDSandIDLE_THRESHOLD_SECONDSimport values fromSessionWatchdogConfig, whose fields evaluate the environment during import. The assertion can therefore fail when either variable is set. Clear or set both variables before importing the module, then assert the intended defaults.web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx-160-162 (1)
160-162: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReduce this to one short comment line.
The frontend rule limits each comment to one short line.
Source: Coding guidelines
api/oss/src/dbs/redis/sessions/locks.py-12-12 (1)
12-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unused import.
validate_session_idis unused, soruff checkreports F401. Remove it or use it.As per coding guidelines, run
ruff formatthenruff check --fixand fix all errors before committing.Sources: Coding guidelines, Linters/SAST tools
api/oss/src/utils/logging.py-212-215 (1)
212-215: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReduce the added explanatory comments. Keep only a short non-obvious rationale.
api/oss/src/utils/logging.py#L212-L215: replace the four-line comment with one short invariant comment, or remove it.api/oss/tests/pytest/unit/test_multilogger.py#L1-L9: reduce the module docstring to a short test-purpose statement.
As per coding guidelines: “Keep AI-generated in-code comments minimal. Comment only the non-obvious why.”Source: Coding guidelines
web/mobile/src/features/chat/LiveConversation.tsx-98-99 (1)
98-99: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep each new code comment to one short line.
web/mobile/src/features/chat/LiveConversation.tsx#L98-L99: reduce the prop comment to one short line.web/mobile/src/features/chat/LiveConversation.tsx#L1317-L1319: reduce the routing comment to one short line.As per coding guidelines, “At most ONE short line per comment.”
Source: Coding guidelines
docs/design/session-control-and-live-events/spike-a-sandbox-cancel.md-387-389 (1)
387-389: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDefine the Codex reap result for every outcome.
Step 4 requires
stage=harness_reap killed=..., but Step 9 allows failed or unknown reaping and only says to recordcleanup_miss=true. Define distinct reap records and the gate assertions for success, failure, and unknown outcomes. Otherwise, make the reap assertion conditional on successful cleanup.
🧹 Nitpick comments (4)
api/oss/src/core/sessions/commands/interfaces.py (1)
172-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign this docstring with the
expected_statesguard.
SessionCommandSettlecarriesexpected_states(default[claimed]) and admits a nullclaimed_byfor apendingrow.SessionCommandsService.report_outcomepasses[pending, claimed]. The docstring states a fixedstate='claimed' AND claimed_by=:replica_idguard, which no longer describes the contract.♻️ Proposed doc fix
- """Terminal transition, guarded on `state='claimed' AND claimed_by=:replica_id`. - None means the claim had expired or somebody else settled it first.""" + """Terminal transition, guarded on `state IN settle.expected_states` and, for a + claimed row, `claimed_by = settle.replica_id`. None means the guard did not match: + the claim had expired, or somebody else settled it first."""api/oss/tests/pytest/unit/sessions/test_session_commands_dao.py (1)
606-621: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound this concurrent settle with
asyncio.wait_for.Two other concurrency tests in this module wrap
asyncio.gatherwithasyncio.wait_for(..., timeout=30), and line 325 states the reason: contention on the same row must fail the run, not hang it.test_runner_and_watchdog_have_one_terminal_winnercontends on the same(project_id, session_id, execution_id)settlement without that bound. A regression that blocks instead of conflicting hangs CI rather than reporting a failure.♻️ Proposed change
- runner, watchdog = await asyncio.gather( - dao.settle( - project_id=command_scope["project_id"], - session_id=command_scope["session_id"], - execution_id="turn-A", - terminal_outcome="stopped", - settled_by="runner", - ), - dao.settle( - project_id=command_scope["project_id"], - session_id=command_scope["session_id"], - execution_id="turn-A", - terminal_outcome="lost", - settled_by="watchdog", - ), - ) + runner, watchdog = await asyncio.wait_for( + asyncio.gather( + dao.settle( + project_id=command_scope["project_id"], + session_id=command_scope["session_id"], + execution_id="turn-A", + terminal_outcome="stopped", + settled_by="runner", + ), + dao.settle( + project_id=command_scope["project_id"], + session_id=command_scope["session_id"], + execution_id="turn-A", + terminal_outcome="lost", + settled_by="watchdog", + ), + ), + timeout=30, + )api/oss/tests/pytest/unit/sessions/test_heartbeat_release_auth.py (1)
78-97: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBroken Authentication (CWE-287): Improper Authentication
Reachability: External
Add a wrong-token test for
release_owner.The tests cover absent and exact-match tokens, but not a mismatched token. Add a test that sends
"wrong-secret"and asserts401plusservice.heartbeat.assert_not_awaited().💚 Proposed test
`@pytest.mark.asyncio` async def test_release_owner_rejects_a_wrong_runner_token(monkeypatch): monkeypatch.setattr(env.runner, "token", "runner-secret") monkeypatch.setattr( router_module, "check_action_access", AsyncMock(return_value=True) ) service = SimpleNamespace(heartbeat=AsyncMock()) with pytest.raises(HTTPException) as exc_info: await _router(service).heartbeat_session_stream( _request({"X-Agenta-Runner-Token": "wrong-secret"}), SessionHeartbeatRequest( session_id="session-1", replica_id="replica-1", release_owner=True, ), ) assert exc_info.value.status_code == 401 service.heartbeat.assert_not_awaited()web/oss/src/components/AgentChatSlice/AgentConversation.tsx (1)
554-555: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep this code comment to one short line. Replace it with one concise line or remove it. As per coding guidelines: “At most ONE short line per comment.”
Proposed change
- // A rewrite of a held message: nothing is sent, so the transcript must not move. - // The input clears itself on submit, so the displaced draft goes back after that. + // Restore the displaced draft after the editor clears it.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 5ba59c97-d20a-4d19-abf8-d1ae491ceefa
⛔ 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 (205)
.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.pyapi/entrypoints/routers.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000022_add_session_commands.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000023_add_session_executions.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000024_add_execution_redis_reconciliation.pyapi/oss/databases/postgres/migrations/core_oss/versions/oss000000026_add_execution_ending_marker.pyapi/oss/databases/postgres/migrations/tracing_oss/versions/oss000000005_add_records_quarantined_at.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/executions/__init__.pyapi/oss/src/core/sessions/executions/dtos.pyapi/oss/src/core/sessions/executions/interfaces.pyapi/oss/src/core/sessions/interactions/interfaces.pyapi/oss/src/core/sessions/interactions/service.pyapi/oss/src/core/sessions/records/dtos.pyapi/oss/src/core/sessions/records/interfaces.pyapi/oss/src/core/sessions/records/service.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/interfaces.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/executions/__init__.pyapi/oss/src/dbs/postgres/sessions/executions/dao.pyapi/oss/src/dbs/postgres/sessions/executions/dbes.pyapi/oss/src/dbs/postgres/sessions/interactions/dao.pyapi/oss/src/dbs/postgres/sessions/records/dao.pyapi/oss/src/dbs/postgres/sessions/records/dbas.pyapi/oss/src/dbs/postgres/sessions/records/mappings.pyapi/oss/src/dbs/postgres/sessions/streams/dao.pyapi/oss/src/dbs/postgres/sessions/streams/dbes.pyapi/oss/src/dbs/postgres/sessions/streams/mappings.pyapi/oss/src/dbs/redis/sessions/contract.pyapi/oss/src/dbs/redis/sessions/locks.pyapi/oss/src/middlewares/auth.pyapi/oss/src/tasks/asyncio/sessions/orphan_sweep.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/tasks/asyncio/shared/consumer.pyapi/oss/src/utils/env.pyapi/oss/src/utils/logging.pyapi/oss/tests/pytest/unit/middlewares/test_auth_public_endpoints.pyapi/oss/tests/pytest/unit/sessions/test_cancel_cancels_pending_interactions.pyapi/oss/tests/pytest/unit/sessions/test_cancel_stop_guard.pyapi/oss/tests/pytest/unit/sessions/test_command_claim_unknown_kind.pyapi/oss/tests/pytest/unit/sessions/test_command_matrix_inputs_data.pyapi/oss/tests/pytest/unit/sessions/test_command_settle_unknown_kind.pyapi/oss/tests/pytest/unit/sessions/test_execution_watchdog.pyapi/oss/tests/pytest/unit/sessions/test_execution_watchdog_loop.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_departed_replica_affinity.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_lock_races.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_release_auth.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_handover.pyapi/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.pyapi/oss/tests/pytest/unit/sessions/test_late_record_quarantine.pyapi/oss/tests/pytest/unit/sessions/test_late_record_quarantine_dao.pyapi/oss/tests/pytest/unit/sessions/test_orphan_sweep_clears_redis.pyapi/oss/tests/pytest/unit/sessions/test_orphan_sweep_thresholds.pyapi/oss/tests/pytest/unit/sessions/test_owner_claim.pyapi/oss/tests/pytest/unit/sessions/test_project_scoped_locks.pyapi/oss/tests/pytest/unit/sessions/test_records_config.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_lifecycle_publish.pyapi/oss/tests/pytest/unit/sessions/test_watchdog_collapse_persistence.pyapi/oss/tests/pytest/unit/sessions/test_watchdog_lifespan_wiring.pyapi/oss/tests/pytest/unit/sessions/test_wp5_dao_fanout.pyapi/oss/tests/pytest/unit/test_multilogger.pydocs/design/session-control-and-live-events/api-design.mddocs/design/session-control-and-live-events/decisions.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-stop-guard.mddocs/design/session-control-and-live-events/slice-watchdog.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/docker-compose.dev.ymlhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/oss/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.dev.exampleservices/runner/src/engines/sandbox_agent/acp-fetch.tsservices/runner/src/engines/sandbox_agent/cancel-turn.tsservices/runner/src/engines/sandbox_agent/daytona.tsservices/runner/src/engines/sandbox_agent/engine.tsservices/runner/src/engines/sandbox_agent/environment.tsservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/engines/sandbox_agent/reap-exec.tsservices/runner/src/engines/sandbox_agent/run-limits.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/runtime-contracts.tsservices/runner/src/engines/sandbox_agent/sandbox-gone.tsservices/runner/src/engines/sandbox_agent/sandbox-liveness.tsservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/environment/mount-lifecycle.tsservices/runner/src/lifecycle/session-coordinator.tsservices/runner/src/server.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/turn-settle.tsservices/runner/tests/unit/cancel-continuity.test.tsservices/runner/tests/unit/control-command-apply.test.tsservices/runner/tests/unit/harness-cancel-park.test.tsservices/runner/tests/unit/mount-lifecycle.test.tsservices/runner/tests/unit/reap-exec.test.tsservices/runner/tests/unit/sandbox-agent-acp-fetch.test.tsservices/runner/tests/unit/sandbox-agent-orchestration.test.tsservices/runner/tests/unit/sandbox-gone.test.tsservices/runner/tests/unit/sandbox-liveness.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-approval.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/turn-settle.test.tsweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/StopButton.tsxweb/mobile/src/features/chat/stopHereState.tsweb/mobile/src/features/chat/useSessionWatch.tsweb/mobile/src/features/sessions/useActionableInteractions.tsweb/mobile/src/features/sessions/useLivenessPoll.tsweb/mobile/tests/unit/stopHereState.test.tsweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/refusedMessageRecovery.test.tsweb/oss/src/components/AgentChatSlice/assets/refusedMessageRecovery.tsweb/oss/src/components/AgentChatSlice/assets/stopState.test.tsweb/oss/src/components/AgentChatSlice/assets/stopState.tsweb/oss/src/components/AgentChatSlice/assets/stopWhileResolvingExecution.test.tsweb/oss/src/components/AgentChatSlice/assets/stopWhileResolvingExecution.tsweb/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.test.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionRecordsWatch.tsweb/oss/src/components/AgentChatSlice/state/liveness.tsweb/packages/agenta-chat/src/assets/agentTurn.tsweb/packages/agenta-chat/src/assets/composerState.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/components/ChatComposer.tsxweb/packages/agenta-chat/src/components/RunningElsewhereStrip.tsxweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/hooks/useComposerAttachments.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/interactionAvailability.tsweb/packages/agenta-chat/src/model/userStop.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/tests/unit/assets/agentTurn.test.tsweb/packages/agenta-chat/tests/unit/assets/composerState.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/interactionAvailability.test.tsweb/packages/agenta-chat/tests/unit/model/liveApprovals.test.tsweb/packages/agenta-chat/tests/unit/model/userStop.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/tests/unit/session-cancel-api.test.tsweb/packages/agenta-entities/tests/unit/session-cancel-stream.test.tsweb/packages/agenta-entities/tests/unit/session-liveness.test.tsweb/packages/agenta-entities/tests/unit/session-query-schema.test.tsweb/packages/agenta-navigation/src/dynamic/sessionsSource.tsweb/packages/agenta-navigation/tests/unit/sidebarChildren.test.tsweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- web/packages/agenta-chat/tests/unit/model/error.test.ts
- docs/design/session-control-and-live-events/rfc.md
- web/packages/agenta-chat/src/model/error.ts
- docs/design/session-control-and-live-events/research.md
- docs/design/session-control-and-live-events/decisions.md
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Expose the fixture-generated database name to the concurrency test. Connect the raw sweep and observer clients to that isolated database. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
[fix] Point the watchdog collapse test at its own database
1 participant
Context
Today a Stop in an agent session destroys the warm sandbox and the native session, a second message during a running turn can kill both turns, and a runner that dies leaves the session marked running with no record of what happened. This branch collects the first milestone of the session-control design in PR #6495: increments 1 to 3, the Stop package. It is cut from
release/v0.114.8so that its diff shows only this work, and it merges intorelease/v0.115.0when that branch exists.What this branch contains, in merge order (all merged, head
86201077e4)AGENTA_SESSIONS_DURABLE_STOP.Every new behaviour ships dark.
AGENTA_SESSIONS_DURABLE_STOPandAGENTA_SESSIONS_LATE_OUTPUTdefault to off. Migrations 022, 023, 024 and 026 on the core database and 005 on tracing are additive and nullable with downgrades. The history producer (#6517) is out of scope and is not part of this branch; the chain skips 025.Known consequence of the flag
With
AGENTA_SESSIONS_DURABLE_STOPoff (the production default at merge time), a Stop pressed on an approval card does not cancel the pending approval, so a late answer given a few seconds after the Stop is still accepted. That is today's behaviour; the durable path cancels it. The flag-off matrix run of 4 September recorded it.Tests
~/agenta-qa-evidence/.e65e6246c7(5 September, driver43e29546c6), no unexpected failure: Pi local 16 of 16; Codex local 15 plus 1 n/a; Claude Code local 13 plus 2 n/a plus the five re-run cells 5 of 5 (all five concurrent Stops resumed in the same sandbox); Pi Daytona 14 plus 1 n/a plus 1 known limitation; Codex Daytona 7 plus 1 n/a; flag off 9 of 9; flip back 2 of 2; Daytona continuity 2 of 2 by sandbox id. Run folders under~/agenta-qa-evidence/2026-09-05-final-matrix/.release/v0.115.0once that branch exists.What to QA
Release readiness page: https://claude.ai/code/artifact/62f080fc-6be0-457d-aed5-a566443d8f20
Agent-generated, low weight. Nothing here merges to main or a release branch on its own.
https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk