fix(web): expose structured terminal command outcomes on agent_error frames

[codeacme17]

Summary

Part 1 of #1500 (backend wire contract; the frontend consumption follows in a separate PR).

• The identity-bearing first-party terminal agent_error frame now carries outcome, resend_safe and message_code, read from the terminal-event draft the dispatcher binds before broadcasting. A client can decide whether resending a durably accepted reply is safe without parsing human-readable error text.
• Field names match the durable terminal-event projection in fix: replay terminal command outcomes #1904, so the frontend contract survives the later switch from this live broadcast to durable delivery/replay.
• A terminal first-party MESSAGE stops restating the condition it was last waiting on (Task command message failed: Message <id> is waiting for the live-control resume slot). The notice now asserts non-application only when the bound draft proves it (resend_safe=True), and uncertainty otherwise, mirroring external_input_terminal_message.
• The two anonymous external frames keep their pinned minimal shape ({type, message, task_id, timestamp}): a retry decision needs the command_id those frames deliberately withhold.

Scope

This PR delivers the structured post-ack terminal outcome contract from #1500's acceptance criteria plus the exhaustion-wording criteria added in the 2026-08-26 issue comment. Frontend correlation, retry gating, draft preservation, staleness guards, and the full-sequence regression coverage land in the follow-up PR.

Deliberate wording-scope notes:

• The proof-aware sentence applies to every first-party MESSAGE terminal, including generic-failure exhaustion, not only deferral exhaustion. A generic failure's outcome is unknown by construction (a worker may have injected the message before crashing), and the previous fallback ended in "Please try again." — exactly the duplicate-submission invitation fix(web): gate clarification retries on structured terminal command outcomes #1500 forbids for unknown outcomes.
• The structured fields also appear on first-party PAUSE/RESUME/CANCEL terminal frames. They carry the same draft-derived values and match the durable projection; MESSAGE-only fields would have made the frame shape depend on the kind.

Out of scope (pre-existing, tracked):

• A first-party MESSAGE rejected after ack keeps its handler-owned notification and is deliberately not re-broadcast here; when the origin socket is gone, the live notice is lost. The terminal event for that rejection is already persisted by fix: persist terminal command outcomes atomically #1901; delivering it durably across reconnects is fix: replay terminal command outcomes #1904 (part of fix(web): deliver terminal task-command outcomes durably across workers #1858).
• resend_safe stays false for rejection outcomes even though a rejection proves non-application. That is the persisted draft contract from fix: persist terminal command outcomes atomically #1901 and is conservative in the resend direction (a rejected reply should not be invited for an identical resend).

Verification

• New focused suite tests/web/api/test_terminal_command_outcome_fields.py: resend-safe/unsafe deferral exhaustion, generic-failure exhaustion, wording never restates the wait reason, non-MESSAGE frames keep their wording, external frame keeps its pinned shape.
• Wording unit tests in tests/web/services/test_task_command_terminal_events.py.
• Affected neighbor suites (client-safe sweep + AST guard, transport, message/resume contention, external cancel, external input seam): 360 passed.
• Ruff format/check and mypy on both changed source files; git diff --check clean.
• The full backend suite was not run locally; CI covers it.
• PostgreSQL-parametrized tests were skipped locally because XAGENT_TEST_POSTGRES_URL is not configured.

[gemini-code-assist]

Code Review

This pull request introduces structured command-outcome fields (outcome, resend_safe, and message_code) to terminal agent_error frames in the WebSocket API, allowing clients to determine if a command is safe to resend without parsing human-readable text. It also adds specific terminal wording for first-party MESSAGE commands based on whether non-application can be proven. I have no feedback to provide.

[rogercloud]

Summary

This PR adds structured outcome, resend_safe, and message_code fields to terminal first-party WebSocket frames (agent_error for MESSAGE, PAUSE, RESUME, and CANCEL), sourced from a TerminalTaskEventDraft that is bound onto the raised exception before broadcast. It also changes the first-party MESSAGE terminal notice to stop restating the specific wait-condition a command was blocked on, instead asserting non-application only when resend_safe=True and uncertainty otherwise. This is explicitly Part 1 of #1500 — a backend wire-contract change with frontend consumption (correlation, retry-gating, staleness guards) deferred to a follow-up PR.

