fix(execution): telemetry + transcript on a failing error_during_execution/504 row (#1853)

[AndriiPasternak31]

Fixes #1853

Problem

An execution that ends error_during_execution (Claude Code reports is_error with a non-terminal last message — commonly a fan-out whose <task-notification> lands after stop_reason=end_turn and interrupts the follow-on turn) was written to schedule_executions with status + error string only — no claude_session_id, no cost, no execution_log. The Claude JSONL that would carry the evidence is auto-persisted (#678) but reaped on a 6h sweep / 1h age-guard (1–7h effective), and the row carries no session id to locate it with, so the whole failure class was undiagnosable after the fact.

Measured on an ops instance: 26 success rows all carried log + session_id + cost; 6 failed rows carried none. This is the observability substrate for the Aug-14 "system agent identifies errors" initiative (reliability lane; TOWARD the #1401 recovery trace).

Fix (Approach B — inline the sanitized transcript)

Mirror the SUCCESS applier onto the FAILED path, so an error_during_execution (502) / timeout (504) row is diagnosable via the same API as a SUCCESS row.

Agent (headless_executor.py)

• New _execution_error_502_detail(ctx, message) mirrors _timeout_504_detail: {message, metadata, execution_log}, where execution_log = ctx.raw_messages (the full stream-json transcript, in-memory so present for short ≤600s runs too — no JSONL dependency).
• session_id fallback (metadata.session_id or ctx.claude_session_uuid) is UUID-shape-validated via new _valid_session_id (FI-1: the fallback can be an untrusted resume_session_id — a log-forging vector, since sanitize_dict doesn't strip newlines).
• The 502 message text is byte-identical to before, so fix(agent-server): never surface Claude Code's [ede_diagnostic] header as the error cause (#1849) #1938's error-string fix and the backend's resume-not-found self-heal (which reads detail["message"]) are preserved, and the structured body does not trip _is_reader_race_signature (no false Async chat_with_agent: long execution silently fails with null response (reader-thread) #678 auto-retry).
• _timeout_504_detail gains the same validated session_id fallback + execution_log, so the 504 path persists them for real too.

Backend (task_execution_service.py)

• _extract_agent_error returns the transcript (now a 3-tuple); the except httpx.HTTPError handler threads it + session_id onto the existing TerminalEnvelope.execution_log/.session_id fields.
• apply_result's FAILED branch mirrors the SUCCESS branch — identical sanitize_execution_log redaction + bug: schedule_executions.tool_calls stores a verbatim copy of execution_log — breaks the tool-call metric and survives log retention #1741 tool_calls summary + claude_session_id — passing the columns into the existing db.update_execution_status call, added to the SET clause above the if won gate.

Invariants preserved: single terminal applier (#1483) — no new CAS writer, _write_terminal_and_gate untouched; CAS-won close/emit (#1578/#1804) — every side-effect stays gated on won. No schema change (execution_log/claude_session_id/tool_calls already exist).

Named residuals: _write_terminal_and_gate terminals (backend timeout/budget/crash) and standalone-scheduler RETRY-001 FAILED writes still land bare — the Aug-14 initiative must not assume every FAILED row carries telemetry.

⚠️ Rollout constraint (agent-server change)

The agent half lives in trinity-agent-base. A running fleet keeps writing bare FAILED rows until ./scripts/deploy/build-base-image.sh runs AND each agent is COLD-recreated — a plain restart may not adopt it (#1809). The backend half is live on backend deploy. Old-image bodies degrade gracefully (bare string → execution_log/session_id stay null = today's behaviour), so a mixed fleet is safe both directions. Same rollout note #1944 (#1870) and #1938 (#1849) carried.

Docs updated

• docs/memory/requirements/scheduling.md — new §10.4.4
• docs/memory/architecture.md — task_execution_service.py apply_result FAILED-branch note
• docs/memory/feature-flows.md (index) + feature-flows/task-execution-service.md + feature-flows/parallel-headless-execution.md

Tests

tests/unit/test_1853_error_telemetry_salvage.py (26) — agent helpers incl. the FI-1 log-forging guard, the unchanged-message-text (#1938) + no-reader-race-collision regressions, backend FAILED-branch salvage with credential redaction proven in real sqlite, the #1944 §4.4 live-capture pattern (real on-disk error tail → real _finalize_headless_result), and the full real error tail → finalize → _extract_agent_error → apply_result → persisted redacted row chain.

Also migrated two test_1673_execution_error_not_success.py contract tests whose assertions targeted the old bare-string 502 body (now the structured dict) — intent preserved (resume-marker reachability; no reader-race collision).

All touched suites green locally (test_1853, test_1673, test_1083_apply_result, test_1201_timeout_telemetry_salvage, test_792_subscription_retry), including under pytest-randomly.

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[AndriiPasternak31]

CodeQL check — 10 pre-existing false positives dismissed

The CodeQL results check flagged 10 py/clear-text-logging-sensitive-data alerts. All 10 are pre-existing lines from Release: v0.6.0 (#998) (an ancestor of dev), not introduced by #1853:

• docker/base-image/agent_server/services/headless_executor.py (330, 341, 1444, 1474, 1475, 1538)
• docker/base-image/agent_server/services/error_classifier.py (429 ×3) — not in this PR's diff
• docker/base-image/agent_server/services/jsonl_recovery.py (848) — not in this PR's diff

Every line logs pure telemetry (cost_usd, duration_ms, session_id, num_turns, counts) or an already-sanitize_text'd message — no sensitive data.

They surfaced as "new" only because dev's stored CodeQL baseline predates the query that catches them (query-pack drift); this PR modifies headless_executor.py, so its fresh analysis emits them while the last 8 dev merges (which don't touch these files) stayed green. Dismissed as false positive (reversible).

All other checks — regression diff, 6 pytest seeds, prod-image-smoke, both Analyze jobs, container-security, guards — are green.

[vybe]

Validated via /validate-pr. The FAILED branch mirrors the SUCCESS applier and stays inside the existing db.update_execution_status CAS call — no new terminal writer, _write_terminal_and_gate untouched, and every side-effect remains gated on won (#1483/#1578/#1804 preserved). Transcript is sanitized with the same sanitize_execution_log the SUCCESS branch uses, and the 502 message text is byte-identical so #1938 and the reader-race classifier are unaffected. Old-image bodies degrade to null columns, so a mixed fleet is safe both directions. Residuals (_write_terminal_and_gate terminals, scheduler RETRY-001 writes) are named in the description rather than implied. All checks green.