fix(mcp): chat_with_agent carries the caller's turn so delegated work reports back (#3232)

[AndriiPasternak31]

Fixes #3232

chat_with_agent declared an execution_id argument for report-back (ent#224) but never read it, and the dedicated chat_with_<agent> tools never declared it, so work delegated from a Slack, Telegram or Workspace turn never reported back into that conversation. This PR makes the MCP server send the caller's turn as parent_execution_id on every /task route. It also turns report-back on by default for async delegation and adds a kill switch. The backend inheritance guard is unchanged. This is Refs, not Fixes: a plain sequential call still cannot report back (see below).

Where the parent id comes from, and why

• The platform's turn header comes first. On agent images that send it (feat(pull): platform-injected execution id for the side-effect guard — Tier-6 T6.3, blocks default-ON for side-effect agents #2392), the parent is the X-Trinity-Execution-Id turn the agent is actually serving. A model-typed execution_id is used only when there is no header. Reason: a resumed session can copy a stale id out of its history, and the header names the live turn. It is the same rule the other effect tools already use (resolveExecutionId).
• A typed id is held to the header's own format check, and manual is never forwarded.
• On by default for async delegation (parallel=true, async=true, and the experiment: pull-coordination pilot — route MCP chat_with_agent through the agent queue #946 pull-routed sequential call, which also answers with a receipt). The caller's turn ends with a receipt, so the child's note is usually the person's only news. Pass execution_id="manual" to turn it off for one call. A self-task with inject_result=true is excluded, because it already names its destination.
• Sync calls stay opt-in (pass your own execution_id): the caller answers inline, so a default would double-post on almost every call.
• The tool says what it did. A call that sends a parent gets report_back: "requested" in its result (plus a short note on the default path); an opted-in call that cannot send one gets report_back: "off" with a reason. Gate results, depth refusals and thrown errors are unchanged.
• Invariant Unified Executions Dashboard (EXEC-022) #18 is untouched. The parent is not part of the Idempotency-Key, and the turn header forwarded to the backend is computed exactly as before; a typed manual never clears it.
• Every decision is in one pure helper, resolveReportBack (src/mcp-server/src/tools/chat.ts). It logs one [Report-Back #3232] line per call, so the default's uptake can be measured after deploy.

Behaviour change in customer conversations

• Before: an agent in a Slack thread hands a long job to another agent (parallel=true, async=true) and replies "on it". The job finishes, and nobody in the thread hears about it.
• After: the same call, with no new argument, posts the job's outcome (success or failure) into that thread when it ends. If the delegated agent hands the work on asynchronously, the next hop posts too: one note per async hop.
• Consent controls are unchanged and still apply: Slack channel binding allow_proactive, Telegram group allow_proactive (Telegram DMs are consent by construction), and the Workspace, where the note goes only to the client thread that started the work. Delivery is best-effort: no binding, no consent or a failed send means no note.
• Rollback: set MCP_REPORT_BACK_ENABLED=false on the mcp-server and restart it (false, 0, no or off, any case). No parent is sent anywhere, default or typed, and receipts are byte-identical to today. No image revert is needed. The knob is in docker-compose.yml, docker-compose.prod.yml, docker-compose.hosted.yml and .env.example.

One backend change: delegated children no longer speak into the conversation

POST /api/agents/{name}/voice-reply (src/backend/routers/agents.py) now refuses a delegated child, i.e. a row that inherited its parent's conversation (source_channel_agent set), with reason="delegated_turn", before any channel branch. Inherited context exists for the consent-gated completion report. The voice route has no proactive-consent check, so the default above would otherwise let a delegated child with voice enabled speak into the user's thread. A direct channel turn still delivers. chat_execution_service.py is untouched, and _inherited_channel_context (ent#265) is still the only gate on who may inherit a conversation; its documented limits apply unchanged.

What still cannot report back, and why

• A plain sequential call (parallel=false, /chat): ChatMessageRequest has no parent field, and /chat creates its row without inheritance. This needs backend work; follow-up bug: a sequential chat_with_agent call cannot report back to the caller's conversation #3295. The result says report_back: "off", reason sequential_chat, when an id was passed.
• A child closed by backend shutdown or a cleanup sweep is written failed with no report (pre-existing).
• An identical request from a later turn within 24 h replays the first run, which reports where that run was asked to; follow-up bug: an identical delegation from a later turn replays the first run, so report-back goes to the first turn's conversation #3296.
• A delegated task that needs a skill-gate approval. The approved run is re-created from the frozen dispatch, which carries no inherited conversation.
• fan_out: its backend request has no parent field.

Note for abilityai/trinity-enterprise#566

Its body says "chat.ts forwards execution_id". That was false before this PR. It is now true on every /task route, and by default for async delegation. When its intent key is designed, it should prefer the platform turn (X-Trinity-Execution-Id, which the backend already receives on /chat and /task), with the typed parent as a fallback, as the skill-gate requester already does.

Tests

• src/mcp-server/src/chat-parent-execution.test.ts (new). It uses the real createServer, a real MCP client over streamable HTTP, the real reconciler for a dedicated tool, and a fake backend that records the /task and /chat request bodies, the forwarded turn header and the Idempotency-Key. Every acceptance criterion is asserted on what leaves the MCP server. Each "nothing sent" assertion is paired in the same test with a forwarding sibling, so the file is red on dev. resolveReportBack is also checked over its full input product against an independent oracle.
• tests/unit/test_3232_voice_reply_delegated_child.py: a delegated child is refused on Slack and Telegram; a direct turn still delivers. Red on dev.
• Commit 2 (test(mcp): …) is red on its own by design (failing tests first). Squash-merge is assumed.

Local proof (targeted, as agreed for this PR; CI runs the full suites):

• src/mcp-server: npm run build clean; npm test 775/775.
• Backend: 35 test files covering routers.agents, the ent#224/ent#265/ent#457 report paths, compose and .env.example parity, and the registry guard: 780 passed.
• Teeth on the final tip: with no parent on either /task branch, 18 of the 55 tests fail; with the original bug restored (execute drops execution_id), 16 fail; with the dedicated tool not forwarding, 3 fail; with the kill-switch env read ignored, 1 fails.
• Every compose stack renders MCP_REPORT_BACK_ENABLED on the mcp-server service only, and false overrides it.

Follow-ups: #3295 (sequential /chat report-back) and #3296 (cross-turn replay).

The ui label is on because the compose files put this PR in the Lane C merge gate, which runs frontend-e2e on the PR (it passed). No frontend code changed.

Reviewers: this PR turns on a behaviour that posts into customer conversations without an argument (async delegation report-back), so the review is also the product sign-off for that default.

[dolho]

/review Report

Branch: AndriiPasternak31/issue-3232 → dev (merge-base a72271d9c)
Files Changed: 28 (+1546/-43); code: src/mcp-server/src/tools/{chat,dynamic-agents,execution_id}.ts, server.ts, index.ts, src/backend/routers/agents.py, compose files and .env.example; the rest is tests and docs
Scope: CLEAN
Plan Completion (issue #3232 acceptance criteria): 4 done / 0 partial / 0 not done / 1 changed / 0 unverifiable

AC	Status	Evidence
parallel=true with execution_id sends parent_execution_id, proven through the real tool execute	DONE	chat.ts execute now destructures and forwards execution_id; the /task body carries reportBack.parentExecutionId. Asserted end to end through createServer and a real MCP client in chat-parent-execution.test.ts
Decide and document sequential / pull-routed branches and the header-vs-typed source	DONE (CHANGED)	Pull-routed /task now carries the parent too; sequential /chat does not (no backend field, follow-up #3295). The header turn wins over a typed id (resolveExecutionId). The PR also goes beyond the AC: report-back is on by default for async delegation, with a kill switch. That default is a product decision, and the PR body asks for sign-off on it
Dedicated chat_with_<agent> tools accept the same argument	DONE	dynamic-agents.ts declares execution_id with the shared EXECUTION_ID_PARAM_DESCRIPTION and forwards it
The execution_id description promises only what the code does	DONE	One shared parameter text. It states the async default, the manual opt-out, that sync is opt-in and sequential never reports back, and that requested is not a delivery guarantee

The backend voice-reply refusal is additive scope, but it follows directly from the default: without it, a delegated child with voice enabled could speak into the user's thread with no proactive-consent check. It is justified in the PR body, so it is not drift.

Execution coverage (Step 2.5)

changed symbol / test file	executed by	live consumer	verdict
resolveReportBack	chat-parent-execution.test.ts (end to end, plus the 864-combination product against an independent oracle)	runAgentChat (chat.ts)	✅ executed
withReportBack / report_back* fields	same file, every result path	runAgentChat return paths	✅ executed
execute forwarding execution_id (chat_with_agent)	same file, real MCP client	tool registration in createChatTools	✅ executed
dedicated tool execution_id + reportBackEnabled	same file, real reconciler	startExposedToolsReconciler ← index.ts / server.ts	✅ executed
MCP_REPORT_BACK_ENABLED env parse	same file (env read case)	createServer defaults; compose renders it on mcp-server	✅ executed
isWellFormedExecutionId	same file	resolveReportBack	✅ executed
voice-reply delegated_turn refusal	test_3232_voice_reply_delegated_child.py (calls the route)	POST /api/agents/{name}/voice-reply	✅ executed

Source-text grep (readFileSync|getsource|read_text|toContain|toMatch) over both new test files: no hits.

Fix mutation (run locally, then restored):

• parent_execution_id removed from both /task branches in chat.ts: 18 of 55 tests in chat-parent-execution.test.ts go red.
• Voice guard disabled (if False:): 3 of 3 tests in test_3232_voice_reply_delegated_child.py go red.

Local runs on the PR head: tsc --noEmit clean; mcp-server npm test 775/775; the new backend test passes 3/3. A wider -k run over the voice, channel-report, compose/env and registry areas showed 1057 passed. The 4 failures and 1 collection error in that run are in unrelated files (test_ent14_registry_url_ssrf.py, test_3215_plan_scheme.py) and come from my local venv (Python 3.12, stale payments_py), not from this diff. All PR CI checks are green.

Critical Findings (block merge)

None.

Informational Findings (review required)

[I1] Product Quality (4.15): the default turns on unprompted posts into customer threads, once per async hop (Confidence: 8/10)
File: src/mcp-server/src/tools/chat.ts (resolveReportBack)
Evidence: else if (asyncRoute && headerTurn && !(input.isSelfTask && input.injectResult)) arm = "default";
Issue: This is intended and documented, but a chain A (Slack) → B async → C async now produces two notes in the user's thread with no argument passed. An agent tasking itself asynchronously without inject_result is also default-armed, so it reports into its own conversation. Consent gates (allow_proactive, Workspace client scoping) and the ent#265/ent#457 inheritance guard still apply, so this is a noise and UX question, not a disclosure.
Suggestion: Make the product sign-off an explicit decision on the PR. After deploy, use the [Report-Back #3232] log line to check how often multi-hop chains post more than once.

[I2] Product Quality (4.15): the kill switch exists only as an env var (Confidence: 6/10)
File: src/mcp-server/src/server.ts, compose files, .env.example
Issue: MCP_REPORT_BACK_ENABLED can only be changed by editing env and restarting the mcp-server. There is no system_settings / Settings UI surface. This matches the MCP_AGENT_CHAT_PULL_ENABLED precedent and is acceptable as an emergency rollback knob. An operator who wants report-back off for one noisy agent or channel has no finer control than the existing channel consent flags.
Suggestion: No change needed for this PR. Consider a per-agent or Settings surface if operators ask for it.

[I3] Observability: report_back: "requested" is set whenever a header turn exists, including turns with no conversation (Confidence: 6/10)
File: src/mcp-server/src/tools/chat.ts (resolveReportBack fields for arm === "default")
Issue: For a scheduled or non-channel caller turn, the child inherits nothing (_inherited_channel_context returns no context when the parent has no source_channel), but the result and the log still say requested. The note hedges this ("if there is one"), so the model is not misled. However, the log line the PR proposes for measuring uptake will overcount actual channel deliveries.
Suggestion: When measuring uptake, join the log against rows whose parent has source_channel set, or say in the log line that requested means only that a parent was sent.

Low confidence (appendix)

• resolveReportBack reason precedence: a typed malformed id on the sequential /chat route with no header reports invalid_execution_id rather than sequential_chat. Either reason is truthful and nothing is sent, so this does not matter in practice. (Confidence: 4/10)

Clean Categories

• SQL & data safety: no SQL or schema changes; the backend change is a single early return in a route handler.
• Race conditions: none introduced. The parent is the live header turn, and ent#457 still refuses inheritance from a non-running parent (chat_execution_service._inherited_channel_context).
• Auth boundaries: _inherited_channel_context (ent#265 provenance guard: an agent principal must be the parent's executing agent, a human must be the owner or an admin, a connector never inherits) is unchanged and is still the only gate. A typed id is format-checked and never wins over the header. The voice-reply route keeps AuthorizedAgentByName with a matching {agent_name} path param, plus its self-gate.
• Invariant Unified Executions Dashboard (EXEC-022) #18: the parent is not part of the Idempotency-Key (asserted in the new test), and the forwarded turn header is computed as before.
• Credential exposure: the new log line contains execution ids, agent names and the caller id only, no tokens.
• Enterprise disclosure (4.5): the enterprise-docs-guard pattern finds no hits on lines this PR adds under docs/.
• Enum completeness: the new delegated_turn reason is returned in the same {delivered: false, reason} fail-soft shape as the route's existing reasons. source_channel_agent is written only on the inheritance path (chat_execution_service.py row creation), which matches turn_audience._is_delegated, so a direct channel turn is never refused.
• Documentation staleness: requirements (mcp.md, public-access.md), architecture area files, feature flows and user docs are updated in the PR.
• Incomplete fix (4.14): the fix covers every /task route (parallel async, parallel sync, pull-routed) and the dedicated tools. The gaps that remain (sequential /chat, fan_out, cross-turn replay, skill-gate re-creation) are named in the PR, with follow-ups bug: a sequential chat_with_agent call cannot report back to the caller's conversation #3295 and bug: an identical delegation from a later turn replays the first run, so report-back goes to the first turn's conversation #3296.

Summary

• Critical: 0 (none found)
• Informational: 3 (I1 is the product sign-off on the default; I2 and I3 are minor)
• Scope: clean

🤖 Generated with Claude Code

[dolho]

Approved — /review found no blocking findings; see review comment above.

[vybe]

merge-train (2026-10-07): changed the body's Refs to Fixes — validation found every acceptance criterion met (follow-ups are already filed separately), so the issue should close/promote on merge. Mechanical; nothing pushed to the branch.

[vybe]

merge-train (2026-10-07): merged as part of train #3329 (green). Before the squash, dev was merged into this branch because an earlier sibling had appended to the same files (tests/registry.json and/or docs/memory/feature-flows*.md). Both sides were kept, nothing else was changed.