Verdict: acceptable-with-reservations

The mechanism is fail-safe: resend_safe cannot be wrongly defaulted to true (TaskCommandDeferred.__init__ defaults it to False, TaskCommandRejected has no such attribute at all, and only one call site in the whole tree can ever produce True — a data-derived proof at the contention-deferral path). The wording fix genuinely resolves the ClientVisibleTaskCommandDeferred PAUSE/CANCEL wait-condition leak. The gating is strictly resend_safe is True, never a weaker check. My reservations are about the wire contract being an incomplete projection of the durable event, and about a second retry-safety vocabulary that a future frontend PR will need to reconcile with the existing tri-state MessageDeliveryDisposition system — neither is a defect in what's shipped here, since nothing consumes these fields yet.

Findings

1. [Major/design, non-blocking] src/xagent/web/api/websocket.py:9785-9806 — wire frame omits correlation keys needed for the staleness invariant #1500 requires.
The persisted projection (task_command_terminal_event.py, unique on (task_command_id, outcome_version)) carries task_run_id/outcome_version specifically to disambiguate two terminal outcomes of the same command_id — and retry_failed_task_command (task_command_transport.py:1043-1075, reachable from a2a.py:1452) can cause the same command_id to reach a terminal broadcast more than once. command.target_run_id and command.attempt_count are already in hand at this call site, so adding the two fields is low-cost. The comment at websocket.py:9779 claims the field names "match the durable terminal-event projection," but these two disambiguating fields were dropped from that match. Worth adding now rather than as a second breaking wire-contract iteration once the frontend PR lands.

2. [Minor/design, non-blocking] New boolean resend_safe/message_code vocabulary duplicates an existing tri-state pattern, on a different correlation key.
The codebase already has a working tri-state disposition for pre-ack message delivery: send_message_delivery emits rejection_outcome: "not_accepted" | "outcome_unknown" keyed by client_message_id/turn_id (websocket.py:538-568); the frontend's MessageDeliveryDisposition (use-websocket.ts:56) gates retry on it (use-websocket.ts:1497). This PR adds a second, boolean-only disposition system for post-ack outcomes, keyed by command_id, covering all command kinds. This looks like a deliberate, reasoned choice (naming matches the durable DB schema per #1904; the pre-ack system doesn't reach non-MESSAGE kinds), but it does mean a future frontend PR must hold and reconcile two different vocabularies. Flagging as an architectural-coherence note, not a defect.

3. [Minor, non-blocking] src/xagent/web/api/websocket.py:9798 — resend_safe is unconditionally false for PAUSE/RESUME/CANCEL, backwards for the idempotent-command case.
Every PAUSE/CANCEL deferral raise site (websocket.py:9521) and the RESUME deferral site (websocket.py:9596-9597) omits resend_safe, so it defaults to False. The only site anywhere that can produce True is MESSAGE-only (websocket.py:9534-9551). PAUSE/RESUME/CANCEL are largely safe to retry (state-version checks, dedup on client_command_id), so a naive future frontend rule like "hide retry unless resend_safe" would block retry exactly where it's safest. Inert today since nothing consumes the field, but worth resolving — e.g. making resend_safe command-kind-aware, or documenting the exemption — before a frontend PR builds a generic rule on it.

4. [Major, non-blocking] tests/web/api/test_terminal_command_outcome_fields.py:124 — the test asserting the wording fix uses the wrong exception class and would pass on pre-PR code too.
test_message_terminal_wording_never_restates_the_wait_reason constructs a plain TaskCommandDeferred, not ClientVisibleTaskCommandDeferred. Tracing the pre-PR path: a plain TaskCommandDeferred was never a ClientVisibleError, so pre-PR it would have fallen through to the generic fallback text ("The message could not be processed. Please try again.") — which already doesn't contain "waiting" or "resume slot". So this test's assertions hold identically on both the buggy and fixed code paths; it doesn't actually pin the regression it's named for. The real leak sites use ClientVisibleTaskCommandDeferred (websocket.py:9554, 9570 pre-PR) — the test should construct that class to actually exercise the fix.

5. [Minor, non-blocking] src/xagent/web/services/task_command_terminal_events.py:71 — docstring claims a "mirror" with external_input_terminal_message that doesn't hold; the provably-safe first-party rejection case has no outlet.
The comment claims first_party_message_terminal_text "mirrors" external_input_terminal_message, but the two use different proof rules: the external function checks isinstance(error, TaskCommandRejected) (exception-type-based, "definitely not applied"), while the first-party function reads only draft.resend_safe, which is unconditionally False for any non-TaskCommandDeferred including TaskCommandRejected — so it would say "cannot confirm" for the exact case the external version proves safe. Inert today since first-party MESSAGE rejections are deliberately not re-broadcast through this path per this PR's stated scope, but the inaccurate "mirrors" claim could mislead whoever wires up that broadcast later.

6. [Minor, non-blocking, code quality] Duplicated derivation logic and redundant draft lookups.

• src/xagent/web/api/websocket.py:9785 / src/xagent/web/api/websocket.py:483 — the same TerminalTaskEventDraft is fetched via terminal_event_draft_for_error(error) twice for one broadcast: once in _broadcast_terminal_command_error, again inside client_safe_task_command_failure → first_party_message_terminal_text. Passing the already-fetched draft down as a parameter would remove the re-derivation.
• src/xagent/web/services/task_command_terminal_events.py:95-97 — the "missing draft ⇒ unsafe/unknown" fallback rule is independently encoded in three places with no shared helper: here, websocket.py:9781-9784, and task_command_terminal_events.py:139-146. A future rule change needs to touch all three.
• The wire-side identity-disclosure branching in _broadcast_terminal_command_error (the scope-based if/else around websocket.py:9753) re-derives the same policy that draft.include_command_identity (websocket.py:9855) already encodes independently — the wire side could read draft.include_command_identity directly (with care to preserve the current default when draft is None).

7. [Minor, non-blocking, dead/unreachable code]

• src/xagent/web/api/websocket.py:9781 — this "a missing draft degrades to the unsafe/unknown reading" comment is dead: all three call sites always bind a draft first, and _terminal_command_event_draft never returns None. Also, TerminalTaskEventMessageCode has 4 members but only 2 (TASK_COMMAND_FAILED/TASK_COMMAND_DEFERRED) can ever appear on this wire frame — the other two only arise on the external-cancel branch, a different identity-free frame that never carries message_code.
• src/xagent/web/api/websocket.py:9797 — "outcome": "failed" is a hardcoded literal that happens to always be correct today (this function is only called from failure paths), but conveys no information and has no shared enum tying it to the durable side's str(command.status) (task_command_terminal_events.py:138), inviting silent drift.
• ClientVisibleTaskCommandDeferred is now dead weight at its two MESSAGE-kind raise sites (websocket.py:9554, 9570) — after this PR, the new branch discards the exception's own message text entirely for MESSAGE kind, so the "client-visible" subclass there is pointless (no other code path reads .message/.args off these exceptions for MESSAGE kind).

8. [Minor, non-blocking, test coverage]

• No first-party test does an exhaustive set(frame) == {...} key-set assertion (only the external-scope test does), so a leaked extra field or a silently-dropped command_id/command_kind on the first-party frame wouldn't be caught.
• Thin coverage of first-party non-MESSAGE kinds: only PAUSE + generic RuntimeError is tested; no test covers RESUME, first-party CANCEL, or message_code == "task_command_deferred" on a non-MESSAGE frame.
• No test asserts that one step below the deferral/failure exhaustion boundary (defer_count+1 >= max_command_defers(), websocket.py:9867; failure_count+1 >= MAX_COMMAND_FAILURES, websocket.py:9917) produces zero broadcasts — an off-by-one regression wouldn't be caught.
• (The docstring's claim that "the anonymous external frames" are pinned is accurate — the external-cancel variant is exhaustively pinned in tests/web/api/test_external_task_cancel.py:950, just not the file that has the gap noted above.)

9. [Minor design/race, non-blocking, pre-existing — not introduced by this PR] Broadcast can assert outcome:"failed"/resend_safe before the disposition is durably persisted.
_broadcast_terminal_command_error is called before dispatch_one_task_command actually persists the terminal disposition; at least 3 paths (DB pool timeout, TTL-recovery heartbeat outcome, defer CAS failure) can leave the row non-terminal/reclaimable after the broadcast already fired. Combined with the one resend_safe=True producer's proof only being valid "as of observation time," a client could theoretically be told "safe to resend" while the command is later re-executed elsewhere. This ordering and race pre-date this PR — this PR only makes an existing prose signal machine-gateable. Not blocking here since nothing consumes resend_safe to auto-gate a UI action yet, but flag it as a design concern to resolve before/alongside the frontend PR that will actually read this field (e.g. have the frontend trust only the durably-persisted terminal event, not the live broadcast, for auto-gating).

10. [Minor, non-blocking] tests/web/services/test_task_command_terminal_events.py:1051 — tests don't pin that fields are derived from the bound draft rather than the exception directly.
Every test builds the draft through the real _terminal_command_event_draft, which for TaskCommandDeferred just copies error.resend_safe onto the draft — so a hypothetical regression reading error.resend_safe directly instead of draft.resend_safe at the broadcast site would pass all current tests unchanged. This isn't load-bearing in any code path today (every draft in the codebase is currently derived mechanically from its exception), so it's low severity — forward-looking preparation for durable replay (#1904) that's worth a differential test before that lands.

Simplification opportunities

src/xagent/web/api/websocket.py:9798-9803: shrink redundant `is not None` guards before falsy checks on a value that's already None-or-truthy:
  "resend_safe": bool(draft and draft.resend_safe)
  "message_code": draft.message_code.value if draft and draft.message_code else None

net: -2 lines possible

Blocking status & recommended decision

Blocking: no. No confirmed finding meets the blocking bar — nothing produces an incorrect user-visible result or violates a currently-consumed data/compatibility contract in a reachable, supported scenario. Every issue above is either a pre-existing race that stays inert until a future frontend PR consumes these fields, or a design/test-quality gap in an explicitly-scoped partial deliverable.

Recommended event: APPROVE.

Confirmed non-blocking findings:

• src/xagent/web/api/websocket.py:9785-9806 — major/design — wire frame is missing task_run_id/outcome_version, so a future consumer can't disambiguate two terminal outcomes of a retried command_id.
• src/xagent/web/api/websocket.py:9798 — minor — resend_safe is always false for PAUSE/RESUME/CANCEL, the opposite of their actual retry-safety.
• tests/web/api/test_terminal_command_outcome_fields.py:124 — major — wording-regression test uses the wrong exception class and can't fail against the pre-fix code.
• src/xagent/web/services/task_command_terminal_events.py:71 — minor — "mirrors external" docstring claim is inaccurate; the provably-safe rejection case has no outlet.
• src/xagent/web/api/websocket.py:9785 / 483 — minor — redundant duplicate draft fetch per broadcast.
• src/xagent/web/services/task_command_terminal_events.py:95-97 — minor — "missing draft" fallback duplicated across three call sites with no shared helper.
• src/xagent/web/api/websocket.py:9781 — minor — dead comment describing an unreachable "missing draft" fallback.
• src/xagent/web/api/websocket.py:9797 — minor — hardcoded "outcome": "failed" literal with no shared enum tying it to the durable side.
• (unfiled line) — minor — ClientVisibleTaskCommandDeferred is now dead weight at its two MESSAGE-kind raise sites.
• (unfiled line) — minor — test coverage gaps: no exhaustive key-set assertion, no RESUME/CANCEL/non-MESSAGE-deferred coverage, no off-by-one boundary test.
• (unfiled line, pre-existing) — minor design/race — broadcast can precede durable persistence of the disposition it's reporting.
• tests/web/services/test_task_command_terminal_events.py:1051 — minor — tests don't differentially pin draft-vs-exception derivation.

[codeacme17]

Thanks for the thorough round. All findings are addressed in one commit, f89de99; per-thread replies carry the details. Summary:

Fixed in f89de99

• Finding 1: task_run_id + outcome_version added to the first-party frame (the durable projection's disambiguators; outcome_version uses the attempt count the terminal CAS write pins). The exhaustive key-set assertion pins the full frame shape.
• Finding 3 (documentation direction): the frame comment and a new parametrized test state that resend_safe is a proof of non-application, not a retryability rating — PAUSE/RESUME/CANCEL always carry false, and a consumer must reason from command_kind for those. fix(frontend): gate clarification retries on structured terminal command outcomes #2126 builds no generic rule on it.
• Finding 4: the wait-reason regression test now constructs ClientVisibleTaskCommandDeferred as the real raise sites do; verified the leak reproduces on origin/main with that class, so the test genuinely pins the fix.
• Finding 5: the "mirrors" claim replaced with an accurate description of the narrower proof rule, plus an explicit warning for whoever wires up first-party rejection broadcasts later.
• Finding 8: added the exhaustive key-set assertion, RESUME/CANCEL deferral-exhaustion coverage, and one-step-below-boundary zero-broadcast tests for both the deferral and failure budgets.
• Finding 10: a differential test binds a draft contradicting the exception and asserts the broadcast follows the draft.
• Simplification: applied.

Deferred with tracking issues

• Finding 9 → fix(web): live terminal command broadcast can assert resend_safe before the disposition is durably persisted #2135 (P1): the broadcast-before-persist ordering, with the two fix directions you outlined (broadcast after the disposition commit, or make fix: replay terminal command outcomes #1904's durable event the authority for auto-gating). Pre-existing; it becomes consequential once fix(frontend): gate clarification retries on structured terminal command outcomes #2126 consumes resend_safe, so it is flagged there too.
• Finding 6 → ref(web): consolidate the terminal-event draft fallback rule and disclosure policy encodings #2136 (P2): the duplicate draft fetch, the triplicated missing-draft rule, and the dual-encoded disclosure policy, consolidated in one pass (the fetch-threading trade-off interacts with the client-safe egress guard's shape recognition).

Declined with evidence

• Finding 2: acknowledged as accurate — the two vocabularies deliberately split by lifecycle: rejection_outcome answers "was this delivery accepted" pre-ack (keyed by client_message_id), the new fields answer "was the accepted command applied" post-ack (keyed by command_id, matching the durable schema for fix: replay terminal command outcomes #1904). fix(frontend): gate clarification retries on structured terminal command outcomes #2126 consumes them side by side without translation, so no reconciliation layer is planned.
• Finding 7 (first bullet): the missing-draft fallback stays as defensive live code — bind_terminal_event_draft is a public seam and an unbound future call site must degrade to the unsafe reading rather than crash.
• Finding 7 (second bullet): the two EXTERNAL_* message codes are consumed by the durable projection via the executor-bound drafts, not dead.
• Finding 7 (third bullet, ClientVisibleTaskCommandDeferred at the MESSAGE raise sites): confirmed inert for egress post-PR, but its text still lands in the command row's error column (read by the A2A cancel poll), and the class may matter again for fix: replay terminal command outcomes #1904 replay; noted as an optional fold-in on ref(web): consolidate the terminal-event draft fallback rule and disclosure policy encodings #2136 rather than changed here.

Verification for the range 0033d33..f89de99: the six neighbor suites (client-safe sweep + AST guard, transport, terminal events, outcome fields, external cancel, external input seam) pass 335/335 locally; ruff, format, mypy, and git diff --check clean. PostgreSQL-parametrized tests skipped (no XAGENT_TEST_POSTGRES_URL).