fix(idempotency): a failed async run stops replaying its receipt (#3245)

docs/KNOWN_ISSUES.md

@@ -60,7 +60,7 @@
 
 **Symptoms:**
 - Agent A calls `mcp__trinity__chat_with_agent(agent_name="B", message="...", timeout_seconds=900)` and B works longer than the client's ceiling
-- Before the mitigation, the call failed with a timeout error and the result was lost. Now the MCP server stops waiting at `MCP_CHAT_TIMEOUT_MS` (25 seconds by default) and answers with `{status: "queued_timeout", execution_id, …}` while B keeps working; read the outcome with `get_execution_result` and never re-send
+- Before the mitigation, the call failed with a timeout error and the result was lost. Now the MCP server stops waiting at `MCP_CHAT_TIMEOUT_MS` (25 seconds by default) and answers with `{status: "queued_timeout", execution_id, …}` while B keeps working; read the outcome with `get_execution_result` and never re-send — unless that outcome is a confirmed `failed` or `cancelled`, which a word-for-word re-send retries (#3245)
 - The `timeout_seconds` parameter has no effect on either limit
 
 **Cause:**

docs/MULTI_AGENT_SYSTEM_GUIDE.md

@@ -819,7 +819,7 @@ mcp__trinity__chat_with_agent(
 
 **What this means for agent design:**
 - A long synchronous call no longer just fails: when the server can match it to its execution, it answers with a receipt and the target keeps running it; when it cannot, the error says so and names `list_recent_executions`
-- The caller reads the outcome with `get_execution_result(agent_name, execution_id)` and never re-sends — a reworded re-send runs the work twice. Every agent is taught this as the delegation contract in its platform prompt (§Agent Collaboration); the `chat_with_agent` description carries the same text
+- The caller reads the outcome with `get_execution_result(agent_name, execution_id)` and never re-sends — unless the outcome is a confirmed `failed` or `cancelled`, which a word-for-word re-send retries (if the same `execution_id` comes back, set a reminder and end the turn). A reworded re-send runs the work twice. Every agent is taught this as the delegation contract in its platform prompt (§Agent Collaboration); the `chat_with_agent` description carries the same text
 - The calling turn still decides what to do while it waits: read later, arm `set_reminder`, or — for a `parallel=true` run — subscribe to the target's `agent.task.completed` / `agent.task.failed`
 
 **Design patterns that work within this constraint:**

docs/TRINITY_COMPATIBLE_AGENT_GUIDE.md

@@ -1069,7 +1069,7 @@ Permission checks before any agent-to-agent communication:
 > **Design Limitation**: an MCP client gives up on a tool call at its own ceiling (30–60 seconds is typical). Trinity's MCP server stops waiting first — at `MCP_CHAT_TIMEOUT_MS`, 25 seconds by default — and answers a synchronous `chat_with_agent` with a receipt (`status: "queued_timeout"` plus the `execution_id`) while the work keeps running. `timeout_seconds` does not extend that wait.
 
 When designing agents that collaborate with other agents, ensure that:
-- A receipt is treated as "running", never as a failure: the outcome is read with `get_execution_result`, and the call is never re-sent. Every agent is taught this as the delegation contract in its platform prompt (§Agent Collaboration); the `chat_with_agent` tool description carries the same text
+- A receipt is treated as "running", never as a failure: the outcome is read with `get_execution_result`, and the call is never re-sent unless the outcome is a confirmed `failed` or `cancelled` — then it is re-sent word for word to retry. Every agent is taught this as the delegation contract in its platform prompt (§Agent Collaboration); the `chat_with_agent` tool description carries the same text
 - Complex tasks use the async pattern (`parallel=true, async=true`) from the start
 - Large data exchanges use shared folders instead of MCP return values
 

docs/memory/architecture/execution.md

@@ -143,7 +143,9 @@ schema change (all state is Redis). Details in [redelivery-governor.md](../featu
 
 ### Idempotency (RELIABILITY-006, #525)
 
-Trigger-boundary dedup — policy in Architectural Invariant #18, table DDL under `idempotency_keys`, details in [idempotency-keys.md](../feature-flows/idempotency-keys.md). `services/idempotency_service.py` (`begin`/`complete`/`fail`) over `db/idempotency.py`. The `(scope, key)` PRIMARY KEY is the atomic claim: `claim()` INSERTs an `in_flight` row; a concurrent loser catches `IntegrityError` and reads the surviving row (cross-process safe over the shared SQLite file). Lifecycle: `claim` → (`attach_execution`) → `complete` (stores `response_snapshot` for replay) or `release` (deletes the in_flight row so retry is possible; never deletes a `completed` row). Rows >24h expire and re-claim (cleanup purges via `idempotency_purge_expired`). Duplicates within 24h short-circuit with the original result + `X-Idempotent-Replay: true`; an in-flight duplicate returns 409. Fail-open.
+Trigger-boundary dedup — policy in Architectural Invariant #18, table DDL under `idempotency_keys`, details in [idempotency-keys.md](../feature-flows/idempotency-keys.md). `services/idempotency_service.py` (`begin`/`complete`/`fail`) over `db/idempotency.py`. The `(scope, key)` PRIMARY KEY is the atomic claim: `claim()` INSERTs an `in_flight` row; a concurrent loser catches `IntegrityError` and reads the surviving row (cross-process safe over the shared SQLite file). Lifecycle: `claim` → (`attach_execution`) → `complete` (stores `response_snapshot` for replay) or `release` (deletes the in_flight row so retry is possible; never deletes a `completed` row — two exceptions, each deleting only a row whose replay would be a lie: #2040's `discard_completed` for an agent that no longer exists, and #3245's compare-and-delete below). Rows >24h expire and re-claim (cleanup purges via `idempotency_purge_expired`). Duplicates within 24h short-circuit with the original result + `X-Idempotent-Replay: true`; an in-flight duplicate returns 409. Fail-open.
+
+**A receipt replays only while its run can still succeed (#3245).** An async `/task`, a timed-out sync `/task` and a timed-out pulled `/chat` store a *receipt* (`async_mode: true`, status `accepted`/`queued`/`queued_timeout`) as the `completed` row, and nothing revisits it, so before #3245 an identical retry got the dead run's receipt for 24h. (a) **Where:** the check runs at the shared admission seam (`dispatch_admission_service._reclaim_ended_receipt`, called right after `begin` in `admit_chat_request` and `begin_task_idempotency`), at replay time — never on a terminal writer, so it adds no side effect to the #1804 writer set and heals rows written before the fix. Only receipt-shaped snapshots are checked (a sync success body replays with no read); `sched:` keys are skipped; the skill gate answers a gated request before `begin`. (b) **Verdicts:** `idempotency_service.execution_liveness(execution_id, agent_name)` returns `live` (`queued`/`running`/`pending_retry`), `succeeded`, `ended` (`failed`/`cancelled`/`skipped`), `gone` (no row), `maybe_alive` or `indeterminate` (lookup error, foreign owner, `manual`, unknown status) — reusable by any caller that needs "can this run still succeed?". Only `ended` and `gone` reclaim; everything else replays (fail toward not re-running). It reads `get_execution_gate_state` (two columns) and, only on `failed`, `get_execution_failure_stamp` (status, error, completed_at). (c) **The `lease_expired` window:** a FAILED whose error starts `lease_expired:` is `maybe_alive` until `completed_at + agent timeout + SLOT_TTL_BUFFER`, then `ended`. The anchor is `completed_at` (the moment the slot reaper wrote FAILED) because the reaper fires only after the slot TTL already ran out from acquire, so a `started_at` anchor would never hold. This is a **heuristic, not a bound**: the agent's restart sweep re-posts persisted results with no age limit and the callback route still accepts SUCCESS over an async FAILED (#378), so a restart after the window can still land SUCCESS beside a fresh run; the timeout is the agent's *current* setting; only the two `fail_stale_slot_execution` writers in `cleanup_service._process_stale_slot_reclaims` carry the tag (an AST guard pins that each such call carries it, not that no other writer ever does) — the Phase-1 stale sweep and the no-session sweep are untagged and go fresh at once; and the tag does not mean "async" (sync push rows are tagged too, whose late SUCCESS can only come from the backend's own dispatcher) — the hold is conservative protection for every tagged row. Nothing wakes the caller when the window ends; the delegation contract tells it to set a reminder. (d) **Race:** two identical retries that see the same dead run start one new run — `idempotency_discard_completed_if_execution` deletes only the row still naming the dead execution id, so a slow retry cannot erase a faster retry's fresh record, and the re-`begin` runs once (no loop). That CAS is sound only because nothing writes a claim after its receipt is stored (pinned by an AST guard over `run_async_task`, `backlog_service`, `pull_coordination_service` and both writers that store a receipt and then raise — the sync `/task` backlog timeout in `_dispatch_sync_backlog` and the pulled `/chat` timeout in `run_pulled_chat_turn`). A late SUCCESS landing in the milliseconds between the liveness read and the delete is part of the residual above (closing it needs cross-domain SQL in `db/idempotency.py`). A delete error replays the original (no second `begin`); a second `begin` that fails open after the compare-and-delete (whether it deleted the row or matched nothing) runs the request without dedupe (logged `outcome=no_dedup`), so a concurrent identical retry can run too — the usual dedupe-layer-failure exception. Each reclaim logs `[#3245] receipt_reclaim … cas=… outcome=…` and each held replay `[#3245] receipt_hold … until=…` (never the raw key). (e) **Leftover risks:** effects can repeat across attempts (effect dedupe is `effect:{execution_id}`); a retry storm is no longer braked by the stale replay (bounded by the caller and the dispatch breaker); a claim still `in_flight` when its run died keeps answering 409 for 24h (needs claim owner tokens; #3298). The MCP client marks a replay `idempotent_replay: true`.
 
 **Effect-scoped extension (#1084):** trigger-boundary dedup stops a re-POSTed `/chat`/webhook from creating a *second execution*; it does NOT reach an agent's individual outbound tool calls. So a re-delivered turn (the at-least-once semantics pull-mode / work-stealing will introduce, Epic #1045/#1081) re-emits the same side effect (re-sends a message, re-charges a payment). The same `idempotency_service` adds a per-sink guard — `effect_guard(effect_type, identifying_args, *, execution_id, agent_name, dedup_label, payment_request_id)` — enforced at the SINK, per resolved action identity. Scopes: `effect:{execution_id}` for messages/voip/share_file (after `resolve_and_validate_execution` confirms the execution belongs to the agent — generalizing MEM-001), `payment:{agent_request_id}` for Nevermined settles (a Nevermined observability id, **not** a provider exactly-once token — this local guard, not the provider, enforces at-most-once per id; residual at-least-once retry tracked by #1408). Key = `{effect_type}:sha256(execution_id ∥ effect_type ∥ resolved_identifying_args ∥ dedup_label)` on **resolved, immutable** identity only (recipient/channel/account) — **never the LLM-generated body** (non-deterministic across a re-run → would defeat dedup); `dedup_label` lets an agent intentionally repeat an effect to the same target in one turn. `in_flight ≠ completed`: a completed replay returns the stored sanitized snapshot (no re-emit); an in-flight replay raises `EffectInProgressError` (router → 409, never a silent skip-and-succeed). Reuses the 24h default TTL (already exceeds the lease window, so a completed row outlives a late re-delivery — no new TTL plumbing). Wired sinks: `proactive_message_service.send_message`, `voip_service.place_outbound_call`, `agent_shared_files_service.create_share`, `a2a_outbound_service.call_agent`, `voice_reply_service`, `channel_completion_report`, `nevermined_payment_service.settle_payment_once`; the platform supplies `execution_id` via the agent's MCP header `X-Trinity-Execution-Id` (the tool arg is the older-image fallback), and on a pull-mode agent an effect with no usable id is **refused** with an operator alarm, elsewhere sent and logged `effect_guard.degraded` (#2392; git push is idempotent-by-construction and needs no key). **Target-direction note:** `TARGET_ARCHITECTURE.md` v2 reframes this from a **per-agent** gate to **per-effect** — read/analysis-only + reversible + capability-confined-irreversible effects default on, only irreversible-**un-confineable** effects gate via the async operator queue (#1402); `effect_guard` (this section) is the reversible/backend-sink slice and retry-with-prior-trace (#1401) is the general recovery. See [effect-idempotency.md](../feature-flows/effect-idempotency.md).
 

docs/memory/feature-flows.md

@@ -24,6 +24,7 @@
 | Date | ID | Change | Flow |
 |------|-----|--------|------|
 | 2026-10-06 | #3123 | chore(skills): **fresh installs seed the bundled community skills catalog at `trinity-skills` v0.3.0** (was v0.2.0). The tag adds `update-dashboard`, the playbook the metrics tiles already name, and drops the four retired `add-*` installers (marketplace-only now): 38 skills. `config.DEFAULT_SKILL_SOURCE_REF`, `.env.example` and the user doc move together; `tests/unit/test_2545_skill_source_pin.py` raises its floor to v0.3.0 and adds the user doc to its parity checks. The user doc gains the steps for moving an existing instance's bundled row to a newer tag (existing installs keep their row; ent#529). No runtime code change beyond the default: the seed and the annotated-tag peel (#2550) already handle the tag. | [skills-library-sync.md](feature-flows/skills-library-sync.md) |
+| 2026-10-06 | #3245 | fix(idempotency): **a failed async run stops replaying its receipt** — an async `/task`, a timed-out sync `/task` and a timed-out pulled `/chat` store a dispatch receipt as the `completed` idempotency row, and for 24 h an identical retry got the dead run back. The shared admission seam (`dispatch_admission_service._reclaim_ended_receipt`, right after `begin` in `/chat` and `/task`) now asks `idempotency_service.execution_liveness` whether that run can still succeed; an `ended` (failed/cancelled/skipped) or `gone` run is compare-and-deleted (`discard_completed_if_execution`, only while the row still names it, so two racing retries start one run) and the key claimed once more. A `lease_expired` FAILED is held until `completed_at + timeout + 300 s`. Read-time only — no terminal writer changes (#1804). MCP marks replays `idempotent_replay: true`; the delegation contract gains the word-for-word retry bullet (pin 1,650 → 1,730) | [idempotency-keys.md](feature-flows/idempotency-keys.md), [mcp-orchestration.md](feature-flows/mcp-orchestration.md), [agent-to-agent-collaboration.md](feature-flows/agent-to-agent-collaboration.md), [system-wide-trinity-prompt.md](feature-flows/system-wide-trinity-prompt.md) |
 | 2026-10-05 | ent#752 | feat(gate): **a gated skill is refused inside the agent unless its run was approved for it** (Claude Code only). A PreToolUse hook on `Skill` and on `Agent`/`Task` skill preloads (`/opt/trinity/hooks/skill-gate.py`, its own managed-settings drop-in, exec form) asks `POST /api/skill-gate/check` with the agent's own key; the run is cleared by #751's approved-run record or a new `self_approved` record written where a self-approval lets a run through (no migration). No answer → a root-owned marker decides (gated agents fail closed, others keep working); never exit 1. A self-approved `/chat` turn runs in its own session; `/health` reports `skill_gate_hook` | [skill-gate.md](feature-flows/skill-gate.md) |
 | 2026-10-05 | ent#568 | feat(delegation): **every caller is taught that a receipt means the work is running — never re-send on silence**. One text (`DELEGATION_CONTRACT`, byte-identical in `platform_prompt_service.py` and `src/mcp-server/src/delegation_contract.ts`) is spliced into the platform prompt's §Agent Collaboration and published verbatim in the `chat_with_agent` and every `chat_with_<agent>` description; `fan_out` / `send_message` repeat its rule sentence. Every `chat_with_*` `execution_id` receipt carries "Do not re-send: read the outcome with `get_execution_result(…)`", and the backend's REST "poll GET" receipts are rewritten with liveness-neutral leads. Each carrying description stays under Claude Code's 2,048-char cap — `chat_with_agent` had been cut there since #2958 | [mcp-orchestration.md](feature-flows/mcp-orchestration.md), [system-wide-trinity-prompt.md](feature-flows/system-wide-trinity-prompt.md), [mcp-agent-exposure.md](feature-flows/mcp-agent-exposure.md) |
 | 2026-10-03 | #3185 | feat(a2a): **the outbound client speaks 402 — a priced remote is an outcome, not an outage**. `_read_capped` collapsed every HTTP ≥ 400 into `rpc_http_error` without reading the body, so a remote answering 402 was unreachable: the caller saw neither the price nor a way to attach a token. 402/403 on the RPC hop are now classified BEFORE the encoding and length guards (a gzipped or oversized "pay me" read as an outage), both rails are read (the HTTP 402's `payment-required` header or body, and an in-band `x402.payment.status` task — checked before `_parse_task` on send AND poll, or a priced `input-required` reaches the agent as a prompt it polls forever), and the route answers **402** with an allowlisted `detail.payment` = `{summary, x402, truncated}`, scrubbed against the token, its base64 forms and its decoded leaves. `payment_rejected` / `rpc_forbidden` stay 502 with `remote_status`, so 402 ("buy") is always distinguishable from 403 ("top up"). The endpoint credential carries a `credential_kind` (`api_key` default — every pre-#3185 record — or `payment_token`, inferred from the value when omitted with the SAME predicate the client sends on), an x402 v3 token is flagged `credential_single_use` rather than refused, and the MCP tools return `payment_required` + `payment` + `task_id` + `do_not_retry` with a description that names who can act. Trinity never buys, never retries a priced call, and never snapshots a 402 — a stored one would replay "pay me" after the operator paid | [a2a-outbound-call.md](feature-flows/a2a-outbound-call.md), [architecture/backend.md](architecture/backend.md), [architecture/mcp-server.md](architecture/mcp-server.md) |