fix(sessions): guard Stop by execution and cancel pending approvals - #6504
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (119)
🚧 Files skipped from review as they are similar to previous changes (36)
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe change adds guarded session cancellation, single-turn admission, durable record redelivery, release-gate validation, persisted build-kit UI state, and related client, runner, API, documentation, and maintenance updates. ChangesSession control and cancellation
Records durability
Release validation and UI persistence
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟠 High · up to Accepted Stops can remain running, valid later Stops can be rejected, refused attachments can be lost, and invalid redelivery settings can delete pending records. These issues should be fixed before merge. Sequence Diagram(s)sequenceDiagram
participant ChatUI
participant SessionClient
participant SessionsAPI
participant Redis
participant Runner
ChatUI->>SessionClient: Submit Stop with expected execution ID
SessionClient->>SessionsAPI: Send cancellation request
SessionsAPI->>Redis: Validate owner and turn start
Redis-->>SessionsAPI: Cancelled turn IDs or stale mismatch
SessionsAPI-->>SessionClient: Structured cancellation outcome
Runner->>ChatUI: Stream admitted turn metadata and terminal events
ChatUI->>SessionClient: Update Stop and transcript state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 31.54% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 279 functions across 50 files. (68 skipped: 32 unsupported, 36 over the file limit.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
mmabrouk
changed the title
📘 Docs preview
This comment updates in place on every push. |
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Review summary
- Risky:
api/oss/src/core/sessions/streams/service.py:283- Stop checks the current owners, then removes the Redis locks in separate operations. A replacement turn can acquire the session between those operations and be cancelled. - Missing tests:
web/packages/agenta-chat/tests/unit/model/userStop.test.tsandweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsdo not load in a fresh install because the new model import pulls UI modules into the package test boundary.
Reviewed at 017ab21a53cf92c7107488e63c47c584c0c59a70. The committed backend tests pass, but a forced interleaving test reproduces the lock race. The focused entities, desktop stop-state, and mobile stop-state tests pass. The focused chat run passes 69 tests, but the two suites above fail before collecting tests with TypeError: jsxDEV is not a function.
Railway Preview Environment
Updated at 2026-09-04T23:20:00.071Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/oss/src/components/AgentChatSlice/AgentConversation.tsx (1)
433-442: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear session status after a stopped parked turn.
When
stoppedbecomes true, this effect can still publish"awaiting"from the stale approval part. The approval dock is hidden and sends are unblocked, but the session status remains incorrect until a later transcript update. Derive"idle"forstoppedbefore pending states and includestoppedin the dependencies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 692c9546-94a7-488a-9c01-6c68a4281722
⛔ Files ignored due to path filters (2)
web/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/SessionStreamCommandRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStreamCommandResponse.tsis excluded by!**/generated/**
📒 Files selected for processing (53)
api/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/dbs/redis/sessions/contract.pyapi/oss/src/dbs/redis/sessions/locks.pyapi/oss/tests/pytest/unit/sessions/test_cancel_cancels_pending_interactions.pyapi/oss/tests/pytest/unit/sessions/test_cancel_stop_guard.pydocs/design/session-control-and-live-events/README.mddocs/design/session-control-and-live-events/context.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/plan.mddocs/design/session-control-and-live-events/records-invariants.mddocs/design/session-control-and-live-events/requirements.mddocs/design/session-control-and-live-events/research.mddocs/design/session-control-and-live-events/rfc.mddocs/design/session-control-and-live-events/slice-stop-guard.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/StopButton.tsxweb/mobile/src/features/chat/stopHereState.tsweb/mobile/src/features/chat/useSessionWatch.tsweb/mobile/tests/unit/stopHereState.test.tsweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/stopState.test.tsweb/oss/src/components/AgentChatSlice/assets/stopState.tsweb/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/packages/agenta-chat/src/assets/agentTurn.tsweb/packages/agenta-chat/src/assets/composerState.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/components/ChatComposer.tsxweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/userStop.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/tests/unit/assets/agentTurn.test.tsweb/packages/agenta-chat/tests/unit/assets/composerState.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/model/liveApprovals.test.tsweb/packages/agenta-chat/tests/unit/model/userStop.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/tests/unit/session-cancel-stream.test.tsweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
017ab21 to
3982337
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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)
docs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.md (1)
97-102: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAuthorization Bypass (CWE-863): Incorrect Authorization
Document the mixed-version authorization impact.
During a staggered deployment, an old runner can interpret the SDK's seven
request.toolsnames as a grant list and restore previously deselected tools. State this migration risk explicitly.
🧹 Nitpick comments (5)
api/oss/src/tasks/asyncio/sessions/records_worker.py (1)
221-225: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the per-record retry when the write path itself is down.
The retry issues one
append_manycall per entry. When Postgres is unreachable, every call fails, so a 50-entry group produces 50 additional failing calls. The reclaim pass repeats this in each window, which adds load to a database that is already unavailable.Consider stopping the loop after a few consecutive failures. The remaining entries stay pending either way, so the outcome does not change and the cost drops.
♻️ Proposed bound on consecutive failures
total_appended = 0 committed_ids: List[bytes] = [] - for entry in entries: + consecutive_failures = 0 + for entry in entries: appended, ok = await self._append(project_id=project_id, entries=[entry]) if ok: total_appended += appended committed_ids.append(entry[0]) + consecutive_failures = 0 + continue + consecutive_failures += 1 + # Consecutive failures mean the write path is down, not a poison record. + # The rest stays pending, so stop hammering it. + if consecutive_failures >= 3: + breakweb/packages/agenta-entities/src/workflow/state/store.ts (1)
1469-1473: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce the added comments to one short line each. Keep only the non-obvious invariant in the comments at lines 1469–1473, 1481–1486, and 1522.
web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (1)
86-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce each added comment to one short line. The
web/AGENTS.mdhard rule covers all six files; longer comments are allowed only for genuinely surprising constraints and must remain brief.api/ee/src/dbs/postgres/sessions/records/dao.py (1)
102-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the historical failure explanation. The query and
RecordDBEdefinition already show the composite(project_id, record_id)key, so this comment adds past-defect narration instead of non-obvious rationale.web/packages/agenta-chat/tests/unit/model/error.test.ts (1)
87-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep each test comment to one short line.
web/AGENTS.mdapplies to both files and prohibits multi-line explanatory blocks. Collapse the four-line admission comment and the three-linelocalStoragecomment to one line each.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 5c593072-dfb7-4541-8ed8-47b725b41efb
⛔ Files ignored due to path filters (6)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/uv.lockis excluded by!**/*.lockweb/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/SessionStreamCommandRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStreamCommandResponse.tsis excluded by!**/generated/**
📒 Files selected for processing (119)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/path_triggers.py.agents/skills/agent-release-gate/resources/qa_product.py.agents/skills/agent-release-gate/resources/session_control.py.agents/skills/agent-release-gate/resources/test_qa_product_concurrency.py.agents/skills/agent-release-gate/resources/test_session_control.py.all-contributorsrcREADME.mdapi/ee/src/dbs/postgres/sessions/records/dao.pyapi/entrypoints/worker_streams.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/dbs/redis/sessions/contract.pyapi/oss/src/dbs/redis/sessions/locks.pyapi/oss/src/routers/user_profile.pyapi/oss/src/services/db_manager.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/tasks/asyncio/shared/consumer.pyapi/oss/src/tasks/taskiq/triggers/worker.pyapi/oss/src/utils/caching.pyapi/oss/src/utils/crypting.pyapi/oss/src/utils/env.pyapi/oss/src/utils/exceptions.pyapi/oss/src/utils/helpers.pyapi/oss/tests/pytest/unit/sessions/test_cancel_cancels_pending_interactions.pyapi/oss/tests/pytest/unit/sessions/test_cancel_stop_guard.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_lock_races.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_handover.pyapi/oss/tests/pytest/unit/sessions/test_project_scoped_locks.pyapi/oss/tests/pytest/unit/sessions/test_records_worker_durability.pyapi/oss/tests/pytest/unit/sessions/test_watch_publish.pyapi/oss/tests/pytest/unit/triggers/test_triggers_worker_lifecycle.pyapi/oss/tests/pytest/unit/utils/test_caching.pyapi/pyproject.tomlclients/python/pyproject.tomldocs/design/agent-workflows/documentation/adapters/agenta.mddocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mddocs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.mddocs/design/session-control-and-live-events/README.mddocs/design/session-control-and-live-events/context.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/plan.mddocs/design/session-control-and-live-events/records-invariants.mddocs/design/session-control-and-live-events/requirements.mddocs/design/session-control-and-live-events/research.mddocs/design/session-control-and-live-events/rfc.mddocs/design/session-control-and-live-events/slice-admission.mddocs/design/session-control-and-live-events/slice-records-ack.mddocs/design/session-control-and-live-events/slice-stop-guard.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/kubernetes/helm/Chart.yamlsdks/python/agenta/sdk/agents/adapters/vercel/stream.pysdks/python/oss/tests/pytest/unit/agents/adapters/test_vercel_stream_conformance.pysdks/python/pyproject.tomlservices/pyproject.tomlservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/lifecycle/session-coordinator.tsservices/runner/src/protocol.tsservices/runner/src/server.tsservices/runner/src/sessions/admission.tsservices/runner/src/sessions/alive.tsservices/runner/tests/unit/session-admission.test.tsservices/runner/tests/unit/session-alive-interrupt.test.tsservices/runner/tests/unit/session-keepalive-dispatch.test.tsservices/runner/tests/unit/session-steer-mount-loss.test.tsweb/ee/package.jsonweb/mobile/package.jsonweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/StopButton.tsxweb/mobile/src/features/chat/stopHereState.tsweb/mobile/src/features/chat/useSessionWatch.tsweb/mobile/tests/unit/stopHereState.test.tsweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/stopState.test.tsweb/oss/src/components/AgentChatSlice/assets/stopState.tsweb/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/package.jsonweb/packages/agenta-api-client/package.jsonweb/packages/agenta-chat/src/assets/agentTurn.tsweb/packages/agenta-chat/src/assets/composerState.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/components/ChatComposer.tsxweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/userStop.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/tests/unit/assets/agentTurn.test.tsweb/packages/agenta-chat/tests/unit/assets/composerState.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/liveApprovals.test.tsweb/packages/agenta-chat/tests/unit/model/userStop.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.tsweb/packages/agenta-entities/tests/unit/session-cancel-stream.test.tsweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx
🚧 Files skipped from review as they are similar to previous changes (36)
- web/packages/agenta-chat/tests/unit/assets/composerState.test.ts
- web/packages/agenta-chat/tests/unit/model/liveApprovals.test.ts
- web/packages/agenta-entities/src/session/index.ts
- web/packages/agenta-chat/src/assets/index.ts
- web/packages/agenta-chat/src/model/approvals.ts
- web/packages/agenta-chat/src/model/index.ts
- web/mobile/src/features/chat/useSessionWatch.ts
- web/mobile/tests/unit/stopHereState.test.ts
- web/mobile/src/features/chat/stopHereState.ts
- web/packages/agenta-entities/src/session/core/schema.ts
- docs/design/session-control-and-live-events/README.md
- web/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx
- web/packages/agenta-chat/tests/unit/model/userStop.test.ts
- web/packages/agenta-chat/src/assets/agentTurn.ts
- web/oss/src/components/AgentChatSlice/assets/stopState.test.ts
- web/mobile/src/features/chat/StopButton.tsx
- web/oss/src/components/AgentChatSlice/assets/stopState.ts
- web/packages/agenta-ui/src/RichChatInput/RichChatInput.tsx
- web/packages/agenta-entities/tests/unit/session-cancel-stream.test.ts
- web/packages/agenta-chat/src/model/userStop.ts
- web/packages/agenta-chat/src/hooks/useAgentConversation.ts
- api/oss/src/core/sessions/streams/dtos.py
- web/packages/agenta-chat/src/assets/composerState.ts
- web/packages/agenta-chat/src/components/ChatComposer.tsx
- web/packages/agenta-chat/tests/unit/assets/agentTurn.test.ts
- api/oss/src/core/sessions/streams/types.py
- web/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsx
- web/packages/agenta-chat/src/state/sessionEphemera.ts
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- api/oss/tests/pytest/unit/sessions/test_cancel_cancels_pending_interactions.py
- api/oss/src/apis/fastapi/sessions/router.py
- web/packages/agenta-entities/src/session/api/api.ts
- docs/design/session-control-and-live-events/decisions.md
- docs/design/session-control-and-live-events/tonight-handoff.md
- web/packages/agenta-chat/src/assets/transcriptToMessages.ts
- web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.md (1)
97-102: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAuthorization Bypass (CWE-863): Incorrect Authorization
Document the mixed-version authorization impact.
During a staggered deployment, an old runner can interpret the SDK's seven
request.toolsnames as a grant list and restore previously deselected tools. State this migration risk explicitly.
🧹 Nitpick comments (5)
api/oss/src/tasks/asyncio/sessions/records_worker.py (1)
221-225: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the per-record retry when the write path itself is down.
The retry issues one
append_manycall per entry. When Postgres is unreachable, every call fails, so a 50-entry group produces 50 additional failing calls. The reclaim pass repeats this in each window, which adds load to a database that is already unavailable.Consider stopping the loop after a few consecutive failures. The remaining entries stay pending either way, so the outcome does not change and the cost drops.
♻️ Proposed bound on consecutive failures
total_appended = 0 committed_ids: List[bytes] = [] - for entry in entries: + consecutive_failures = 0 + for entry in entries: appended, ok = await self._append(project_id=project_id, entries=[entry]) if ok: total_appended += appended committed_ids.append(entry[0]) + consecutive_failures = 0 + continue + consecutive_failures += 1 + # Consecutive failures mean the write path is down, not a poison record. + # The rest stays pending, so stop hammering it. + if consecutive_failures >= 3: + breakweb/packages/agenta-entities/src/workflow/state/store.ts (1)
1469-1473: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce the added comments to one short line each. Keep only the non-obvious invariant in the comments at lines 1469–1473, 1481–1486, and 1522.
web/packages/agenta-chat/src/hooks/useAgentChatQueue.ts (1)
86-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce each added comment to one short line. The
web/AGENTS.mdhard rule covers all six files; longer comments are allowed only for genuinely surprising constraints and must remain brief.api/ee/src/dbs/postgres/sessions/records/dao.py (1)
102-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the historical failure explanation. The query and
RecordDBEdefinition already show the composite(project_id, record_id)key, so this comment adds past-defect narration instead of non-obvious rationale.web/packages/agenta-chat/tests/unit/model/error.test.ts (1)
87-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep each test comment to one short line.
web/AGENTS.mdapplies to both files and prohibits multi-line explanatory blocks. Collapse the four-line admission comment and the three-linelocalStoragecomment to one line each.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 5c593072-dfb7-4541-8ed8-47b725b41efb
⛔ Files ignored due to path filters (6)
api/uv.lockis excluded by!**/*.lockclients/python/uv.lockis excluded by!**/*.locksdks/python/uv.lockis excluded by!**/*.lockservices/uv.lockis excluded by!**/*.lockweb/packages/agenta-api-client/src/generated/api/resources/sessions/client/requests/SessionStreamCommandRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SessionStreamCommandResponse.tsis excluded by!**/generated/**
📒 Files selected for processing (119)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/path_triggers.py.agents/skills/agent-release-gate/resources/qa_product.py.agents/skills/agent-release-gate/resources/session_control.py.agents/skills/agent-release-gate/resources/test_qa_product_concurrency.py.agents/skills/agent-release-gate/resources/test_session_control.py.all-contributorsrcREADME.mdapi/ee/src/dbs/postgres/sessions/records/dao.pyapi/entrypoints/worker_streams.pyapi/oss/src/apis/fastapi/evaluations/router.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/dbs/redis/sessions/contract.pyapi/oss/src/dbs/redis/sessions/locks.pyapi/oss/src/routers/user_profile.pyapi/oss/src/services/db_manager.pyapi/oss/src/tasks/asyncio/sessions/records_worker.pyapi/oss/src/tasks/asyncio/shared/consumer.pyapi/oss/src/tasks/taskiq/triggers/worker.pyapi/oss/src/utils/caching.pyapi/oss/src/utils/crypting.pyapi/oss/src/utils/env.pyapi/oss/src/utils/exceptions.pyapi/oss/src/utils/helpers.pyapi/oss/tests/pytest/unit/sessions/test_cancel_cancels_pending_interactions.pyapi/oss/tests/pytest/unit/sessions/test_cancel_stop_guard.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_is_current_turn.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_lock_races.pyapi/oss/tests/pytest/unit/sessions/test_heartbeat_turn_handover.pyapi/oss/tests/pytest/unit/sessions/test_project_scoped_locks.pyapi/oss/tests/pytest/unit/sessions/test_records_worker_durability.pyapi/oss/tests/pytest/unit/sessions/test_watch_publish.pyapi/oss/tests/pytest/unit/triggers/test_triggers_worker_lifecycle.pyapi/oss/tests/pytest/unit/utils/test_caching.pyapi/pyproject.tomlclients/python/pyproject.tomldocs/design/agent-workflows/documentation/adapters/agenta.mddocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mddocs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.mddocs/design/session-control-and-live-events/README.mddocs/design/session-control-and-live-events/context.mddocs/design/session-control-and-live-events/decisions.mddocs/design/session-control-and-live-events/plan.mddocs/design/session-control-and-live-events/records-invariants.mddocs/design/session-control-and-live-events/requirements.mddocs/design/session-control-and-live-events/research.mddocs/design/session-control-and-live-events/rfc.mddocs/design/session-control-and-live-events/slice-admission.mddocs/design/session-control-and-live-events/slice-records-ack.mddocs/design/session-control-and-live-events/slice-stop-guard.mddocs/design/session-control-and-live-events/status.mddocs/design/session-control-and-live-events/tonight-handoff.mdhosting/kubernetes/helm/Chart.yamlsdks/python/agenta/sdk/agents/adapters/vercel/stream.pysdks/python/oss/tests/pytest/unit/agents/adapters/test_vercel_stream_conformance.pysdks/python/pyproject.tomlservices/pyproject.tomlservices/runner/src/engines/sandbox_agent/errors.tsservices/runner/src/lifecycle/session-coordinator.tsservices/runner/src/protocol.tsservices/runner/src/server.tsservices/runner/src/sessions/admission.tsservices/runner/src/sessions/alive.tsservices/runner/tests/unit/session-admission.test.tsservices/runner/tests/unit/session-alive-interrupt.test.tsservices/runner/tests/unit/session-keepalive-dispatch.test.tsservices/runner/tests/unit/session-steer-mount-loss.test.tsweb/ee/package.jsonweb/mobile/package.jsonweb/mobile/src/features/chat/Composer.tsxweb/mobile/src/features/chat/LiveConversation.tsxweb/mobile/src/features/chat/StopButton.tsxweb/mobile/src/features/chat/stopHereState.tsweb/mobile/src/features/chat/useSessionWatch.tsweb/mobile/tests/unit/stopHereState.test.tsweb/oss/package.jsonweb/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/assets/stopState.test.tsweb/oss/src/components/AgentChatSlice/assets/stopState.tsweb/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/package.jsonweb/packages/agenta-api-client/package.jsonweb/packages/agenta-chat/src/assets/agentTurn.tsweb/packages/agenta-chat/src/assets/composerState.tsweb/packages/agenta-chat/src/assets/index.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/components/ChatComposer.tsxweb/packages/agenta-chat/src/hooks/useAgentChatQueue.tsweb/packages/agenta-chat/src/hooks/useAgentConversation.tsweb/packages/agenta-chat/src/model/approvals.tsweb/packages/agenta-chat/src/model/error.tsweb/packages/agenta-chat/src/model/index.tsweb/packages/agenta-chat/src/model/userStop.tsweb/packages/agenta-chat/src/state/sessionEphemera.tsweb/packages/agenta-chat/tests/unit/assets/agentTurn.test.tsweb/packages/agenta-chat/tests/unit/assets/composerState.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentChatQueue.test.tsweb/packages/agenta-chat/tests/unit/hooks/useAgentConversation.test.tsweb/packages/agenta-chat/tests/unit/model/error.test.tsweb/packages/agenta-chat/tests/unit/model/liveApprovals.test.tsweb/packages/agenta-chat/tests/unit/model/userStop.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.tsweb/packages/agenta-entities/tests/unit/session-cancel-stream.test.tsweb/packages/agenta-ui/src/RichChatInput/RichChatInput.tsxweb/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx
🚧 Files skipped from review as they are similar to previous changes (36)
- web/packages/agenta-chat/tests/unit/assets/composerState.test.ts
- web/packages/agenta-chat/tests/unit/model/liveApprovals.test.ts
- web/packages/agenta-entities/src/session/index.ts
- web/packages/agenta-chat/src/assets/index.ts
- web/packages/agenta-chat/src/model/approvals.ts
- web/packages/agenta-chat/src/model/index.ts
- web/mobile/src/features/chat/useSessionWatch.ts
- web/mobile/tests/unit/stopHereState.test.ts
- web/mobile/src/features/chat/stopHereState.ts
- web/packages/agenta-entities/src/session/core/schema.ts
- docs/design/session-control-and-live-events/README.md
- web/packages/agenta-ui/src/RichChatInput/plugins/SendButton.tsx
- web/packages/agenta-chat/tests/unit/model/userStop.test.ts
- web/packages/agenta-chat/src/assets/agentTurn.ts
- web/oss/src/components/AgentChatSlice/assets/stopState.test.ts
- web/mobile/src/features/chat/StopButton.tsx
- web/oss/src/components/AgentChatSlice/assets/stopState.ts
- web/packages/agenta-ui/src/RichChatInput/RichChatInput.tsx
- web/packages/agenta-entities/tests/unit/session-cancel-stream.test.ts
- web/packages/agenta-chat/src/model/userStop.ts
- web/packages/agenta-chat/src/hooks/useAgentConversation.ts
- api/oss/src/core/sessions/streams/dtos.py
- web/packages/agenta-chat/src/assets/composerState.ts
- web/packages/agenta-chat/src/components/ChatComposer.tsx
- web/packages/agenta-chat/tests/unit/assets/agentTurn.test.ts
- api/oss/src/core/sessions/streams/types.py
- web/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsx
- web/packages/agenta-chat/src/state/sessionEphemera.ts
- web/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.ts
- api/oss/tests/pytest/unit/sessions/test_cancel_cancels_pending_interactions.py
- api/oss/src/apis/fastapi/sessions/router.py
- web/packages/agenta-entities/src/session/api/api.ts
- docs/design/session-control-and-live-events/decisions.md
- docs/design/session-control-and-live-events/tonight-handoff.md
- web/packages/agenta-chat/src/assets/transcriptToMessages.ts
- web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review.
🛑 Comments failed to post (10)
.agents/skills/agent-release-gate/resources/qa_product.py (1)
3166-3173: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Report the recorded SKIP cells;
PASScurrently hides them.
statusisPASSwhenever no cell failed, so an artifact where every cell SKIPped recordsPASS. Eight of the sixteen cells SKIP whenever the standalone run omits--project(see SKILL.md lines 207-212). The loader collectsskipped, but no consumer reads it: the "where" line (Line 3456), the mandatory table (Line 3561), and the exit code (Line 3595) all usestatusonly. A release run without--projecttherefore printsrecorded PASSfor coverage that never executed.Surface the skip count where the status is displayed.
🔧 Proposed fix: carry the skip count into the displayed status
failed = sorted(name for name, status in statuses.items() if status == "FAIL") skipped = sorted(name for name, status in statuses.items() if status == "SKIP") return { "path": str(result_path), "status": "FAIL" if failed else "PASS", + # A SKIP is untested coverage, never a pass. The label the gate prints has to say so, + # or a run without --project reads as green on eight cells that never executed. + "label": ("FAIL" if failed else "PASS") + + (f" ({len(skipped)}/{len(statuses)} SKIPPED)" if skipped else ""), "failed": failed, "skipped": skipped, }Then print
session_control_result['label']at Lines 3456 and 3561.api/oss/src/utils/env.py (1)
528-532: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject non-positive redelivery settings.
SessionsRecordsConfigaccepts zero and negative integers without validation, andworker_streamspasses them toRecordsWorker. Withreclaim_idle_ms=0,reclaim_batch()disables its cooldown and Redis idle filter, so failed records can be reclaimed in a tight loop. Withmax_deliveries <= 0, every reclaimed record is over budget; after an unrelated commit makes the write path healthy,drop_expired()ACKs and deletes the failed record. Add model-level validation requiring both values to be at least1.docs/design/session-control-and-live-events/records-invariants.md (1)
198-200: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventions /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learningsLength of output: 28313
🏁 Script executed:
#!/bin/bash set -eu file="docs/design/session-control-and-live-events/records-invariants.md" printf '%s\n' '--- target document ---' sed -n '90,125p;185,235p' "$file" printf '%s\n' '--- related files ---' git ls-files | rg '(^|/)(records|session|live|event|worker|invariant)' | head -200 printf '%s\n' '--- acknowledgement and drop references ---' rg -n -i 'acknowledge|acknowledg|deliberate.?drop|permanent.?drop|retryable|commit.?scoped|failed database|incomplete.?history' --glob '!**/node_modules/**' .Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash set -eu doc="docs/design/session-control-and-live-events/records-invariants.md" printf '%s\n' '--- document ---' sed -n '100,122p;190,230p' "$doc" printf '%s\n' '--- scoped files ---' git ls-files 'docs/design/session-control-and-live-events/**' 'services/**' 'api/**' | rg -i 'record|event|session|worker|queue|redis|live' printf '%s\n' '--- scoped references ---' rg -n -i 'commit.?scoped|deliberate.?drop|permanent.?drop|retryable|failed database|acknowledg|incomplete.?history' \ docs/design/session-control-and-live-events services api \ --glob '!**/*.jsonl' --glob '!**/*.json' --glob '!**/node_modules/**' \ --glob '!**/dist/**' --glob '!**/build/**' | head -300Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- records worker ---' sed -n '45,85p;220,355p' api/oss/src/tasks/asyncio/sessions/records_worker.py printf '%s\n' '--- consumer acknowledgement contract ---' sed -n '175,285p;305,350p' api/oss/src/tasks/asyncio/shared/consumer.py printf '%s\n' '--- worker durability tests ---' sed -n '1,185p;235,285p;400,445p' api/oss/tests/pytest/unit/sessions/test_records_worker_durability.py printf '%s\n' '--- records acknowledgement design slice ---' sed -n '1,82p;110,182p' docs/design/session-control-and-live-events/slice-records-ack.md printf '%s\n' '--- current diff for target document ---' git diff -- docs/design/session-control-and-live-events/records-invariants.mdRepository: Agenta-AI/agenta
Length of output: 32461
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- exact drop handling ---' sed -n '260,315p' api/oss/src/tasks/asyncio/shared/consumer.py sed -n '330,395p' api/oss/src/tasks/asyncio/sessions/records_worker.py printf '%s\n' '--- history-gap contract ---' sed -n '1,35p;145,175p;212,225p' docs/design/session-control-and-live-events/records-invariants.md rg -n -i 'incomplete|gap|dropped|drop|loss' docs/design/session-control-and-live-events api/oss/src/tasks/asyncio/sessions/records_worker.py api/oss/src/tasks/asyncio/shared/consumer.py --glob '!*.json' --glob '!*.jsonl' | head -160Repository: Agenta-AI/agenta
Length of output: 17832
Define acknowledgement and loss handling consistently.
The worker acknowledges committed records, undecodable messages, and over-quota drops. Retryable database and quota failures remain pending for reclamation. Replace the absolute “only successfully committed” rule with these cases, and require intentional or unrecoverable drops to mark history incomplete before acknowledgement. Otherwise, the contract does not distinguish accounted loss from silent data loss.
docs/design/session-control-and-live-events/rfc.md (2)
304-313: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learnings /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventionsLength of output: 21940
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- candidate files ---' git ls-files | rg '(^|/)(requirements\.md|rfc\.md)$|session-control-and-live-events' printf '%s\n' '--- RFC references ---' rg -n -C 5 'stopp|terminal|settle|watchdog|lost|claim|Redis|Stream|acknow|delet|execution_id|generation' docs/design/session-control-and-live-events/rfc.md printf '%s\n' '--- requirements references ---' for f in $(git ls-files | rg '(^|/)requirements\.md$|session-control-and-live-events'); do printf '\n--- %s ---\n' "$f" rg -n -C 5 'exactly one|terminal|settle|stale|writer|watchdog|lost|idempot|claim|ownership|generation' "$f" || true doneRepository: Agenta-AI/agenta
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -eu f=docs/design/session-control-and-live-events/rfc.md printf '%s\n' '--- RFC terminal/race contract ---' rg -n -C 8 'terminal outcome|terminal settlement|settle|settlement|compare-and-set|compare|CAS|late|duplicate|idempot|race|watchdog|runner disappears|lost|failed|stopped' "$f" printf '%s\n' '--- requirements and decisions contract ---' for f in \ docs/design/session-control-and-live-events/requirements.md \ docs/design/session-control-and-live-events/decisions.md \ docs/design/session-control-and-live-events/records-invariants.md; do printf '\n--- %s ---\n' "$f" rg -n -C 6 'exactly one|terminal outcome|terminal|settle|settlement|late|duplicate|idempot|race|watchdog|lost|stopped|failed' "$f" doneRepository: Agenta-AI/agenta
Length of output: 31978
Define an atomic terminal claim for each execution.
The runner and watchdog can both settle the same execution, but the RFC does not define race arbitration or handling for late reports. Add an execution-keyed compare-and-set rule, make losing reports idempotent, and add a runner/watchdog race test.
331-333: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventions /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learningsLength of output: 7556
🏁 Script executed:
#!/bin/bash set -e printf '%s\n' '--- candidate files ---' fd -t f . docs/design/session-control-and-live-events | sort printf '%s\n' '--- RFC structure and target context ---' rg -n -C 4 'Redis Stream|stream entries|acknowledge|acknowledged|deleted|consumer group|consumer-group|live reader|durable projector|projector|watchdog|terminal|settle|ownership|generation|fenc' docs/design/session-control-and-live-events/rfc.md printf '%s\n' '--- requirements references ---' if [ -f docs/design/session-control-and-live-events/requirements.md ]; then rg -n -C 4 'durable terminal|terminal outcome|stale|writer|ownership|generation|fenc|project|stream|acknowledge|delete' docs/design/session-control-and-live-events/requirements.md fiRepository: Agenta-AI/agenta
Length of output: 12842
🏁 Script executed:
#!/bin/bash set -e for f in \ docs/design/session-control-and-live-events/rfc.md \ docs/design/session-control-and-live-events/records-invariants.md \ docs/design/session-control-and-live-events/slice-records-ack.md \ docs/design/session-control-and-live-events/requirements.md \ docs/design/session-control-and-live-events/research.md do printf '\n--- %s: matching terms ---\n' "$f" rg -n -C 3 'Redis|Stream|XADD|XREAD|XREADGROUP|XACK|acknowledge|delete|retention|consumer|reader|projector|temporary|durable|replay|pending|claim' "$f" || true done printf '\n--- RFC live-ingress and replay sections ---\n' sed -n '329,420p' docs/design/session-control-and-live-events/rfc.md printf '\n--- record acknowledgement slice ---\n' sed -n '1,260p' docs/design/session-control-and-live-events/slice-records-ack.mdRepository: Agenta-AI/agenta
Length of output: 50374
Define Redis Stream acknowledgement and deletion ownership.
The RFC does not state whether live readers and the durable projector use separate consumer groups or who may acknowledge and delete entries. If a live reader deletes an entry before projector commit, a disconnect or retry can create a durable history gap. Define retention, acknowledgement, and deletion rules. Delete entries only after durable projection commits.
docs/design/session-control-and-live-events/slice-stop-guard.md (1)
186-189: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Clear the guard when the newest assistant message has no turn ID.
useAgentConversationclears the guard before sending, but itslatestTurnIdeffect scans the preceding assistant message and restores its ID. If the next turn has no metadata, Stop sends that staleexpected_execution_idand can be rejected. Update the effect to callclearSessionTurnIdwhenlatestTurnIdreturnsnull, and extend the second-send regression test with metadata on the first turn only.🧰 Tools
🪛 LanguageTool
[style] ~186-~186: To elevate your writing, consider using more formal language here.
Context: ...ssion's first turn carried the metadata and a later turn did not, the stored id wou...(AND_WHEREAS)
[style] ~189-~189: Consider an alternative for the overused word “exactly”.
Context: ...he guard, because retrying unguarded is exactly the behavior#6417is about. ## The ho...(EXACTLY_PRECISELY)
services/runner/src/lifecycle/session-coordinator.ts (1)
1337-1338: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
The backstop refusal carries no stable error code.
This branch returns only
error: SESSION_TURN_IN_USE_MESSAGE. It emits noerrorevent, soSESSION_TURN_IN_USE_CODEnever reaches the client on this path. Two consequences follow in the fail-open window this branch exists for:
- The client cannot key on
session_turn_in_useand renders the refusal as a generic run failure instead of "not sent, try again".server.ts(Lines 637-639) assumes a failed engine run already emitted its own error event. This branch breaks that assumption, so nothing is persisted and the terminal result is the only signal.Emit the coded error event here, as the edge gate does.
♻️ Proposed change
klog(`refuse (busy) key=${key}; another turn owns this session`); + trackedEmit?.({ + type: "error", + message: SESSION_TURN_IN_USE_MESSAGE, + code: SESSION_TURN_IN_USE_CODE, + }); return { ok: false, error: SESSION_TURN_IN_USE_MESSAGE };This needs
SESSION_TURN_IN_USE_CODEadded to the Line 43 import.web/mobile/src/features/chat/LiveConversation.tsx (1)
290-294: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Retry the accepted Stop request automatically. The watchdog only sets
retryStopRefand clearsstoppingHere; it never sends the required retry, so a non-terminating accepted cancellation remains active until the user clicks Stop again.web/oss/src/components/AgentChatSlice/AgentConversation.tsx (1)
444-445: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Restore the complete refused payload. A session-busy refusal loses attached files: desktop discards file-only messages and restores only text, while mobile clears the submission with no refusal recovery.
web/oss/src/components/AgentChatSlice/AgentConversation.tsx#L444-L445: recover or requeue the fullQueuedMessage, includingfileParts.web/mobile/src/features/chat/LiveConversation.tsx#L598-L598: detect the session-busy refusal and restore or requeue text and file parts before the composer clears them.📍 Affects 2 files
web/oss/src/components/AgentChatSlice/AgentConversation.tsx#L444-L445(this comment)web/mobile/src/features/chat/LiveConversation.tsx#L598-L598web/packages/agenta-entities/src/workflow/state/store.ts (1)
1488-1488: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash set -eu file='web/packages/agenta-entities/src/workflow/state/store.ts' printf '%s\n' '--- focused source ---' sed -n '1,35p' "$file" sed -n '1440,1545p' "$file" printf '%s\n' '--- related tests and symbols ---' rg -n -C 4 'readBuildKitUiState|buildKitUiStateByRevisionAtom|atomWithStorage|malformed|null|reload|revision' \ web/packages/agenta-entities/src/workflow/state/store.ts \ web/packages/agenta-entities -g '*test*' -g '*spec*' | head -240Repository: Agenta-AI/agenta
Length of output: 28222
🤖 get_repo_knowledge executed:
get_repo_knowledge Agenta-AI/agenta /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/learnings /tmp/coderabbit-repo-knowledge/agenta-ai-agenta-4b53879a/conventionsLength of output: 44747
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- build-kit references and tests ---' rg -n -C 8 'buildKitUiState|BuildKitUiState|build-kit|disabledOps' \ web/packages/agenta-entities web/packages -g '*test*' -g '*spec*' -g '*.tsx' -g '*.ts' \ | head -320 printf '%s\n' '--- Jotai dependency declarations ---' rg -n -C 3 '"jotai"|jotai@' \ package.json web/package.json web/packages/*/package.json pnpm-lock.yaml 2>/dev/null \ | head -120Repository: Agenta-AI/agenta
Length of output: 39785
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- persistence test ---' cat -n web/packages/agenta-entities/tests/unit/agent-build-kit-ui-state-atom.test.ts printf '%s\n' '--- agenta-entities package metadata ---' cat -n web/packages/agenta-entities/package.json | sed -n '1,140p' printf '%s\n' '--- resolved Jotai entries ---' rg -n -m 12 '(^|/ )jotai(@|:)|jotai:' pnpm-lock.yamlRepository: Agenta-AI/agenta
Length of output: 13044
Guard the persisted value before indexing it: JSON
nullmakesreadBuildKitUiStatethrow; treat it as empty and add the reload regression case.
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
CHANGES NEEDED
The atomic Stop displacement fix is sound: ownership validation, tombstones, and lock deletion happen in one Redis EVAL, and the new test replaces the owner immediately before that EVAL and proves the replacement survives. The userStop import boundary is also local to @agenta/chat and its focused test passes. The LiveConversation rebase retained the pending-task and blocked-model changes from its base.
Four frontend state races remain in the inline comments. In addition, feat/session-control has advanced to 40e8f80; GitHub currently reports this PR as CONFLICTING, with prospective conflicts in api/oss/src/core/sessions/streams/service.py and two session-control design documents.
AGENTA_SESSIONS_DURABLE_STOP is not present in this head or the fetched base, so its off-path cannot be established from this PR. At this head both clients still use the legacy /sessions/streams command endpoint, while the desktop and mobile presentation behavior intentionally changes to server-first Stop. The later durable-cancel flag adapter is outside this reviewed diff and needs an integration-level flag-off check.
The turn id arrives as a `message-metadata` chunk on `message.metadata.turnId`, not as a `data-agent-turn` part. Updates the mechanism, the path:line table, the runner commit, and adds why the store stays in memory: metadata round-trips through the message cache, so a persisted id would name a turn from a past page load. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
… a callback The pinned ai@6.0.0-beta.150 exposes exactly four client chat callbacks — onError, onToolCall, onFinish and onData — and no metadata hook. onData takes a DataUIPart, so it never sees a `message-metadata` chunk, and onFinish is too late for a Stop that happens mid-turn. `messageMetadataSchema` is a validation schema, not a hook. Reading the merged metadata off the streaming message is the only channel this version offers. Verified in the installed package. Claude-Session: https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV
Move ownership validation, tombstoning, and lock removal into one Redis operation. Use the Redis clock for both cancellation arrival and turn acquisition, and keep accepted cancellation independent from approval cleanup. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep the Stop predicate inside the chat package dependency boundary. Close cancelled approval replays, clear stale execution guards before new turns, and require cancellation evidence from the API response. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Settle parked Stop only after the server accepts cancellation. Ignore stale mobile responses after a session switch and keep idle state neutral on desktop and mobile. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Document the Postgres and Redis transaction boundary, the id-less arrival guard, and capability-qualified warm resume. Remove environment-specific identifiers and unsafe permission guidance from the verification record. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Update heartbeat race coverage to assert behavior at the new atomic Redis boundary. Preserve the same-turn ownership assertion without patching removed displacement helpers. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep the desktop Stop comments focused on the execution guard and accepted-cancellation invariants. Remove protocol history already captured in the design documents. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Clear cached execution identifiers at every desktop turn entry point and at the shared transport boundary so automatic approval and client-tool resumes cannot inherit the parked turn guard. Add shared approval-resume and desktop hook regression coverage. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Track the stopped turn identity across transcript updates. Preserve the local latch while the same turn settles, but clear it when watch or revalidation adopts a newer sent or resumed turn. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Evaluate both the request-time and response-time HITL state after cancellation succeeds. This preserves the stopped presentation when a streaming run becomes approval-paused before the response arrives. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Gate approval, elicitation, and connection actions on the Stop request phase for desktop and mobile. Failed requests restore the actions when the stopping state returns to idle. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep the mocked transcript and reducer identities stable so the desktop hook regression does not schedule an artificial render loop in the complete OSS suite. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Apply the OSS import grouping required by the package lint configuration. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep the missing-start guard focused on an actively running legacy turn and format the durable reconciliation coverage after moving atomic cleanup onto the durable path. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
Keep one guard-clearing wrapper around the aliased chat send and regenerate methods so hook initialization and immediate-Stop fencing both remain correct. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
0344c3e to
fff1540
Compare
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
CHANGES NEEDED
The four round-two fixes and their regression tests survived the rebase. The range-diff accounts for every old feature change; the apparent generated/schema drops are already in the base. The public cancel route still feature-gates durable Stop, preserves the flag-off legacy call, and both legacy displacement and durable settlement are atomic.
One refresh gap remains: accepted Stop state is mount-local, so a paused approval is re-enabled if the page reloads before cancellation settles.
Focused validation: @agenta/chat useAgentConversation.test.ts — 11 passed.
Preserve the backend stopping turn marker through the frontend session schema. Recover matching desktop and mobile stop guards across remounts until settlement clears the marker. Claude-Session: https://claude.ai/code/session_0164kzT6ttwpBtzvcDC6YzYk
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Agent-generated Codex review, low weight.
SHIP
8a8b2ae7f825 closes the remount gap on desktop and mobile. The durable guard applies only when stopping_turn_id matches the current turn_id; settlement clears that marker, and a newer turn makes an older marker non-owning. With durable Stop disabled, no marker is written and the existing local Stop lifecycle remains in control.
Focused validation: useAgentChatSession.test.ts — 2 passed, including pause → accepted Stop → remount → settlement.
1 participant
Context
Pressing Stop in the desktop chat marked the turn as stopped and cut the stream before the server had accepted the request. If the request failed, the warning showed but the turn stayed "stopped" and the stream was gone, while the runner kept working and billing. The same pattern existed in the mobile chat. This is the client half of the Stop package in the session-control design (PR #6495); the original slice on this branch added the execution guard and the approval handling.
Changes
The desktop Stop now has three states. On click the composer shows "Stopping" with the stream still attached. On 200 or 202 it stays in "Stopping" until the stream's terminal event or the session's terminal state arrives, then "Stopped". A response with an empty
cancelled_turn_ids(the session was already idle) goes straight to idle. A failed request (network, 5xx, 409 execution_mismatch) restores the running state, keeps the stream, and shows the warning with the server's error message.An accepted Stop that never terminates does not trap the button: after 30 s the button re-enables and a second click sends Stop again with the same expected execution id. Only after an accepted retry does the client fall back to a local abort.
Mobile
LiveConversationfollows the same rule. The mobileStopButtonis unchanged.Before: click →
stopped=true, stream aborted, request in flight; on failure a warning and a dead stream.After: click → "Stopping", stream attached; on failure the running state is back and the stream continues.
Tests
@agenta/ossstopState.test.ts: 7 passed; full package 430 passed, 1 skipped.@agenta/entitiessession-cancel-stream.test.ts: 9 passed; full package 1,473 passed.@agenta/mobile: 144 passed; production build 6/6.cancelled_turn_ids, which the API does send.What to QA
Browser pass, 2026-09-04
Stack agenta-ee-dev-session-integration at
8e746d66f1, Pi on the local sandbox, desktop at 8580 and mobile at /m. Fifty screenshots under~/agenta-qa-evidence/2026-09-04-browser-stop/.sleepreturned 202 and the UI showed Stopped in 1 to 2 s; but in a clean session a codeword committed in an EARLIER completed turn was not recalled afterwards (session d602e8bf, screenshots 38 to 45). Root cause in progress; the QA driver missed it because it replays the transcript while the desktop sends only the last messageAlso: the same user Stop sometimes renders as neutral "Stopped / Resend" and sometimes as the red failure card (screenshots 21, 37, 44). Fixes for the missing Stop during approval, the card not updating on cancel, and the styling are being prepared on this branch.
Agent-generated, low weight. Not merged.
https://claude.ai/code/session_01GAqSs7fw6QRi2n1ZJ2tmAV