[feat] Session control milestone 3: durable approvals, queue, steer - #6575
[feat] Session control milestone 3: durable approvals, queue, steer#6575mmabrouk wants to merge 189 commits into
Conversation
Publish temporary session frames to a dedicated bounded Redis Stream. Restore the durable records queue and both consumers to independent ACK and delete behavior, with count and age retention tests. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Reject live frames above 64 KiB during request validation and immediately before Redis publication. Apply the same limit to declared HTTP request bodies and cover each boundary with regression tests. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Mark the identity-scoped session event stream as no-store so intermediaries and browsers do not retain live session content. Pin the response header in a route regression test. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document one HTTP ingress with separate durable-record and disposable-frame Redis Streams. Record the deployment-wide frame bounds and clarify that clients receive the global shared-reader capability. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Validate session live events with the shared diagnostic helper and cover invalid-frame logging. Shorten the three reviewed live-relay comments without changing behavior. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Require each live execution to start at frame zero and advance contiguously. Suppress the preview after a gap and refresh durable session records. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Restore the base branch's 100,000-entry approximate trim for durable record publication. Pin the flag-off Redis stream arguments in a regression test. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Initialize durable consumers before the optional live relay. Log relay setup failures and continue running the durable stream consumers. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document the live-frame relay and gap handling as the shipped contract. Mark sender handoff, durable event envelopes, sequences, watermarks, and Postgres replay as later target behavior. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
The two Postgres-backed records tests declared an async autouse fixture with the plain pytest decorator, which pytest-asyncio 1.x does not collect, so both files errored at setup before any query ran. With the pytest_asyncio decorator both files run: 3 passed against the proof stack database. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Await replay queue capacity so a finite durable backlog cannot be mistaken for a live producer outrunning its reader. Keep the existing drop-and-reconnect policy for live frames and cover replay larger than the configured buffer. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Route live frames and committed durable events through one bounded relay-stream append path while leaving record ingest retention unchanged. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Acquire per-session sequence cursors in a stable order while preserving record order inside each session. Leave every project batch message pending when its transactional append fails. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true📝 SummarySummary by CodeRabbit
WalkthroughThis pull request adds durable session continuations, pending-input queueing and steering, a disposable live-frame relay, and a durable session-events endpoint. It touches the Postgres schema and DAOs, the FastAPI sessions API, the TypeScript runner, the Python SDK, and desktop, mobile, and shared web chat packages, plus migrations, docs, and configuration. ChangesDurable Session Approvals and Live Events
Estimated code review effort: 5 (Critical) | ~150 minutes Merge Risk: 🟠 High · up to Queued messages, approvals, replay, and recovered turns can become stale, misidentified, or fail to advance. These issues should be fixed before merge. Sequence Diagram(s)sequenceDiagram
participant Browser
participant SessionsAPI
participant CommandsService
participant Runner
participant Postgres
Browser->>SessionsAPI: POST respond_interaction (Idempotency-Key)
SessionsAPI->>CommandsService: respond_interaction(answer)
CommandsService->>Postgres: lock execution, create continuation
CommandsService->>Runner: deliver continue_interaction command
Runner->>Runner: claimContinuationAdmission
Runner->>SessionsAPI: reportContinuationAdmission(applied)
SessionsAPI->>Postgres: settle command started
Runner->>SessionsAPI: publish records (done)
SessionsAPI->>Postgres: settle_execution_completed
Browser->>SessionsAPI: GET /sessions/{id}/events (SSE)
SessionsAPI-->>Browser: durable events + ready watermark
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 17.62% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 261 functions across 50 files. (210 skipped: 23 unsupported, 187 over the file limit.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Create and drop the records sequence index concurrently from Alembic autocommit blocks so populated tracing tables remain writable during the index operation. Cover empty and populated-table upgrades and downgrades in a disposable tracing database. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
|
@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
[fix] Build the records sequence index concurrently
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
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