[feat] Approve, deny, and stop a run from your phone (8/12) - #5687
[feat] Approve, deny, and stop a run from your phone (8/12)#5687ardaerzin wants to merge 21 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds server-side approval replay and detached resume dispatch, mobile approval and stop controls, project-wide session monitoring, a 30-minute runner approval TTL, and mobile execution and live-relay documentation. ChangesBackend approval resume
Mobile session controls
Runner approval TTL
Mobile execution and relay documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant MobileClient
participant SessionAPI
participant RecordsService
participant InteractionsDispatcher
participant Runner
MobileClient->>SessionAPI: Query project interactions and session records
SessionAPI-->>MobileClient: Pending approvals and transcript
MobileClient->>MobileClient: Stamp approval response
MobileClient->>SessionAPI: Submit detached resume request
SessionAPI->>InteractionsDispatcher: Dispatch interaction answer
InteractionsDispatcher->>RecordsService: Load durable session records
RecordsService-->>InteractionsDispatcher: Return replayable records
InteractionsDispatcher->>Runner: Invoke with composed approval messages
Runner-->>SessionAPI: Resume session
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-chat/tests/unit/transport/agentResumeRequest.test.ts (1)
1-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPrettier formatting failures block CI on both new test files. The pipeline logs report the same "TypeScript format" Prettier failure for both files; the shared root cause is that neither file has been run through the project formatter.
web/packages/agenta-chat/tests/unit/transport/agentResumeRequest.test.ts#L1-L58: runpnpm run format(orpnpm lint-fixfromweb) and commit the reformatted file.web/packages/agenta-chat/tests/unit/transport/resolveInvocationUrl.test.ts#L1-L72: runpnpm run format(orpnpm lint-fixfromweb) and commit the reformatted file.As per coding guidelines, "Run
pnpm lint-fixfrom thewebdirectory before committing."Sources: Coding guidelines, Pipeline failures
🧹 Nitpick comments (3)
docs/design/agenta-mobile/plans/2026-07-27-m3-live-relay.md (1)
218-227: 🩺 Stability & Availability | 🔵 TrivialSet limits for long-lived SSE connections.
Each watcher owns one HTTP stream and one Redis pub/sub connection. Define per-process and per-principal connection limits, maximum stream duration, and metrics for open streams and Redis connections.
Without limits, mobile reconnect storms or many open chats can exhaust API or Redis resources.
api/oss/src/apis/fastapi/sessions/models.py (1)
161-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSurface the approval-answer contract in the OpenAPI schema.
The comment documents the
user_approvalanswer shape, but a plain code comment does not appear in the generated OpenAPI schema. API consumers, including the mobile client, only see the schema, not this comment.Move this documentation into a
Field(description=...)so it is discoverable through the API docs.📝 Proposed fix
class SessionInteractionRespondRequest(BaseModel): - # For a user_approval interaction the answer is {approved: bool, tool_call_id?: str, - # message?: str} — the dispatcher composes the full resume conversation server-side - # (interactions_dispatcher.compose_approval_messages). Other kinds pass through as-is. - answer: Optional[Dict[str, Any]] = None + answer: Optional[Dict[str, Any]] = Field( + default=None, + description=( + "For a user_approval interaction: {approved: bool, tool_call_id?: str, " + "message?: str}. The dispatcher composes the full resume conversation " + "server-side. Other interaction kinds pass the answer through as-is." + ), + )web/mobile/src/features/sessions/useActionableInteractions.ts (1)
19-36: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider passing liveness state explicitly instead of reading the sibling query's cache.
refetchIntervalreadsuseLivenessPoll's cached data directly throughqueryClient.getQueryData. This works today becauseSessionListScreenrenders both hooks together, so a freshrefetchIntervalfunction is supplied on every render and the interval recomputes correctly. If a future caller usesuseActionableInteractionswithout also renderinguseLivenessPollin the same tree, the poll can silently stop reacting to liveness changes.Pass the liveness alive-count (or the liveness query result) into
useActionableInteractionsas an explicit argument. This removes the implicit coupling and keeps the dependency visible at the call site.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fbbe30b-6bce-4f91-ab82-da6aaf3df285
📒 Files selected for processing (40)
api/entrypoints/routers.pyapi/entrypoints/worker_queues.pyapi/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/tasks/asyncio/sessions/interactions_dispatcher.pyapi/oss/tests/pytest/unit/sessions/test_interactions_dispatcher.pyapi/oss/tests/pytest/unit/sessions/test_respond_interaction_enqueue.pydocs/design/agenta-mobile/README.mddocs/design/agenta-mobile/plans/2026-07-27-m3-live-relay.mddocs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.mdservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/server.tsservices/runner/tests/unit/session-pool.test.tsweb/mobile/src/features/auth/SignInScreen.tsxweb/mobile/src/features/chat/ApprovalCard.tsxweb/mobile/src/features/chat/ChatHeader.tsxweb/mobile/src/features/chat/ChatScreen.tsxweb/mobile/src/features/chat/StopButton.tsxweb/mobile/src/features/chat/TurnRow.tsxweb/mobile/src/features/chat/approvalStamp.tsweb/mobile/src/features/chat/useApprovalActions.tsweb/mobile/src/features/chat/useSessionTranscript.tsweb/mobile/src/features/chat/useTranscriptAutoScroll.tsweb/mobile/src/features/context/ContextResolver.tsxweb/mobile/src/features/context/WorkspaceProjectList.tsxweb/mobile/src/features/context/states/SignedOutNotice.tsxweb/mobile/src/features/sessions/SessionListScreen.tsxweb/mobile/src/features/sessions/SessionRow.tsxweb/mobile/src/features/sessions/SessionSearchBar.tsxweb/mobile/src/features/sessions/states/SessionListStates.tsxweb/mobile/src/features/sessions/useActionableInteractions.tsweb/mobile/src/features/sessions/useLivenessPoll.tsweb/mobile/src/features/sessions/useSessionListScrollRestore.tsweb/mobile/tests/unit/approvalStamp.test.tsweb/packages/agenta-chat/src/transport/agentResumeRequest.tsweb/packages/agenta-chat/src/transport/index.tsweb/packages/agenta-chat/src/transport/resolveInvocationUrl.tsweb/packages/agenta-chat/tests/unit/transport/agentResumeRequest.test.tsweb/packages/agenta-chat/tests/unit/transport/resolveInvocationUrl.test.tsweb/packages/agenta-entities/src/session/api/api.ts
| - Auth on a long-lived GET: `auth_middleware` (`api/oss/src/middlewares/auth.py:134`, | ||
| registered `api/entrypoints/routers.py:469` via `app.middleware("http")`) accepts Bearer, | ||
| ApiKey, AND the `sAccessToken` cookie (auth.py:290), and sets | ||
| `request.state.{user_id,project_id}` once at request start — the SSE handler then does the | ||
| same `check_action_access(VIEW_SESSIONS)` as `query_records` (router.py:475-480). Auth is | ||
| evaluated once at connect; scope holds for the connection's lifetime (standard SSE; cap the | ||
| connection age server-side if that ever matters). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Define authorization expiry for long-lived streams.
The stream checks VIEW_SESSIONS only at connection time. If access is revoked, the client can continue receiving session_id notifications until it reconnects.
Set a maximum stream age, or re-check authorization periodically and close unauthorized streams.
| - **T1 — contract + publish.** Add `records_changed_channel(project_id, session_id)` | ||
| (`records-changed:<project_id>:session:<session_id>`) and its payload shape | ||
| (`{session_id, turn_id?}`) to `api/oss/src/dbs/redis/sessions/contract.py`. In | ||
| `RecordsWorker.process_batch` (`records_worker.py:143-160`), after each successful | ||
| `append_many`, publish ONCE per distinct `(project_id, session_id)` in that project batch, | ||
| using the worker's existing durable redis client (`worker_streams.py:134-138`). | ||
| Log-and-continue on publish failure — persistence is already committed and must not be | ||
| re-driven by relay errors. Unit test with fakeredis: batch with 2 sessions ⇒ 2 publishes, | ||
| each after append; append failure ⇒ no publish for that batch. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define lifecycle event types in the wire contract.
The current payload shape does not identify running, ended, or approval-pending events. The SSE task defines only records-changed, while the mobile task handles only record revalidation.
Add explicit event types and payload fields for lifecycle changes. Add matching client invalidation for liveness and actionable interactions, or keep lifecycle events out of this iteration.
| - **T4 — `useSessionWatch(sessionId, projectId)`** in `web/mobile/src/features/chat/`: | ||
| `EventSource` on `/api/sessions/streams/watch?session_id=&project_id=` (cookie auth, | ||
| same-origin); on `records-changed` → exactly `tick()`'s body | ||
| (`useSessionTranscript.ts:47-62`): `revalidateSessionRecordsAtom` + `loadSessionMessages`; | ||
| on `open` → one revalidation (missed-event coverage); teardown on background/unmount | ||
| (visibility rules as today). `ChatScreen` cadence (`ChatScreen.tsx:38-42`) becomes: SSE | ||
| open ⇒ slow safety-net poll (30s); SSE errored/unsupported ⇒ today's 4s/7.5s cadence | ||
| unchanged (the fallback IS the current behavior — no regression path). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use the authenticated project for channel selection.
The server contract accepts session_id and obtains project_id from request.state.project_id. The client task adds project_id to the URL.
Remove project_id from the URL, or reject mismatches and always construct the channel from the authenticated project. Do not use the query value for authorization or tenant selection.
| 2. **Lifecycle events on the same channel: YES** — the watch stream also carries turn | ||
| lifecycle (running/ended/approval-pending), so mobile retires all three polls | ||
| (records tick, liveness, actionable-interactions) in favor of one EventSource; the | ||
| polls remain as the documented no-regression fallback when the stream is down. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not retire project-wide polls with a session-scoped stream.
records_changed_channel(project_id, session_id) and useSessionWatch(sessionId, projectId) cover one session. They cannot update liveness and actionable-interaction badges for other sessions in SessionListScreen.
The supplied docs/designs/sessions/frontend-integration.md:24-35 context uses one project-wide liveness query. Add a project-scoped lifecycle stream, or keep the project-wide polls. Limit this decision to an open chat session if no project-scoped stream is planned.
| # Mobile approvals + steering — design & plan | ||
|
|
||
| **Status:** PLANNED · **Date:** 2026-07-27 · **Branch:** `feat/agenta-mobile-wave-1` | ||
| **Goal:** from a phone, on a session whose agent runs in the cloud: (1) see that a turn is | ||
| running and an approval is pending with enough context to decide, (2) approve/deny and have the | ||
| agent proceed, (3) stop, and steer where feasible — all WITHOUT being the SSE stream holder. | ||
| Raw-UI ethos applies (flows/logic, no polish). All findings below are code-trace verified | ||
| (file:line); nothing was executed live. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize the plan with the executed detached-response contract.
The README records M2 as executed, but this plan still presents pre-M2 state. It also documents a different client payload from the implemented {approved} contract.
docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md#L1-L8: mark the document as a historical pre-execution snapshot or update its status.docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md#L81-L91: replace the “no producer” and “UNVERIFIED” statements with the implemented/respondbehavior.docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md#L260-L267: document{approved}as the client payload and keep resume-message composition server-side.
📍 Affects 1 file
docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md#L1-L8(this comment)docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md#L81-L91docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md#L260-L267
| // Failure-path re-arm: if the resume was accepted but the run dies before the gate | ||
| // resolves, the poll never settles us — drop back to idle so the buttons re-arm. | ||
| useEffect(() => { | ||
| if (phase !== "resuming") return | ||
| const handle = setTimeout(() => setPhase("idle"), 60_000) | ||
| return () => clearTimeout(handle) | ||
| }, [phase]) | ||
|
|
||
| const submit = useCallback( | ||
| async (target: {all: true} | {all?: false; approvalId: string}, approved: boolean) => { | ||
| if (busyRef.current) return | ||
| busyRef.current = true | ||
| setPhase("resuming") | ||
| setErrorText(null) | ||
| try { | ||
| // Never stamp a stale tail — re-read the durable records first. | ||
| const messages = (await loadSessionMessages(sessionId)) ?? [] | ||
| const pending = getPendingApprovals(messages) | ||
| if (pending.length === 0) { | ||
| throw new Error("No pending approval found — the turn may have moved on.") | ||
| } | ||
| const ids = target.all ? pending.map((p) => p.approvalId) : [target.approvalId] | ||
| const stamped = stampApprovalResponses(messages, ids, approved) | ||
| if (stamped === messages) { | ||
| throw new Error("This approval is no longer pending — refresh and retry.") | ||
| } | ||
| // The interaction row stores the run's role-keyed workflow references — | ||
| // the resolver hydrates config from them server-side (references-only body). | ||
| const interactions = await queryInteractions({ | ||
| sessionId, | ||
| projectId, | ||
| actionableOnly: true, | ||
| }) | ||
| const withRefs = (interactions ?? []).filter( | ||
| (row) => row.data?.references && Object.keys(row.data.references).length > 0, | ||
| ) | ||
| // Bind to the answered gate's own row when possible — two parked runs on | ||
| // different revisions in one session must not resume with the wrong config. | ||
| const answeredId = target.all ? undefined : target.approvalId | ||
| const matched = answeredId | ||
| ? withRefs.find((row) => row.token === answeredId) | ||
| : undefined | ||
| const references = sanitizeReferences((matched ?? withRefs[0])?.data?.references) | ||
| if (!references) { | ||
| throw new Error( | ||
| "This approval carries no workflow reference — answer on desktop.", | ||
| ) | ||
| } | ||
| const invocationUrl = await resolveInvocationUrl({ | ||
| projectId, | ||
| revisionId: | ||
| references.workflow_revision?.id ?? references.application_revision?.id, | ||
| workflowId: references.workflow?.id ?? references.application?.id, | ||
| }) | ||
| if (!invocationUrl) { | ||
| throw new Error("Could not resolve the agent's invoke URL.") | ||
| } | ||
| const request = buildAgentResumeRequest({ | ||
| invocationUrl, | ||
| references, | ||
| sessionId, | ||
| messages: stamped, | ||
| projectId, | ||
| applicationId: references.application?.id ?? undefined, | ||
| }) | ||
| const response = await fetch(request.invocationUrl, { | ||
| method: "POST", | ||
| headers: {...request.headers, "Content-Type": "application/json"}, | ||
| body: JSON.stringify(request.requestBody), | ||
| credentials: "include", | ||
| }) | ||
| if (!response.ok) { | ||
| throw new Error(`Resume failed (HTTP ${response.status}).`) | ||
| } | ||
| // Fire-and-forget: release the stream immediately — session runs survive | ||
| // client disconnect, and holding the SSE open for the whole turn is waste. | ||
| void response.body?.cancel().catch(() => undefined) | ||
| } catch (err) { | ||
| setPhase("error") | ||
| setErrorText(err instanceof Error ? err.message : "Resume failed.") | ||
| } finally { | ||
| busyRef.current = false | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to the resume fetch call to avoid a permanently stuck approve/deny flow.
fetch at Line 136 has no timeout or abort signal. If the request hangs, busyRef.current stays true forever, because it only clears in the finally block at Line 152, which never runs until the promise settles. Meanwhile, the 60-second re-arm timer at Lines 73-77 resets the visible phase to "idle" independently of the request state, so the Approve/Deny buttons look usable again but silently no-op on every click, because submit returns early at Line 81 while busyRef.current is still true.
Mobile approval is used over cellular connections where hangs are common. Add a timeout so a stalled request fails fast, clears busyRef, and surfaces the existing error state.
🔧 Proposed fix: bound the resume request with a timeout
const response = await fetch(request.invocationUrl, {
method: "POST",
headers: {...request.headers, "Content-Type": "application/json"},
body: JSON.stringify(request.requestBody),
credentials: "include",
+ signal: AbortSignal.timeout(20_000),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Failure-path re-arm: if the resume was accepted but the run dies before the gate | |
| // resolves, the poll never settles us — drop back to idle so the buttons re-arm. | |
| useEffect(() => { | |
| if (phase !== "resuming") return | |
| const handle = setTimeout(() => setPhase("idle"), 60_000) | |
| return () => clearTimeout(handle) | |
| }, [phase]) | |
| const submit = useCallback( | |
| async (target: {all: true} | {all?: false; approvalId: string}, approved: boolean) => { | |
| if (busyRef.current) return | |
| busyRef.current = true | |
| setPhase("resuming") | |
| setErrorText(null) | |
| try { | |
| // Never stamp a stale tail — re-read the durable records first. | |
| const messages = (await loadSessionMessages(sessionId)) ?? [] | |
| const pending = getPendingApprovals(messages) | |
| if (pending.length === 0) { | |
| throw new Error("No pending approval found — the turn may have moved on.") | |
| } | |
| const ids = target.all ? pending.map((p) => p.approvalId) : [target.approvalId] | |
| const stamped = stampApprovalResponses(messages, ids, approved) | |
| if (stamped === messages) { | |
| throw new Error("This approval is no longer pending — refresh and retry.") | |
| } | |
| // The interaction row stores the run's role-keyed workflow references — | |
| // the resolver hydrates config from them server-side (references-only body). | |
| const interactions = await queryInteractions({ | |
| sessionId, | |
| projectId, | |
| actionableOnly: true, | |
| }) | |
| const withRefs = (interactions ?? []).filter( | |
| (row) => row.data?.references && Object.keys(row.data.references).length > 0, | |
| ) | |
| // Bind to the answered gate's own row when possible — two parked runs on | |
| // different revisions in one session must not resume with the wrong config. | |
| const answeredId = target.all ? undefined : target.approvalId | |
| const matched = answeredId | |
| ? withRefs.find((row) => row.token === answeredId) | |
| : undefined | |
| const references = sanitizeReferences((matched ?? withRefs[0])?.data?.references) | |
| if (!references) { | |
| throw new Error( | |
| "This approval carries no workflow reference — answer on desktop.", | |
| ) | |
| } | |
| const invocationUrl = await resolveInvocationUrl({ | |
| projectId, | |
| revisionId: | |
| references.workflow_revision?.id ?? references.application_revision?.id, | |
| workflowId: references.workflow?.id ?? references.application?.id, | |
| }) | |
| if (!invocationUrl) { | |
| throw new Error("Could not resolve the agent's invoke URL.") | |
| } | |
| const request = buildAgentResumeRequest({ | |
| invocationUrl, | |
| references, | |
| sessionId, | |
| messages: stamped, | |
| projectId, | |
| applicationId: references.application?.id ?? undefined, | |
| }) | |
| const response = await fetch(request.invocationUrl, { | |
| method: "POST", | |
| headers: {...request.headers, "Content-Type": "application/json"}, | |
| body: JSON.stringify(request.requestBody), | |
| credentials: "include", | |
| }) | |
| if (!response.ok) { | |
| throw new Error(`Resume failed (HTTP ${response.status}).`) | |
| } | |
| // Fire-and-forget: release the stream immediately — session runs survive | |
| // client disconnect, and holding the SSE open for the whole turn is waste. | |
| void response.body?.cancel().catch(() => undefined) | |
| } catch (err) { | |
| setPhase("error") | |
| setErrorText(err instanceof Error ? err.message : "Resume failed.") | |
| } finally { | |
| busyRef.current = false | |
| } | |
| // Failure-path re-arm: if the resume was accepted but the run dies before the gate | |
| // resolves, the poll never settles us — drop back to idle so the buttons re-arm. | |
| useEffect(() => { | |
| if (phase !== "resuming") return | |
| const handle = setTimeout(() => setPhase("idle"), 60_000) | |
| return () => clearTimeout(handle) | |
| }, [phase]) | |
| const submit = useCallback( | |
| async (target: {all: true} | {all?: false; approvalId: string}, approved: boolean) => { | |
| if (busyRef.current) return | |
| busyRef.current = true | |
| setPhase("resuming") | |
| setErrorText(null) | |
| try { | |
| // Never stamp a stale tail — re-read the durable records first. | |
| const messages = (await loadSessionMessages(sessionId)) ?? [] | |
| const pending = getPendingApprovals(messages) | |
| if (pending.length === 0) { | |
| throw new Error("No pending approval found — the turn may have moved on.") | |
| } | |
| const ids = target.all ? pending.map((p) => p.approvalId) : [target.approvalId] | |
| const stamped = stampApprovalResponses(messages, ids, approved) | |
| if (stamped === messages) { | |
| throw new Error("This approval is no longer pending — refresh and retry.") | |
| } | |
| // The interaction row stores the run's role-keyed workflow references — | |
| // the resolver hydrates config from them server-side (references-only body). | |
| const interactions = await queryInteractions({ | |
| sessionId, | |
| projectId, | |
| actionableOnly: true, | |
| }) | |
| const withRefs = (interactions ?? []).filter( | |
| (row) => row.data?.references && Object.keys(row.data.references).length > 0, | |
| ) | |
| // Bind to the answered gate's own row when possible — two parked runs on | |
| // different revisions in one session must not resume with the wrong config. | |
| const answeredId = target.all ? undefined : target.approvalId | |
| const matched = answeredId | |
| ? withRefs.find((row) => row.token === answeredId) | |
| : undefined | |
| const references = sanitizeReferences((matched ?? withRefs[0])?.data?.references) | |
| if (!references) { | |
| throw new Error( | |
| "This approval carries no workflow reference — answer on desktop.", | |
| ) | |
| } | |
| const invocationUrl = await resolveInvocationUrl({ | |
| projectId, | |
| revisionId: | |
| references.workflow_revision?.id ?? references.application_revision?.id, | |
| workflowId: references.workflow?.id ?? references.application?.id, | |
| }) | |
| if (!invocationUrl) { | |
| throw new Error("Could not resolve the agent's invoke URL.") | |
| } | |
| const request = buildAgentResumeRequest({ | |
| invocationUrl, | |
| references, | |
| sessionId, | |
| messages: stamped, | |
| projectId, | |
| applicationId: references.application?.id ?? undefined, | |
| }) | |
| const response = await fetch(request.invocationUrl, { | |
| method: "POST", | |
| headers: {...request.headers, "Content-Type": "application/json"}, | |
| body: JSON.stringify(request.requestBody), | |
| credentials: "include", | |
| signal: AbortSignal.timeout(20_000), | |
| }) | |
| if (!response.ok) { | |
| throw new Error(`Resume failed (HTTP ${response.status}).`) | |
| } | |
| // Fire-and-forget: release the stream immediately — session runs survive | |
| // client disconnect, and holding the SSE open for the whole turn is waste. | |
| void response.body?.cancel().catch(() => undefined) | |
| } catch (err) { | |
| setPhase("error") | |
| setErrorText(err instanceof Error ? err.message : "Resume failed.") | |
| } finally { | |
| busyRef.current = false | |
| } |
| void loadSessionMessages(sessionId) | ||
| .then((msgs) => { | ||
| if (!cancelled && msgs && msgs.length > 0) { | ||
| setMessages(msgs) | ||
| setState("ready") | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| inFlight = false | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a .catch() to the poll chain.
loadSessionMessages(sessionId) at Line 52 has a .then() and a .finally() but no .catch(). A rejection propagates unhandled past .finally(), producing an unhandled promise rejection on every failed poll tick.
🔧 Proposed fix
void loadSessionMessages(sessionId)
.then((msgs) => {
if (!cancelled && msgs && msgs.length > 0) {
setMessages(msgs)
setState("ready")
}
})
+ .catch(() => undefined)
.finally(() => {
inFlight = false
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void loadSessionMessages(sessionId) | |
| .then((msgs) => { | |
| if (!cancelled && msgs && msgs.length > 0) { | |
| setMessages(msgs) | |
| setState("ready") | |
| } | |
| }) | |
| .finally(() => { | |
| inFlight = false | |
| }) | |
| void loadSessionMessages(sessionId) | |
| .then((msgs) => { | |
| if (!cancelled && msgs && msgs.length > 0) { | |
| setMessages(msgs) | |
| setState("ready") | |
| } | |
| }) | |
| .catch(() => undefined) | |
| .finally(() => { | |
| inFlight = false | |
| }) |
fe228cc to
54e141c
Compare
1d70a28 to
e059cec
Compare
54e141c to
08c72cb
Compare
e059cec to
c110d9b
Compare
08c72cb to
d23caf8
Compare
c110d9b to
5a9a6a0
Compare
d23caf8 to
74b3f55
Compare
5a9a6a0 to
d3537e8
Compare
|
Went through the three code findings. Two are fixed; one is superseded further up the stack.
Transcript poll had no Resume The plan-document findings (§ live-relay authorization expiry, refetch bounds, lifecycle event types in the wire contract, keeping the pending-approval poll after liveness goes idle) are accuracy issues in a design doc rather than in shipped behaviour. They are tracked with the rest of the plan-doc set and not applied in this round. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
74b3f55 to
7d6b07f
Compare
d3537e8 to
672590a
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
7d6b07f to
478248f
Compare
672590a to
b67caf8
Compare
An awaiting_approval park is exactly the pending-interaction case: the turn paused on a human gate and the sandbox waits warm. Phone-latency answers (mobile approvals, plan 4b-4) mostly landed after the old 5-minute window and degraded to cold replay; 30 minutes keeps them on the warm respondPermission resume. Still bounded by the mount-credential expiry check and overridable via AGENTA_RUNNER_SESSION_APPROVAL_TTL_MS.
One project-scoped querySessionStreams({isAlive:true}) poll mirroring the desktop
liveness pattern: low-priority, 15s while anything is alive, stops when idle,
re-checks on focus. Session rows read a fresh running/live badge off the shared
poll, falling back to the list row's own flags until it resolves.
…n list queryInteractions in @agenta/entities/session now allows omitting session_id (the backend already treats it as optional), so ONE actionable_only query returns every pending approval in the project. Mobile polls it on the liveness cadence (15s while pending or alive, stop when idle, refetch on focus) and renders a needs-approval badge per row plus a pending count in the header.
…respond path
POST /sessions/interactions/{id}/respond existed but had no producer of a
runner-consumable answer: the dispatcher forwarded the raw client payload as
data.inputs, which the agent service cannot turn into a resumable conversation.
The dispatcher now composes the resume conversation server-side for
user_approval interactions (mobile approvals plan M2.1): it replays the
session's durable records into wire messages and appends the
{approved, interactionToken} tool_result envelope bound to the gated
toolCallId — the exact shape the runner's decision map and warm approval-park
resume read. The client payload stays {approved, tool_call_id?, message?};
an optional message rides as a trailing user note (deny-with-redirect, M2.3).
The envelope lands on the last assistant message, never a new user prompt, so
a warm-parked sandbox keeps its history-fingerprint match and resumes live;
with no records the gated call anchor is synthesized from the interaction row
so cold replay can still bind the decision by name+args.
Wiring: the dispatcher gains the records service in both compositions (API
producer and queue worker), and the route's no-worker fallback now goes
through the dispatcher so both paths share one composition.
…the chat screen Records replay already reconstructs the approval-requested tool part; the chat transcript now renders it as a highlighted raw card (tool name + exact JSON payload) with disabled Approve/Deny buttons until the resume path lands. While the foregrounded screen shows a pending approval or a running turn the records poll tightens to 7.5s (invalidate + shared-cache re-read), skipping ticks when the tab is hidden; otherwise the default staleTime governs.
buildAgentResumeRequest composes the invoke body for answering a HITL approval
without the hydrated workflow molecule: {session_id, references, data.inputs
.messages} with stream Accept + vercel format headers, and project_id ALWAYS
on the query string (the routing middleware reads it for cookie auth). The
body never carries data.parameters — that absence is what triggers server-side
reference hydration in the SDK resolver — and a unit test pins the invariant.
resolveInvocationUrl fetches the revision through the Fern-backed retrieveWorkflowRevision (revision-id ref preferred, workflow-id fallback, one call carries both) and applies the data.url|uri -> /invoke rule mirrored from the entities invocationUrl atom — no molecule store required, so the lite resume path can derive its endpoint from a session or interaction row.
The approval card's buttons go live: fresh records are re-read, the decision is
stamped onto the tail as the approval-responded shape transcriptToMessages
produces (the SDK folds it into the {approved, interactionToken} tool_result
envelope), and ONE references-only resume POST fires via buildAgentResumeRequest
with the interaction row's role-keyed workflow refs + resolveInvocationUrl.
Fire-and-forget per the plan decision: the response is drained in the
background and the records poll (tightened to 4s while resuming) repaints the
transcript until the turn settles. Deny also resumes; approve-all answers every
gate in the same single POST.
A running turn surfaces a raw Stop button in the chat screen: the no-inputs commandSessionStream call drops the running locks (cancel mode) and the runner aborts on its next heartbeat, up to 30s later — the liveness poll confirms and unmounts the strip. Until feat/agent-cancel-steer lands the turn settles as an error record rather than a clean cancelled state; the UI copy says so.
The chat header and the sessions search bar scrolled away with the page because both screens used document scroll (min-h-dvh columns). Make each screen an h-dvh flex column with a shrink-0 header and a flex-1 overflow-y-auto transcript/list scroller, with overscroll-contain so reaching the edge of the scroller does not chain into pull-to-refresh. An inner scroller loses the browser's native scroll restoration, so the sessions list records its scrollTop per project and restores it once per mount — back-navigation from a chat lands where the user left off (the infinite-query cache still holds the loaded pages).
The chat transcript opened at the oldest message and stayed there while the records poll appended new ones. Pin the scroller to the bottom on first content and after each poll delivery, but only while the user is already within 80px of the bottom — scrolling up to read history is never yanked back down. Plain scrollTop math on the transcript scroller, no libraries.
- 16px font (text-base) on the search, email, and password inputs so iOS Safari stops auto-zooming the page on focus. - env(safe-area-inset-bottom) padding on the transcript tail, the sessions scroller, and the root escape-hatch footer so the home indicator never covers the last row or link (viewport-fit=cover is already set in _app). - min-h-11 (~44px) hit areas on Approve/Deny/Approve-all, Stop, both Retry buttons, the project picker rows, and the sign-in submit; padding-with-negative-margin hit areas on the Back and Sign in links. - overscroll containment on the approval payload pre scroller.
A tool_result record stores only the call id, so every result the respond dispatcher replayed was anonymous — including the approval envelope itself. The runner renders an approved-but-unrun call as "Call <toolName> again with the same arguments", which degraded to the literal word "tool": an instruction naming nothing the model could call. Faced with that, the model reproduced the replay's own [called ...] notation as prose and reported a fabricated completion. Carry the name forward from the tool_call, as the runner's own reconstructMessages does, and stamp it on the envelope. toolName is not part of historyFingerprint, so warm-resume parity is unaffected.
v0.107.0 replaced SessionInteractionData.request with a typed SessionInteractionRequest (extra="allow", and it declares tool_call_id). The dispatcher still treated it as a dict, so recovering the gated call's name+args raised AttributeError on the no-records path.
478248f to
823f0e5
Compare
b67caf8 to
df43ce9
Compare
1 participant
Context
This is the point of the mobile app. Your agent runs in the cloud and pauses for approval; without a phone you cannot answer until you are back at your desk, and the run sits parked.
Changes
The session list gains a project-wide liveness poll (a running badge) and pending-approval badges, both from one project-scoped query rather than one request per row. The chat screen shows the pending gate and tightens its records poll while a turn is live.
Answering happens through the detached respond path: the client sends
{approved}and the backend composes the resume. The runner's warm-park window widens to 30 minutes so an approval answered from a phone still lands on a warm sandbox instead of a cold replay.Stopping a run is cooperative cancel, not a kill.
Also here: the chat UX mechanics that make the screen usable on a phone. Pinned headers with contained scrolling, the transcript pinned to the latest message, safe-area insets, and inputs at 16px so iOS does not zoom the viewport on focus.
Tests / notes
docs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.md, including why the answer rides the detached dispatcher rather than a client-built invoke.What to QA