fix(channels): an approval answer resumes the parked turn, and reaches the agent that asked - #6650
fix(channels): an approval answer resumes the parked turn, and reaches the agent that asked#6650mmabrouk wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 SummarySummary by CodeRabbit
WalkthroughChannel approval choices now retain their parked interaction identifiers. Channel routing resolves clicks and typed labels, and inbox dispatch forwards answers to parked sessions without opening a new turn. ChangesChannel approval responses
Priority: ➖ Normal — Schedule the channel approval-response change because it spans API, workers, routing, persistence, and session resumption, while card rendering still defers the first release. Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟠 High · up to Approval responses can resume the wrong interaction, resume one interaction twice, or fail to resume it at all. These paths should be corrected before enabling channel approvals. Sequence Diagram(s)sequenceDiagram
participant Channel
participant ChannelsService
participant ChannelsDAO
participant InboxDispatcher
participant InteractionsDispatcher
Channel->>ChannelsService: submit approval click or typed label
ChannelsService->>ChannelsDAO: find thread awaiting choice
ChannelsDAO-->>ChannelsService: pending choice
ChannelsService-->>InboxDispatcher: resolved interaction answer
InboxDispatcher->>InteractionsDispatcher: respond_many
InboxDispatcher->>ChannelsDAO: clear pending choice
🚥 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 |
|
Live QA on the stack found the outbound half of approvals is blocked by a sessions-layer gap, not this lane. What this lane fixes (the inbound/return half) is unit- and integration-tested and correct: a click, a typed "Approve", or a numbered reply answers the parked session interaction through the sessions respond path and opens no new turn. The gap: when an Fix (a separate, sessions-domain change): publish a park signal to |
|
Codex (astra, medium, simplify) reviewed this lane: "request changes" on answer selection. Applied now: the awaiting-choice DAO lookup excludes a cleared pending choice (stored as JSON null, which passed a bare IS NOT NULL) and inactive rows before the limit, so a newer cleared or closed thread can't hide an older one still waiting. Two edge cases Codex raised are recorded as follow-ups, since approvals are deferred from the first cut until the outbound card-render-on-park gap is wired: (1) bind a button's value to its interaction id so a stale card can't answer a newer interaction; (2) an atomic claim so two clicks racing before the choice clears can't both respond. Both only bite once the card actually renders live, which it does not yet. @coderabbitai review |
|
|
4f7c9d6 to
a6e8a40
Compare
|
Superseded context: the feature-flag lane is #FLAG (stacked on this). Linking once created. |
…real interaction id This completes the approval loop (F100/F101), which PR #6650 left with a working answer path but no visible card. - A parked turn never calls complete_turn, so the channels outbox was never told to fold it and the card never drew. The record-ingress now publishes a turn-ended to streams:sessions on every terminal record. That stream is consumed only by the channels outbox, and the outbox keys its output by (turn_id, index), so a turn that also ends through complete_turn just edits the same message instead of posting twice. It also covers the approval continuation, a detached run that bypasses complete_turn the same way. - The outbox stored the fold's ACP token as the pending choice's interaction id; the sessions respond path answers by the SessionInteraction row id. The outbox now resolves the row id for the turn's open interaction (matched on the token) before storing it. Verified live on the Agenta channel: the card renders, an "approve" reply resumes the parked turn, the approved tool runs, and the agent's answer ("Done! I've created hello.txt...") posts back. A normal turn still answers exactly once, no duplicate. Claude-Session: https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT
…s the agent that asked
The approval card rendered, and a click started a new turn with the
word "Approve" in it, so the runner kept waiting for its answer and
Slack sat on the card (F100, F101). Now:
- The pending choice the outbox stores with the card carries the parked
interaction's id. A click, a typed "Approve", or a numbered reply that
resolves against it makes the dispatcher answer the interaction
through the sessions respond path, the same path a playground click
uses, and no new turn opens. The continuation's turn events reach the
outbox through the thread's session as any turn does. The pending
choice is cleared once answered.
- A typed answer carries no agent, and the room's default was the wrong
one when another agent asked. The thread that holds the open question
now names the agent, whichever it is.
- A typed answer in the agent's own words ("Approve", "deny") resolves
like a click; the numbered reply still works.
- Both composition roots, the API process and the queue worker, wire the
answer path through their interactions dispatcher.
Claude-Session: https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT
…-choice lookup Codex review of the approval lane: a cleared pending choice is stored as JSON null, which passes a bare IS NOT NULL, and an inactive row could win LIMIT 1 and then be rejected in Python. Filter active rows and a real (non-JSON-null) pending choice before the limit, so a newer cleared or closed thread never hides an older one that is still waiting. Two further edge cases Codex raised stay as follow-ups, since approvals are deferred from the first cut until the park signal that renders the card is wired: binding a button's value to its interaction id so a stale card cannot answer a newer interaction, and an atomic claim so two clicks racing before the choice clears cannot both respond. Claude-Session: https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT
…real interaction id This completes the approval loop (F100/F101), which PR #6650 left with a working answer path but no visible card. - A parked turn never calls complete_turn, so the channels outbox was never told to fold it and the card never drew. The record-ingress now publishes a turn-ended to streams:sessions on every terminal record. That stream is consumed only by the channels outbox, and the outbox keys its output by (turn_id, index), so a turn that also ends through complete_turn just edits the same message instead of posting twice. It also covers the approval continuation, a detached run that bypasses complete_turn the same way. - The outbox stored the fold's ACP token as the pending choice's interaction id; the sessions respond path answers by the SessionInteraction row id. The outbox now resolves the row id for the turn's open interaction (matched on the token) before storing it. Verified live on the Agenta channel: the card renders, an "approve" reply resumes the parked turn, the approved tool runs, and the agent's answer ("Done! I've created hello.txt...") posts back. A normal turn still answers exactly once, no duplicate. Claude-Session: https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT
a6e8a40 to
765cc6b
Compare
|
@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: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 45219e19-e342-447e-9b9f-d1b6bc4b7745
📒 Files selected for processing (14)
api/entrypoints/routers.pyapi/entrypoints/worker_queues.pyapi/oss/src/core/channels/dtos.pyapi/oss/src/core/channels/interfaces.pyapi/oss/src/core/channels/render/dtos.pyapi/oss/src/core/channels/render/render.pyapi/oss/src/core/channels/service.pyapi/oss/src/dbs/postgres/channels/dao.pyapi/oss/src/tasks/asyncio/channels/inbox.pyapi/oss/src/tasks/asyncio/channels/outbox.pyapi/oss/tests/pytest/unit/channels/test_channels_inbox_dispatcher.pyapi/oss/tests/pytest/unit/channels/test_channels_outbox_worker.pyapi/oss/tests/pytest/unit/channels/test_channels_render.pyapi/oss/tests/pytest/unit/channels/test_channels_service_routing.py
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
| async def _dispatch_detached_run(*, project_id, user_id, request) -> str: | ||
| result = await workflows_service.invoke_workflow_detached( | ||
| project_id=project_id, | ||
| user_id=user_id, | ||
| request=request, | ||
| ) | ||
| return result.run_id | ||
|
|
||
| # A channel's approval answer resumes the parked session the way a | ||
| # playground click does: through the interactions dispatcher. | ||
| interactions_dispatcher = InteractionsDispatcher( | ||
| workflows_service=workflows_service, | ||
| interactions_service=SessionInteractionsService( | ||
| interactions_dao=SessionInteractionsDAO(engine=transactions_engine), | ||
| watch_publisher=SessionsWatchPublisher(), | ||
| ), | ||
| records_service=RecordsService( | ||
| records_dao=RecordsDAO(engine=get_analytics_engine()), | ||
| ), | ||
| dispatch_fn=_dispatch_detached_run, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 'dispatch_fn|_dispatch_detached_run|InteractionsDispatcher\(|turns_service|streams_service' \
api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.py \
api/entrypoints/routers.py \
api/entrypoints/worker_queues.pyRepository: Agenta-AI/agenta
Length of output: 31573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- dispatcher contract ---'
sed -n '337,490p' api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.py
printf '%s\n' '--- worker callback and compositions ---'
sed -n '230,365p' api/entrypoints/worker_queues.py
printf '%s\n' '--- detached invocation definitions ---'
rg -n -C 8 'def invoke_workflow_detached|async def invoke_workflow_detached|invoke_workflow_detached\(' \
api/oss api/entrypoints -g '*.py'Repository: Agenta-AI/agenta
Length of output: 24373
🤖 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/conventions
Length of output: 15180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- reference resolution implementation ---'
rg -n -C 35 'def resolve_interaction_references|async def resolve_interaction_references' \
api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.py
printf '%s\n' '--- interaction data/reference producers ---'
rg -n -C 12 'SessionInteractionData\(|references\s*=|continuation_execution_id|target_turn_id' \
api/oss/src/tasks/asyncio api/entrypoints \
-g '*.py' | head -n 420Repository: Agenta-AI/agenta
Length of output: 198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'resolve_interaction_references' api/oss/src/tasks/asyncio/sessions api/oss/src -g '*.py'Repository: Agenta-AI/agenta
Length of output: 11793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- reference fallback contract ---'
sed -n '1,150p' api/oss/src/core/sessions/interactions/references.py
printf '%s\n' '--- session service construction in worker entrypoint ---'
rg -n -C 12 'SessionTurnsService|SessionStreamsService|session_turns|session_streams' \
api/entrypoints/worker_queues.py api/entrypoints/routers.pyRepository: Agenta-AI/agenta
Length of output: 25294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '2920,3055p' api/oss/src/core/workflows/service.pyRepository: Agenta-AI/agenta
Length of output: 5237
Wire session reference services into the channel dispatcher.
At api/entrypoints/worker_queues.py:267, InteractionsDispatcher receives no turns_service or streams_service. For interactions without stored references, resolve_interaction_references therefore returns no workflow references, and invoke_workflow_detached raises WorkflowServiceUrlMissing instead of resuming the session. Add the corresponding session services used by the API composition.
| async def fetch_thread_awaiting_choice( | ||
| self, | ||
| *, | ||
| project_id: UUID, | ||
| # | ||
| space_id: UUID, | ||
| external_key: Optional[UUID], | ||
| ) -> Optional[ChannelThread]: | ||
| """The active thread under this key, whichever agent holds it, that | ||
| has a pending choice; None when nothing is waiting for an answer.""" | ||
| return None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make fetch_thread_awaiting_choice abstract.
ChannelsDAO implements the query, but FakeChannelsDAO inherits the current return None fallback. Any such DAO passed to ChannelsService can make _agent_awaiting_answer() skip pending-choice routing for typed answers without a construction error. Add @abstractmethod and explicit implementations to affected interface fakes.
| tool = pending_interaction.get("tool") | ||
| title = f"Approval needed: {tool}" if tool else "Approval needed" | ||
| interaction_id = pending_interaction.get("id") | ||
| interaction_id = str(interaction_id) if interaction_id else None |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 'pending_interaction|interaction_id|UUID\(interaction_id\)' \
api/oss/src/core/sessions \
api/oss/src/tasks/asyncio/channels \
api/oss/tests/pytest/unit/channelsRepository: Agenta-AI/agenta
Length of output: 50372
🤖 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/conventions
Length of output: 42478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- render.py ---'
sed -n '1,180p' api/oss/src/core/channels/render/render.py
printf '%s\n' '--- channel DTO definitions ---'
rg -n -C 8 'class ChannelPendingChoice|class Channel.*Item|interaction_id' api/oss/src/core/channels
printf '%s\n' '--- interaction record and fold producers ---'
rg -n -C 8 'entity_id|interaction_request|pending_interaction|SessionInteraction|class Interaction' \
api/oss/src/core/sessions/records api/oss/src/core/sessions \
-g '*.py' | head -n 260Repository: Agenta-AI/agenta
Length of output: 35643
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- pending_interaction construction ---'
rg -n -C 12 'pending_interaction\s*=|["'\'']pending_interaction["'\'']|interaction_request|interaction_id' \
api/oss/src/core/channels api/oss/src/core/sessions \
-g '*.py' | head -n 360
printf '%s\n' '--- UUID interaction model and persistence types ---'
rg -n -C 8 'class SessionInteraction|id: UUID|interaction_id: UUID|respond_interaction|UUID\(interaction_id\)' \
api/oss/src/core/sessions api/oss/src/core/channels \
-g '*.py' | head -n 320Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- pending_interaction construction ---'
rg -n -C 12 'pending_interaction|interaction_request|interaction_id' \
api/oss/src/core/channels api/oss/src/core/sessions \
-g '*.py' | head -n 360
printf '%s\n' '--- UUID interaction model and persistence types ---'
rg -n -C 8 'class SessionInteraction|id: UUID|interaction_id: UUID|respond_interaction|UUID\(interaction_id\)' \
api/oss/src/core/sessions api/oss/src/core/channels \
-g '*.py' | head -n 320Repository: Agenta-AI/agenta
Length of output: 50372
Validate the interaction ID before rendering the approval card.
render_turn_result converts any truthy pending_interaction["id"] to a string. InboxDispatcher._answer_interaction later passes that value to UUID(interaction_id). If the ID is not UUID-formatted, dispatch raises before clearing the pending choice, so the approval remains pending. Reject invalid IDs before creating the resumable approval card, or enforce a UUID invariant for every paused interaction.
| waiting = await self.channels_dao.fetch_thread_awaiting_choice( | ||
| project_id=project_id, | ||
| space_id=space.id, | ||
| external_key=thread_key, | ||
| ) | ||
| if waiting is None: | ||
| return None | ||
| token = resolve_pending_choice( | ||
| pending_choice=waiting.data.pending_choice, candidate=candidate | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🤖 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/conventions
Length of output: 9981
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- service resolve slice ---'
sed -n '1420,1555p' api/oss/src/core/channels/service.py
printf '%s\n' '--- pending-choice definitions and uses ---'
rg -n -C 4 'def resolve_pending_choice|resolve_pending_choice|pending_choice|answered_interaction_id|interaction_id' api/oss/src/core/channels api/oss/src/tasks/asyncio/channels api/oss/tests/pytest/unit/channels/test_channels_service_routing.py | head -n 500Repository: Agenta-AI/agenta
Length of output: 46287
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- service resolve slice ---'
sed -n '1420,1555p' api/oss/src/core/channels/service.py
printf '%s\n' '--- direct definitions and callers ---'
rg -n -C 5 'def resolve_pending_choice|resolve_pending_choice|pending_choice|answered_interaction_id|interaction_id' api/oss/src/core/channels api/oss/src/tasks/asyncio/channels api/oss/tests/pytest/unit/channels/test_channels_service_routing.pyRepository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- channel adapter action mapping ---'
rg -n -C 6 'ChannelEventKind\.ACTION|kind=.*ACTION|processed\.content|action_id|interaction_id|choice.*token|button|callback' api/oss/src/core/channels api/oss/src/tasks/asyncio/channels | head -n 500
printf '%s\n' '--- exact superseded-choice test ---'
sed -n '560,680p' api/oss/tests/pytest/unit/channels/test_channels_service_routing.py
printf '%s\n' '--- event DTOs ---'
rg -n -C 8 'class Channel(Inbox|Event)|class .*Event|processed:|external_locator|ChannelEventKind' api/oss/src/core/channels/dtos.py api/oss/src/core/channelsRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- action event and adapter definitions ---'
fd -t f -i 'adapter|slack|discord|telegram|event|ingress' api/oss/src/core/channels api/oss/src | head -n 120
rg -n -C 5 'ChannelEventKind|processed\.content|action_id|callback_id|interaction_id|button|callback|choice\.token' api/oss/src/core/channels api/oss/src/integrations api/oss/src/entrypoints 2>/dev/null | head -n 500Repository: Agenta-AI/agenta
Length of output: 41556
Authorization Bypass
Reachability: External
Exploitability: Moderate
CWE: CWE-863 — Incorrect Authorization
Bind each action to its rendered approval card.
Slack action parsing forwards only the button value as the candidate token. The action or card identity is not retained for validation. Since approval cards reuse approve and deny, an old card can resolve the current pending interaction and submit its response.
Persist an immutable card or choice-instance identifier in the rendered action and validate it before setting answered_interaction_id. Extend the superseded-choice test to cover an old approve action after a replacement card also uses approve.
| await self._respond_interaction_fn( | ||
| project_id=project_id, | ||
| user_id=user_id or resolution.agent.created_by_id, | ||
| interaction_id=UUID(interaction_id), | ||
| answer={"approved": approved, "message": resolution.resolved_choice}, | ||
| ) | ||
| # clear the question so the common case (a later card supersedes it) has | ||
| # nothing stale to resolve against; this is not a concurrency guard -- | ||
| # two clicks racing before either clears is a known follow-up (F101). | ||
| await self.channels_service.channels_dao.set_pending_choice( | ||
| project_id=project_id, | ||
| thread_id=resolution.thread.id, | ||
| pending_choice=None, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 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/learnings
Length of output: 24296
🏁 Script executed:
#!/bin/bash
set -eu
file='api/oss/src/tasks/asyncio/channels/inbox.py'
printf '%s\n' '--- target file ---'
sed -n '1,240p' "$file"
printf '%s\n' '--- related definitions and callers ---'
rg -n -C 4 'set_pending_choice|_answer_interaction|respond_interaction|pending_choice|channels_dao' api/oss/srcRepository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- inbox resolution path ---'
sed -n '250,410p' api/oss/src/tasks/asyncio/channels/inbox.py
printf '%s\n' '--- pending-choice contract and implementation ---'
sed -n '450,515p' api/oss/src/core/channels/interfaces.py
sed -n '1175,1235p' api/oss/src/dbs/postgres/channels/dao.py
printf '%s\n' '--- interaction response wiring and contract ---'
rg -n -C 8 'respond_interaction_fn|InboxDispatcher\(|respond_interaction\(' api/oss/src --glob '*.py' | head -240
printf '%s\n' '--- response command implementation ---'
rg -n -C 8 'async def respond_interaction|def respond_interaction|respond_interactions' api/oss/src/core api/oss/src/services api/oss/src/apis/fastapi/sessions api/oss/src/tasks/taskiq/sessions --glob '*.py'Repository: Agenta-AI/agenta
Length of output: 30953
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- interaction worker callback ---'
sed -n '1,85p' api/oss/src/tasks/taskiq/sessions/interactions_worker.py
printf '%s\n' '--- response service implementation ---'
sed -n '611,760p' api/oss/src/core/sessions/commands/service.py
printf '%s\n' '--- response router completion and idempotency handling ---'
sed -n '1338,1455p' api/oss/src/apis/fastapi/sessions/router.py
printf '%s\n' '--- dispatcher construction sites ---'
rg -n -C 12 'InboxDispatcher' api/oss/src --glob '*.py'Repository: Agenta-AI/agenta
Length of output: 21999
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- duplicate-answer handling ---'
sed -n '690,835p' api/oss/src/core/sessions/commands/service.py
printf '%s\n' '--- interaction dispatcher ---'
sed -n '1,220p' api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.py
printf '%s\n' '--- all response callback wiring ---'
rg -n -C 10 'respond_interaction_fn\s*=|respond_interaction_fn=|interactions\.respond|respond_interaction\.kiq|respond_interaction\.kicker' . --glob '*.py' --glob '!**/node_modules/**'Repository: Agenta-AI/agenta
Length of output: 30369
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- channel response callback ---'
sed -n '220,300p' api/entrypoints/worker_queues.py
printf '%s\n' '--- API channel response callback ---'
sed -n '1085,1170p' api/entrypoints/routers.py
printf '%s\n' '--- interactions dispatcher response entrypoints ---'
rg -n 'async def respond|respond_many|idempotency' api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.py
sed -n '300,520p' api/oss/src/tasks/asyncio/sessions/interactions_dispatcher.pyRepository: Agenta-AI/agenta
Length of output: 13709
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- ChannelsService.resolve and pending-choice resolution ---'
rg -n 'async def resolve|answered_interaction_id|pending_choice' api/oss/src/core/channels/service.py api/oss/src/core/channels --glob '*.py'Repository: Agenta-AI/agenta
Length of output: 4682
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '1235,1480p' api/oss/src/core/channels/service.pyRepository: Agenta-AI/agenta
Length of output: 9678
Claim the pending choice before responding.
Two inbox workers can resolve the same pending choice before InboxDispatcher._answer_interaction clears it. The configured callback can then dispatch the same continuation twice because InteractionsDispatcher.respond_many has no atomic claim or idempotency key. Add a service/DAO compare-and-set claim for the expected interaction identifier. Only the successful claimant should invoke the callback. Keep the claim recoverable when the callback fails.
| await self.channels_service.channels_dao.set_pending_choice( | ||
| project_id=project_id, | ||
| thread_id=resolution.thread.id, | ||
| pending_choice=None, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Route the pending-choice write through ChannelsService.
Line 170 bypasses the service layer and calls the DAO directly. Add a ChannelsService.set_pending_choice method and call it from InboxDispatcher.
As per coding guidelines, “Required direction: Router -> Service -> DAO Interface -> DAO Implementation -> DB”.
Source: Coding guidelines
…real interaction id This completes the approval loop (F100/F101), which PR #6650 left with a working answer path but no visible card. - A parked turn never calls complete_turn, so the channels outbox was never told to fold it and the card never drew. The record-ingress now publishes a turn-ended to streams:sessions on every terminal record. That stream is consumed only by the channels outbox, and the outbox keys its output by (turn_id, index), so a turn that also ends through complete_turn just edits the same message instead of posting twice. It also covers the approval continuation, a detached run that bypasses complete_turn the same way. - The outbox stored the fold's ACP token as the pending choice's interaction id; the sessions respond path answers by the SessionInteraction row id. The outbox now resolves the row id for the turn's open interaction (matched on the token) before storing it. Verified live on the Agenta channel: the card renders, an "approve" reply resumes the parked turn, the approved tool runs, and the agent's answer ("Done! I've created hello.txt...") posts back. A normal turn still answers exactly once, no duplicate. Claude-Session: https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT
1 participant
Context
Stacked on #6649. Findings F100 and F101 in the ledger: the approval card rendered in the channel, but a click started a new turn with the word "Approve" in it, so the runner kept waiting for its answer and the card sat there. In a room with several agents, a typed "Approve" also went to the default agent instead of the one that asked.
Changes
Not in this PR
The
askmode under message scope is still not refused at registration; under that scope a card cannot be answered because each message is its own thread. This cut documents it as a limitation; the first release runs DMs and channels in thread scope.Tests
https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT