[feat] Session control milestone 3: durable approvals, queue, steer - #6575
Conversation
Superseded by the milestone 2 generation-safe watchdog fence. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Superseded by the milestone 2 atomic generation-safe watchdog release, which commits database state before Redis cleanup. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Superseded with the removed heartbeat guard by the milestone 2 generation-safe watchdog implementation. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Symptom: a user approves a tool call, POST /sessions/interactions/<id>/respond returns 202, and the turn never continues. The API log repeats "control delivery unreachable ... Workflow revision has no runnable service URL." three times over six minutes, then the command settles obsolete and the card sits on "Answered, waiting for the agent". Cause: a durable continuation is a server-side invoke, and an invoke finds its service URL only through the request's references. _ensure_request_revision returns at once when the request carries neither data.revision nor references, so _get_service_url gets no revision and returns None. The dispatcher took the references from the gate row alone. A session whose first Send carried no references stamps none on the gate row, so the invoke had no URL and every redelivery failed. Fix: InteractionsDispatcher._session_references resolves the identity from the gate row first, then from the session's own session_turns.references, then from session_streams.references. The new keyed_references helper folds the stored flat list back into the keyed map an invoke carries, and drops any family _validate_execution_reference_families would reject. Both reads are best effort and read-only: a read that fails logs and falls through, because the continuation is already durable. routers.py passes the existing turns and streams services in. Five tests cover the turn fallback, the stream fallback, the gate row winning over both, a session with no identity anywhere, and the helper's family filter. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Symptom: after an answer the approval card always showed "Answered, waiting for
the agent", even when the server had marked the continuation recoverable. The
user never saw "Answer saved, retry needed", so the one state that asks for an
action stayed invisible.
Cause: handleApprovalResponse returned answerApproval(...).then(() => { ... }).
The arrow body returned nothing, so the promise resolved to undefined and the
dock's result.value?.recoverable was always false.
Fix: the ordered click is now the extracted answerThenSteer helper. It answers the
gate, sends a denial's steer note after the answer as before, and returns the
submission outcome to the caller. Four tests pin the return value, the steer
ordering, the approve-side suppression, and the blank-note case.
Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
✅ Action performedReview finished.
|
|
@coderabbitai full review |
Probe session capabilities in the background with a two-second transport deadline and no retries. Cache both supported and unsupported results so legacy sends and approvals never wait on or repeat a failed probe. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Only advertise composer Stop and Escape for server-owned runs when queue control is available. Local legacy streams remain stoppable, while mobile keeps one strip Stop for a remote flag-off run. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Negotiate queue support before loading the unified snapshot, share concurrent mount refreshes, and refresh only after a real transition into a settled chat state. Flag-off sessions now make no queue snapshot request. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
web/oss/src/components/AgentChatSlice/assets/answerThenSteer.ts-31-31 (1)
31-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAwait the steer submission.
submit({text})is asynchronous. If it rejects, this helper returns a successful approval outcome and bypassesApprovalDockerror handling. Propagate the rejection so the user can retry the failed steer.Proposed fix
- steer: (text: string) => void + steer: (text: string) => void | Promise<void> ... - if (!approved && note) steer(note) + if (!approved && note) await steer(note)web/oss/src/components/AgentChatSlice/components/ApprovalDock.tsx-94-94 (1)
94-94: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNarrow the fulfilled value before reading
recoverable.Promise.allSettledcan producevoid | ApprovalSubmissionOutcomehere, so strict TypeScript can rejectresult.value?.recoverable. Check thatresult.valueis an object before the property access.web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts-67-68 (1)
67-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep each new code comment to one short line.
web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts#L67-L68: combine the polling rationale into one short line.web/packages/agenta-chat/tests/unit/hooks/useServerSessionInputs.test.ts#L142-L143: shorten or remove the test-harness explanation.As per coding guidelines: “Hard rule. At most ONE short line per comment.”
Source: Coding guidelines
web/packages/agenta-chat/src/hooks/useServerSessionInputs.ts-50-52 (1)
50-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove snapshot loading to a session-keyed
atomWithQuery.
fetchSessionSnapshotAtomperforms an imperative fetch, andrefresh()applies each result without ordering protection. An older poll can therefore resolve after a removal or admission refresh and restore stale pending inputs until the next poll. TheuseEffectplus local-state fetch also violates the web convention that data fetching usesatomWithQuery.Source: Coding guidelines
web/packages/agenta-chat/src/components/ApprovalCard.tsx-54-55 (1)
54-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGate the keyboard shortcuts on
answered.The window
keydownhandler callsapprove()/deny(), and both only return early whenrespondingis true. The mobile host setsresponding={actions.phase === "resuming"}andanswered={actions.phase === "answered" || actions.phase === "recoverable"}(web/mobile/src/features/chat/ApprovalDock.tsx), so after the durable answer is acceptedrespondingis false whileansweredis true. In that state the buttons are collapsed and inert, but Cmd/Ctrl+Enter or Escape still firesonRespondagain for the sameapprovalId.Add
answeredto the guards.🐛 Proposed fix
const approve = () => { - if (responding) return + if (responding || answered) return setFiredAction("approve") if (alwaysAllowArmed && canAlwaysAllow) grantMany(grantableTools) if (batched) return onApproveAll(approvals.map((a) => a.approvalId)) onRespond({approvalId: current.approvalId, approved: true}) } const deny = () => { - if (responding) return + if (responding || answered) return setFiredAction("deny")web/packages/agenta-entities/src/session/api/api.ts-290-292 (1)
290-292: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire one answer or a non-empty
answersbatch; the current optional fields allow an invalid response request with no answer.api/oss/src/dbs/postgres/sessions/records/dao.py-372-388 (1)
372-388: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore the replay watermark when the cursor row is missing.
RecordsDAO.get_records_afteruses0whenSessionSequenceCursorDBEhas no row. Withafter=0, theRecordDBE.sequencefilter becomessequence IS NULL OR sequence BETWEEN(1, 0), so existing sequenced records are omitted. The live-event relay preserves the requested cursor withmax(cursor, result.watermark), so this path does not move the client cursor backward. Derive a fallback watermark from sequenced records or recreate the missing cursor.api/oss/databases/postgres/migrations/core_oss/versions/oss000000027_add_durable_interaction_continuations.py-73-76 (1)
73-76: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winBoth downgrades recreate a narrower
ck_session_commands_kindwithout clearing the rows it forbids. PostgreSQL validates a new check constraint against existing rows, so a rollback aborts once the feature has written command rows of the newly allowed kinds.
api/oss/databases/postgres/migrations/core_oss/versions/oss000000027_add_durable_interaction_continuations.py#L73-L76: delete or remapsession_commandsrows whosekindis notcancelbefore you recreate the constraint.api/oss/databases/postgres/migrations/core_oss/versions/oss000000028_add_session_pending_inputs.py#L88-L93: delete or remap rows whosekindiscontinue_inputbefore you recreate the constraint.api/oss/src/dbs/postgres/sessions/executions/dao.py-148-148 (1)
148-148: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
set_stateerases a storederroron every transition.
errordefaults toNoneand is always included invalues(...). A later transition that does not pass an error clears the recorded failure reason. This happens on the recovery path: a failed continuation delivery stores an error, then redelivery callsset_state(state=running)and wipes it.Write
erroronly when the caller supplies it.🐛 Proposed fix
- row = ( - await session.execute( - stmt.values(state=state.value, error=error).returning( - SessionExecutionDBE - ) - ) - ).scalar_one_or_none() + values: dict = {"state": state.value} + if error is not None: + values["error"] = error + row = ( + await session.execute( + stmt.values(**values).returning(SessionExecutionDBE) + ) + ).scalar_one_or_none()If clearing on success is intended, add an explicit
clear_error: bool = Falseparameter so the intent is visible at each call site.api/oss/src/tasks/asyncio/sessions/records_worker.py-411-418 (1)
411-418: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPublish durable events after the gate reconciliation, not before.
The comment at Line 420 states the ordering invariant for this method: a notification that wakes a client must be emitted only after
reconcile_orphaned_gateshas cancelled the orphaned gates, so the woken client does not re-render a gate that is already dead.publish_durable_eventis exactly such a notification, and it now runs before that call. A client that revalidates on the durable event can re-render an approval the same batch is about to cancel.Move the per-session publish loop below the
reconcile_orphaned_gatescall.docs/design/session-control-and-live-events/live-frame-envelope.md-76-79 (1)
76-79: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDocument the live-stream count limit as approximate.
streams:session-live-framesuses approximateMAXLEN ~andMINID ~trimming. Redis can temporarily retain more than 100,000 frames. Replace “trimmed exactly” inlive-frame-envelope.mdso it matchescontracts/events.md,decisions.md, and the implementation.services/runner/src/sessions/control-channel.ts-335-339 (1)
335-339: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Internal · Exploitability: Difficult
Set
redirect: "error"on the admission report.This request carries the
x-agenta-runner-tokenheader. Preventfetchfrom following redirects on this authenticated request, asreportOutcomealready does.🔒️ Proposed fix
const res = await fetch(url, { method: "POST", + redirect: "error", signal: AbortSignal.timeout( envTimerMs("AGENTA_RUNNER_CONTROL_OUTCOME_TIMEOUT_MS", 5_000), ),services/runner/src/tracing/otel.ts-2094-2094 (1)
2094-2094: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle
stopReason: "error"as a failed turn
transcriptToMessagestreats this value as a normal terminal record. It setsrecordTerminalbut does not set failure metadata.userStop.tsalso sees neitherrunStoppednor a failed turn. Becauserun-turn.tscan pass this reason torun.finish()without emitting anerrorevent, hydration can show a generic no-response turn instead of an error. Persist anerrorevent for this path, or map this stop reason to failure metadata and add coverage.web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts-284-298 (1)
284-298: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake the in-flight test enqueue locally before the server appears.
useAgentChatQueue.submitcallsserver.submitimmediately whenpaused.serveris set, so the test never creates a local queue head. The migration effect therefore has no input, and the later assertions cannot detect a regression in the migration hold.💚 Proposed fix
const paused: HarnessProps = { status: "ready", messages: [userTurn("u1", "go"), assistantAwaitingApproval("a1")], stopped: false, - server, } const {result, rerender, sendQueued} = setup(paused) act(() => void result.current.submit({text: "held by this tab"})) + expect(result.current.queued).toHaveLength(1) rerender({ ...paused, + server, continuationExecutionId: "a1-continuation-execution", messages: [userTurn("u1", "go"), assistantContinuation("a1", "running")], })
🧹 Nitpick comments (8)
web/packages/agenta-chat/src/hooks/useAgentConversation.ts (1)
978-979: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
revalidatewith the signature it now implements.
revalidateaccepts an optionalSessionTranscriptand returnsPromise<boolean>, butAgentConversation.revalidateis still declared as() => void(Line 216). TypeScript accepts the assignment, so callers lose the transcript parameter and the adoption result. The unit test has to cast throughas unknown as (transcript: unknown) => Promise<boolean>.Update the interface member to
revalidate: (transcript?: SessionTranscript) => Promise<boolean>so consumers such asonDisconnectand the desktop hooks can use the real contract without a cast.api/oss/databases/postgres/migrations/tracing_oss/versions/oss000000006_add_session_sequence_cursors.py (1)
45-50: 🚀 Performance & Scalability | 🔵 TrivialConsider a concurrent index build on
records.
op.create_indexwithoutpostgresql_concurrentlytakes anACCESS EXCLUSIVElock onrecordsfor the full build.recordsis the tracing hot path, so writes stall for the build duration on a large deployment. A concurrent build needsautocommit_block()and a retry path for anINVALIDindex, so treat this as a deployment decision rather than a code defect.♻️ Optional concurrent variant
- op.create_index( - "ux_records_session_id_sequence", - "records", - ["project_id", "session_id", "sequence"], - unique=True, - ) + with op.get_context().autocommit_block(): + op.create_index( + "ux_records_session_id_sequence", + "records", + ["project_id", "session_id", "sequence"], + unique=True, + postgresql_concurrently=True, + )sdks/python/agenta/sdk/decorators/routing.py (1)
288-293: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the success-path
response.json()like the error path.Line 289 parses the 200 body without protection. A malformed success body raises
ValueError, which the endpoint converts into a generic 500 invoke failure. The non-200 path at Line 295 already handles this case.♻️ Proposed fix
if response.status_code == 200: - body = response.json() - execution_id = body.get("execution_id") + try: + body = response.json() + except ValueError: + body = {} + execution_id = body.get("execution_id") if isinstance(body, dict) else None if isinstance(execution_id, str) and execution_id: request.meta = {**(request.meta or {}), "run_id": execution_id} return Noneweb/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (2)
248-256: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueClear
migrationRefonly when the backoff ends.The catch handler sets
migrationRef.current = nullimmediately, then waits 2 seconds before it triggers a retry render. During that window the effect guard is open, so any re-render caused by its dependencies (for example a newqueuedarray identity) re-submits the same head before the backoff elapses. Keep the ref set tohead.iduntil the timer fires.♻️ Proposed change
.catch(() => { if (migrationRef.current !== head.id) return - migrationRef.current = null migrationRetryTimerRef.current = setTimeout(() => { migrationRetryTimerRef.current = null migrationRef.current = null setMigrationRetry((attempt) => attempt + 1) }, 2_000) }) .finally(() => { - if (migrationRef.current === head.id) migrationRef.current = null + // Success path only: the catch path keeps the ref until its backoff timer fires. + if (!migrationRetryTimerRef.current && migrationRef.current === head.id) { + migrationRef.current = null + } })
303-306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSurface a failed durable removal.
removeQueueddiscards the error fromserver.remove. The durable row stays in the queue and the user gets no feedback, so the click looks lost. Propagate or report the failure.web/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.ts (1)
144-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAwait the
steerpromise insideact.
steerreturns a promise that rejects when the server refuses. This call leaves it unhandled, so a rejection surfaces as an unhandled rejection warning rather than a test failure. Await both calls.💚 Proposed fix
await act(async () => { - result.current.submit({text: "wait next"}) - result.current.steer({text: "change direction"}) + await result.current.submit({text: "wait next"}) + await result.current.steer({text: "change direction"}) })web/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx (1)
91-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a Tailwind arbitrary-value utility for the spinner arc. This static
color-mix(...)value can replacestyle={{borderTopColor: ...}}, which conflicts with the styling guidance.services/runner/src/protocol.ts (1)
772-783: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a populated session-owned continuation fixture to the
/rungolden contract.Existing serializer checks and the TypeScript key guard cover the declared fields, so this omission does not create a current wire incompatibility. The shared golden comparison still never validates populated
detached,projectId, andcontrolCommandIdvalues across languages.
🔇 Additional comments (171)
web/mobile/src/features/chat/TurnRow.tsx (1)
45-45: LGTM!Also applies to: 362-365
web/mobile/src/features/chat/continuationRetry.ts (1)
1-10: LGTM!web/mobile/src/features/chat/transcriptAdoption.ts (1)
1-10: LGTM!Also applies to: 25-49
web/mobile/src/features/chat/useSessionTranscript.ts (1)
3-21: LGTM!Also applies to: 51-52, 64-70, 92-110, 125-134, 145-189
web/mobile/tests/unit/continuationRetry.test.ts (1)
1-21: LGTM!web/mobile/tests/unit/turnStatus.test.ts (1)
3-102: LGTM!web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.test.ts (1)
10-11: LGTM!Also applies to: 31-39, 68-72, 86-97, 180-181
web/oss/src/components/AgentChatSlice/hooks/useSessionHydration.test.ts (1)
112-120: LGTM!web/oss/src/components/AgentChatSlice/components/ApprovalDock.test.tsx (1)
106-106: 📐 Maintainability & Code QualityThe file contains only one
let resolveFirstdeclaration at line 106. The duplicate shown in the snippet is not present.web/oss/src/components/AgentChatSlice/hooks/useSessionRecordsWatch.ts (1)
49-60: 🩺 Stability & AvailabilityNo stale handler closure exists.
useWatchEventSourceassigns the latestonmap toonRef.currenton each render. Event handlers invokeonRef.current[eventName], sorefreshLegacyObserverLivenessuses the currentsharedReaderAdvertisedvalue.web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts (1)
335-341: 🎯 Functional CorrectnessResolve the
AgentRunErrorBoundaryshape before changingerrorBoundary.
classifyAgentRunErrorreturnsAgentRunErrorBoundary, but the available source does not show whether{}is assignable to that type or whether the later property reads are valid. The type definition and compiler result are required to decide this finding.web/packages/agenta-chat/src/assets/continuationPreflight.ts (1)
7-38: LGTM!web/packages/agenta-chat/src/assets/index.ts (1)
13-15: LGTM!web/packages/agenta-chat/src/assets/serverOwnedApproval.ts (1)
8-49: LGTM!web/packages/agenta-chat/src/components/ConnectionWarningStrip.tsx (1)
5-26: LGTM!web/packages/agenta-chat/src/model/error.ts (1)
85-94: LGTM!Also applies to: 104-133
web/packages/agenta-chat/src/model/index.ts (1)
16-17: LGTM!web/packages/agenta-chat/src/state/sessionEphemera.ts (1)
39-47: LGTM!Also applies to: 58-59
web/packages/agenta-chat/src/transport/AgentChatTransport.ts (1)
49-111: LGTM!Also applies to: 318-389, 398-441
web/packages/agenta-chat/tests/unit/assets/__fixtures__/heldMessageDuringContinuation.records.json (1)
1-252: LGTM!web/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.ts (1)
193-218: LGTM!Also applies to: 748-786
web/packages/agenta-chat/tests/unit/transport/AgentChatTransport.test.ts (1)
199-261: LGTM!Also applies to: 263-344, 346-434
web/packages/agenta-chat/src/hooks/useAgentConversation.ts (1)
384-391: 🩺 Stability & AvailabilityNo change needed. The runner emits
session-acceptedonly whenrequest.detached === true, anddetachedselects shared-session delivery. A legacy invoke cannot producedata-session-accepted, soacceptedRunPendingis not set on the legacy path.web/packages/agenta-chat/src/assets/loadSession.ts (1)
56-73: LGTM!Also applies to: 86-113, 127-132
web/packages/agenta-chat/src/assets/transcriptToMessages.ts (2)
218-243: LGTM!Also applies to: 254-292, 294-318
610-656: LGTM!Also applies to: 671-750, 766-767
web/packages/agenta-chat/src/components/ApprovalCard.tsx (1)
206-212: LGTM!Also applies to: 289-289, 353-353, 400-411
web/packages/agenta-chat/src/hooks/useApprovalDock.ts (1)
79-126: LGTM!Also applies to: 139-161
web/packages/agenta-chat/src/model/approvals.ts (1)
51-79: LGTM!web/packages/agenta-chat/tests/unit/ApprovalCard.test.tsx (1)
103-152: LGTM!web/packages/agenta-chat/tests/unit/assets/loadSession.test.ts (1)
25-57: LGTM!Also applies to: 83-96, 112-126, 135-137, 147-171
web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts (2)
261-406: LGTM!Also applies to: 1045-1054, 1075-1112
19-34: 🗄️ Data Integrity & IntegrationNo change is required for
record.SessionRecordis inferred fromsessionRecordSchema, wheresequenceis declared withz.number().int().positive().nullish(). The factory may omitsequence.web/packages/agenta-chat/tests/unit/hooks/useApprovalDock.test.ts (1)
108-125: LGTM!Also applies to: 134-151, 153-170, 172-191, 193-206, 208-218, 220-241
web/packages/agenta-chat/tests/unit/model/approvalDockRetirement.test.ts (1)
24-45: LGTM!web/packages/agenta-chat/src/hooks/useSessionLivePreview.ts (1)
84-278: LGTM!web/packages/agenta-chat/src/model/durableEvents.ts (1)
33-55: LGTM!web/packages/agenta-chat/src/transport/index.ts (1)
5-5: LGTM!web/packages/agenta-chat/src/transport/sessionLiveEvents.ts (1)
23-90: LGTM!web/packages/agenta-chat/tests/unit/QueuedMessagesDock.test.tsx (1)
7-18: LGTM!web/packages/agenta-chat/tests/unit/assets/__fixtures__/approvalDockRetirement.records.json (1)
1-231: LGTM!web/packages/agenta-chat/tests/unit/assets/continuationPreflight.test.ts (1)
9-56: LGTM!web/packages/agenta-chat/tests/unit/hooks/useSessionLivePreview.test.ts (1)
88-222: LGTM!web/packages/agenta-chat/tests/unit/hooks/useSessionLivePreview.test.tsx (1)
68-469: LGTM!api/entrypoints/worker_streams.py (1)
30-45: LGTM!Also applies to: 92-97, 115-120, 156-169, 201-213
api/oss/src/core/sessions/records/dtos.py (1)
2-13: LGTM!Also applies to: 53-72, 124-141, 199-211, 220-220, 256-271
api/oss/src/core/sessions/records/interfaces.py (1)
8-10: LGTM!Also applies to: 38-64, 96-103
api/oss/src/dbs/postgres/sessions/records/dao.py (1)
4-30: LGTM!Also applies to: 59-63, 72-118, 134-152, 261-275, 277-313, 315-362, 526-556
api/oss/src/dbs/postgres/sessions/records/dbes.py (1)
1-19: LGTM!Also applies to: 50-56
api/oss/src/dbs/postgres/sessions/records/mappings.py (1)
21-21: LGTM!Also applies to: 38-38
api/oss/tests/pytest/unit/sessions/test_late_record_quarantine_dao.py (1)
161-184: LGTM!api/oss/tests/pytest/unit/sessions/test_records_worker_batching.py (1)
12-13: LGTM!Also applies to: 123-153, 155-208, 211-257
api/oss/tests/pytest/integration/sessions/test_records_replay_postgres.py (1)
189-205: 🎯 Functional CorrectnessNo change needed.
AnalyticsEngine.session()commits after the context block exits, so the seeded rows remain visible to later sessions.api/oss/tests/pytest/integration/sessions/test_records_sequence_postgres.py (1)
21-22: 🎯 Functional CorrectnessKeep the existing fixture decorator.
api/pytest.inisetsasyncio_mode = auto, so the strict-mode failure described here does not apply. The explicit decorator may match the sibling test, but it is not required for this fixture to run.api/oss/src/apis/fastapi/sessions/live_events.py (1)
48-188: LGTM!api/oss/src/apis/fastapi/sessions/models.py (1)
126-153: LGTM!Also applies to: 186-238, 313-334, 448-496, 538-609
api/oss/src/apis/fastapi/sessions/router.py (3)
906-934: LGTM!Also applies to: 973-1061, 1351-1459, 2171-2247, 2305-2345, 2681-2684, 2713-2737
2621-2628: 🩺 Stability & AvailabilityNo change needed.
request_canceldeclaressteer_input_id: Optional[UUID] = None, so this call does not raise aTypeErrorfor an unexpected keyword.
2612-2617: 📐 Maintainability & Code QualityNo change needed.
SessionInputsService.admitrejectspolicy == "steer"whenenv.agenta.sessions.queueorenv.agenta.sessions.steeris disabled, so the handler cannot reachrequest_cancelthrough this path.api/oss/src/apis/fastapi/sessions/watch.py (1)
4-7: LGTM!api/oss/src/core/sessions/commands/types.py (1)
54-69: LGTM!api/oss/src/core/sessions/interactions/service.py (1)
29-47: LGTM!Also applies to: 62-83, 143-164, 238-250
api/oss/tests/pytest/unit/sessions/test_record_ingest_endpoint.py (1)
148-194: LGTM!Also applies to: 237-274
api/oss/src/apis/fastapi/sessions/utils.py (1)
273-275: 🩺 Stability & AvailabilityNo change needed.
SessionStream.capabilitiesis a non-optionalSessionCapabilitiesfield with a default value. The database mapping omits the field, so Pydantic supplies that default beforemodel_copy()runs.api/oss/src/core/sessions/commands/dtos.py (1)
27-28: LGTM!Also applies to: 50-50
api/oss/src/core/sessions/commands/interfaces.py (1)
45-50: LGTM!Also applies to: 86-100, 113-134, 136-155, 157-163
api/oss/src/core/sessions/executions/dtos.py (1)
2-16: LGTM!Also applies to: 18-28
api/oss/src/core/sessions/interactions/interfaces.py (1)
32-45: LGTM!Also applies to: 47-52
api/oss/src/core/sessions/watch/interfaces.py (1)
1-1: LGTM!Also applies to: 25-32
api/oss/src/dbs/redis/sessions/watch.py (1)
14-14: LGTM!Also applies to: 99-107
api/oss/tests/pytest/unit/sessions/test_interaction_cancel_records.py (1)
22-25: LGTM!api/oss/tests/pytest/unit/sessions/test_interaction_continuation_admission.py (1)
1-1498: LGTM!api/oss/tests/pytest/unit/sessions/test_watch_interactions_publish.py (1)
18-26: LGTM!Also applies to: 33-44, 82-89, 92-103, 115-122, 161-161
api/oss/tests/pytest/unit/sessions/test_watch_publish.py (1)
185-222: LGTM!api/oss/src/core/sessions/commands/service.py (1)
1033-1044: 🗄️ Data Integrity & IntegrationDo not add
datato the transactional create call. The executions-enabled Stop path callsbind_steer_inputwith the same transaction after_insert. That method updates the commanddatawithsteer_input_id, whichsettlecan then read. The proposed fix is unnecessary.api/oss/src/core/sessions/executions/interfaces.py (1)
14-59: LGTM!api/oss/src/dbs/http/sessions/control_delivery_direct.py (1)
71-94: LGTM!api/oss/src/dbs/postgres/sessions/commands/dao.py (1)
100-149: LGTM!Also applies to: 632-643
api/oss/src/dbs/postgres/sessions/executions/dao.py (1)
67-98: LGTM!Also applies to: 160-199, 228-228
api/oss/src/dbs/postgres/sessions/executions/dbes.py (1)
22-28: LGTM!Also applies to: 35-41
api/oss/tests/pytest/unit/sessions/test_session_cancel_admission.py (1)
152-163: LGTM!Also applies to: 908-933
api/oss/tests/pytest/unit/sessions/test_session_commands_dao.py (2)
207-231: LGTM!Also applies to: 771-808, 947-1027, 1283-1363
609-612: 📐 Maintainability & Code QualityNo change needed.
map_command_dto_to_dbe_createdoes not setupdated_at.LifecycleDBA.updated_atis nullable and has no default. For this pending command,claim_expires_atandupdated_atare null, soexpire_claimsorders it by the supplied 1970created_at.api/oss/tests/pytest/unit/sessions/test_session_inputs_dao.py (1)
42-116: LGTM!Also applies to: 595-642, 710-839
api/oss/tests/pytest/unit/sessions/test_session_snapshot.py (1)
29-78: LGTM!Also applies to: 213-285, 288-324
api/oss/tests/pytest/unit/sessions/test_session_steer_admission.py (1)
51-84: LGTM!api/oss/src/dbs/postgres/sessions/commands/dbes.py (1)
28-31: 🗄️ Data Integrity & IntegrationNo migration change is needed.
oss000000028_add_session_pending_inputs.pyalready recreatesck_session_commands_kindwithcancel,continue_interaction, andcontinue_input.api/oss/src/core/sessions/interactions/references.py (1)
31-95: LGTM!api/oss/src/core/sessions/streams/service.py (1)
556-573: LGTM!api/oss/src/dbs/redis/sessions/contract.py (1)
163-185: LGTM!api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.py (1)
266-316: LGTM!api/oss/src/tasks/asyncio/sessions/records_worker.py (1)
220-260: LGTM!api/oss/tests/pytest/unit/sessions/test_heartbeat_lock_races.py (1)
95-123: LGTM!api/oss/tests/pytest/unit/sessions/test_interactions_dispatcher.py (1)
230-263: LGTM!Also applies to: 818-986
api/oss/tests/pytest/unit/sessions/test_respond_interaction_durable.py (1)
24-397: LGTM!api/oss/tests/pytest/unit/sessions/test_respond_interaction_enqueue.py (1)
220-263: LGTM!api/oss/src/tasks/asyncio/sessions/live_relay_worker.py (1)
39-42: 🎯 Functional CorrectnessKeep
env.sessions.live_frame_max_age_seconds.live_frame_max_age_secondsis defined in the top-levelSessionsRedisConfig, whileenv.agenta.sessionscontains different settings. The access is valid, and the surrounding per-messageexceptdoes not handle errors from the precedingcutoffcalculation.</verification_refuted>
api/oss/src/core/sessions/records/service.py (1)
82-85: LGTM!Also applies to: 120-182, 368-416, 448-458
api/oss/src/core/workflows/service.py (1)
280-304: LGTM!Also applies to: 755-769, 811-826, 842-883, 2965-2977, 3016-3016, 3034-3035, 3055-3055
api/oss/src/dbs/postgres/sessions/interactions/dao.py (2)
80-97: LGTM!Also applies to: 99-126
173-176: 🗄️ Data Integrity & IntegrationNo explicit commit is needed in this branch.
TransactionsEngine.session()commits after theasync withbody returns, soexecute(session)is committed when the context exits.api/oss/src/dbs/postgres/sessions/records/dbas.py (1)
3-3: LGTM!Also applies to: 40-44
api/oss/src/tasks/asyncio/sessions/orphan_sweep.py (1)
311-372: LGTM!Also applies to: 536-561, 597-607
api/oss/tests/pytest/unit/sessions/test_command_settle_unknown_kind.py (1)
4-6: LGTM!Also applies to: 66-66, 85-86, 96-96
api/oss/tests/pytest/unit/sessions/test_late_record_quarantine.py (1)
22-23: LGTM!Also applies to: 123-134, 149-153, 354-439
api/oss/tests/pytest/unit/sessions/test_live_frame_ingest.py (1)
73-106: LGTM!Also applies to: 108-146, 148-174, 176-179, 181-214, 216-238, 240-268, 270-300, 302-321, 323-341, 343-364, 366-378
api/oss/tests/pytest/unit/sessions/test_live_relay.py (1)
82-109: LGTM!Also applies to: 111-130, 132-151, 153-194, 196-217, 219-245, 247-275, 277-329, 331-356, 358-380, 382-411
api/oss/tests/pytest/unit/sessions/test_orphan_sweep_clears_redis.py (1)
88-88: LGTM!api/oss/tests/pytest/unit/sessions/test_orphan_sweep_thresholds.py (1)
143-145: LGTM!Also applies to: 166-169, 190-205, 259-272, 295-325, 354-456
STATUS-round7.md (1)
1-49: LGTM!api/ee/src/middlewares/throttling.py (2)
9-9: LGTM!Also applies to: 44-51, 180-181
182-183: 🔒 Security & PrivacyAuthorization Bypass (CWE-863): Incorrect Authorization
⚠️ Unverified finding
Verification did not complete.Verify that
SECRET_RESOLVE_GRANTis runner-only before bypassing throttling.This branch skips plan lookup and throttle checks for any
POST /sessions/records/ingestrequest carrying the grant. The supplied middleware tests prove branch selection, but they do not prove who can obtain the grant or that the ingest route rejects non-runner callers. If a user-controlled credential can carry this grant, that caller can send unthrottled ingest traffic.api/ee/tests/pytest/unit/test_throttling.py (1)
1-101: LGTM!hosting/docker-compose/oss/docker-compose.gh.yml (1)
376-376: LGTM!hosting/docker-compose/oss/env.oss.dev.example (1)
149-157: LGTM!hosting/docker-compose/oss/env.oss.gh.example (1)
149-157: LGTM!hosting/kubernetes/helm/templates/runner-deployment.yaml (1)
89-92: LGTM!hosting/kubernetes/helm/values.schema.json (1)
310-310: LGTM!hosting/kubernetes/helm/values.yaml (1)
141-141: LGTM!hosting/railway/oss/scripts/configure.sh (1)
514-515: LGTM!hosting/railway/oss/template/template.json (1)
296-297: LGTM!.gitleaksignore (1)
161-162: 🔒 Security & PrivacyThe referenced matches are OTLP endpoint fixtures, not API keys.
docs/design/session-control-and-live-events/contracts/commands.md (1)
1-92: LGTM!docs/design/session-control-and-live-events/contracts/events.md (1)
1-179: LGTM!docs/design/session-control-and-live-events/decisions.md (1)
44-49: LGTM!Also applies to: 180-186, 243-247
hosting/docker-compose/ee/docker-compose.dev.yml (1)
521-521: LGTM!hosting/docker-compose/ee/docker-compose.gh.local.yml (1)
349-349: LGTM!hosting/docker-compose/ee/docker-compose.gh.yml (1)
358-358: LGTM!hosting/docker-compose/ee/env.ee.dev.example (1)
143-151: LGTM!hosting/docker-compose/ee/env.ee.gh.example (1)
144-152: LGTM!hosting/docker-compose/oss/docker-compose.dev.yml (1)
486-486: LGTM!hosting/docker-compose/oss/docker-compose.gh.local.yml (1)
345-345: LGTM!hosting/docker-compose/oss/docker-compose.gh.ssl.yml (1)
371-371: LGTM!sdks/python/agenta/sdk/agents/adapters/sandbox_agent.py (1)
71-74: LGTM!Also applies to: 85-88, 106-109, 183-186, 202-205
sdks/python/agenta/sdk/agents/dtos.py (1)
1171-1178: LGTM!sdks/python/agenta/sdk/agents/handler.py (2)
307-315: LGTM!Also applies to: 324-325, 327-327
326-326: 🔒 Security & PrivacyAuthorization Bypass (CWE-639): Authorization Bypass Through User-Controlled Key (IDOR)
Preserve the workflow-service project scope.
invoke_workflow_detachedoverwritesrequest.meta["project_id"]from itsproject_idargument before this handler runs. The runner derives coordination scope fromrunContext.project.idor signed mount credentials, not from this forwarded field. The caller must still authorize itsproject_idargument.sdks/python/agenta/sdk/agents/utils/ts_runner.py (1)
183-185: LGTM!sdks/python/agenta/sdk/agents/utils/wire.py (1)
96-99: LGTM!Also applies to: 177-182
sdks/python/agenta/sdk/agents/wire_models.py (1)
520-527: LGTM!sdks/python/agenta/sdk/models/workflows.py (1)
139-140: LGTM!Also applies to: 289-289
sdks/python/oss/tests/pytest/unit/agents/test_redaction_scope.py (1)
100-103: LGTM!sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py (1)
103-105: LGTM!Also applies to: 611-631, 1051-1072
sdks/python/agenta/sdk/decorators/routing.py (1)
240-286: LGTM!Also applies to: 710-714
sdks/python/oss/tests/pytest/unit/test_invoke_dispatch_parity_routing.py (1)
27-27: LGTM!Also applies to: 78-80
sdks/python/oss/tests/pytest/unit/test_session_input_admission_routing.py (1)
53-161: LGTM!sdks/python/oss/tests/pytest/unit/test_workflow_request_flags_running.py (1)
54-54: LGTM!Also applies to: 63-63
services/entrypoints/main.py (1)
13-13: LGTM!Also applies to: 91-93
services/runner/src/sessions/live-frames.ts (2)
223-252: LGTM!Also applies to: 265-269, 319-345
66-74: 🗄️ Data Integrity & IntegrationNo change required.
/sessions/records/ingestaccepts a non-empty JSON array ofSessionRecordIngestRequestlive-frame envelopes and publishes each frame throughpublish_live_frame. The existing record-object body remains supported separately.services/runner/src/sessions/persist.ts (1)
31-31: LGTM!Also applies to: 266-268, 303-303, 459-460
services/runner/tests/unit/continuation-admission.test.ts (1)
11-151: LGTM!services/runner/tests/unit/live-frames.test.ts (1)
9-223: LGTM!services/runner/src/server.ts (1)
479-594: LGTM!Also applies to: 646-769
services/runner/src/sessions/alive.ts (1)
152-152: LGTM!Also applies to: 203-214, 298-305, 385-391
services/runner/src/sessions/continuation-admission.ts (1)
55-124: LGTM!services/runner/tests/unit/harness-cancel-park.test.ts (1)
331-336: LGTM!services/runner/tests/unit/server.test.ts (1)
33-33: LGTM!Also applies to: 43-43, 832-931, 933-1021, 1023-1093, 1095-1169, 1171-1254, 1256-1323
services/runner/tests/unit/session-admission.test.ts (1)
140-146: LGTM!Also applies to: 573-606, 608-638
services/runner/tests/unit/wire-contract.test.ts (1)
60-62: LGTM!web/packages/agenta-chat/tests/unit/hooks/durableContinuationHold.test.ts (1)
1-213: LGTM!web/packages/agenta-playground/src/state/execution/agentRequest.ts (1)
52-55: LGTM!Also applies to: 396-403, 436-436
web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (1)
271-276: 📐 Maintainability & Code QualityNo change needed.
The callers either await
submit(...)or ignore its return value. None chains.then()or.catch()directly onsubmit(...).await undefinedis valid.web/packages/agenta-playground/tests/unit/agentRequest.test.ts (1)
700-712: LGTM!Also applies to: 714-719
web/storybook/stories/domain/SessionHistoryNotice.stories.tsx (1)
1-34: LGTM!web/packages/agenta-playground/src/agentChat.ts (1)
11-11: LGTM!Also applies to: 24-29
web/packages/agenta-playground/src/index.ts (1)
93-99: LGTM!web/packages/agenta-playground/src/state/execution/agentMessageQueue.ts (1)
28-52: LGTM!Also applies to: 129-140
web/packages/agenta-playground/src/state/execution/index.ts (1)
359-359: LGTM!Also applies to: 379-385
web/packages/agenta-playground/src/state/index.ts (1)
194-200: LGTM!web/packages/agenta-playground/tests/unit/agentMessageQueue.test.ts (1)
224-235: LGTM!Also applies to: 237-254
|
…lag-off [fix] Keep the flag-off session path unchanged in milestone 3
Return the execution that owns Send so queued and steered inputs lock and target the resumed continuation instead of the stale stream turn. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Validate mapped interaction, message, and tool events at the open-wire boundary so malformed optional strings cannot poison a committed records batch. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Return no command when the guarded bind loses its open-state race, and reject the cancel admission instead of pretending the pending input was attached. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Treat a failed Redis guard release as a bounded lease delay so it cannot replace the heartbeat result already committed to Postgres. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Require the selected stream timestamp to remain unchanged even when the sweep settled that turn as lost, so a concurrent runner heartbeat wins the collapse race. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Supply the required hydration reset callback and live-event cursor and event handler so the regression tests remain valid under strict TypeScript. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Retain the fast first interaction refresh, then exponentially back off to a sixty-second ceiling while a gate remains open. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Treat the first observed frame as the connection baseline and enforce contiguous frame indexes only after that baseline exists. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
…oderabbit [fix] Address CodeRabbit review for milestone 3
Clear accepted shared-turn ownership when the invoke stream finishes cleanly so flag-off browser-held messages can drain. Keep disconnects, aborts, and errors behind the durable terminal event. Cover the mobile shared engine and desktop parity. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
…obile-hold [fix] Release held messages on mobile when queue capabilities are absent
📘 Docs preview
This comment updates in place on every push. |
Railway Preview Environment
|
1 participant
Context
Today an approval answer lives only in the browser that gave it: a Stop, a refresh, or a second tab can lose it, and a message typed while a turn runs is refused or held only in that browser. This branch is milestone 3 of the session-control design (RFC #6495): durable approvals, then a server-held queue with Steer. It sits on milestone 2 (live events, PR #6572), which sits on milestone 1 (the Stop package, already in this release branch).
What this branch contains, in merge order
Every new behaviour ships dark:
AGENTA_SESSIONS_DURABLE_APPROVALS,AGENTA_SESSIONS_QUEUE, andAGENTA_SESSIONS_STEERdefault to off. Migrations 027 (executions state and continuation fields) and 028 (session inputs) are additive and nullable.Tests
~/agenta-qa-evidence/2026-09-05-inc6-browser/,2026-09-05-inc6-mobile-strip/,2026-09-05-inc7-browser/.What to QA
https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk