refactor(execution): decompose execute_task into named phases (#2314)

[dolho]

Summary

Two commits decomposing task_execution_service.py (#2314), both in-place — the module keeps its name and every module global stays put, because ~140 monkeypatch sites across the unit suites patch services.task_execution_service.*, and moving the class to a sibling module is precisely the silent-test-rot hazard (patches apply cleanly and test nothing) the issue's own Technical Notes warn against mid-#1081.

1. execution_envelope.py — TaskExecutionErrorCode / TaskExecutionResult / TerminalEnvelope carved into a leaf module, re-exported from the service so every existing import keeps resolving.
2. execute_task decomposed: the 907-line monolith becomes a ~350-line orchestrator (mostly the unchanged preamble + kwarg plumbing) over nine named phase methods:

Phase	Method	Lines
2 — admission + fast-fail terminals	_admission_gate	115
3 — dispatch activity	_start_dispatch_activity	40
3b — breaker fast-fail	_breaker_fast_fail	58
4a — prompt compose + fallback	_compose_effective_system_prompt	58
4 — call + #678/#792 retries	_call_agent_with_retries	223
5–7 — cancel/#1410/SUCCESS terminals	_finalize_sync_response	122
timeout / budget / HTTP-error handlers	_handle_timeout / _handle_budget_exhausted / _handle_http_error	35/51/89

_AttemptState replaces the old hoisted retry locals — the exception handlers read what the retries wrote (retry count, rolled-up failed-attempt cost, the #792 one-shot switch flag, the retry-reset start_time the timeout handler's elapsed derives from), and creating it before the try removes the historical NameError hazard the old hoisting comment guarded against. Phase 3c, the payload dict, the #1083 202-ACK handoff (which flips async_handoff for the finally) and the finally stay inline: each couples to orchestrator-local control flow.

_admission_gate is added to the #1804 parity allowlist with the same justification its sibling admission-path entries carry (no dispatch activity exists before step 3 — before this PR those writes sat inline in execute_task, whose own close calls satisfied the function-level scan).

What this deliberately does NOT do

The issue's headline "no module >800 lines" is not met for this file (2,398 lines with the added signatures/docstrings): meeting it requires physically relocating execute_task/apply_result, which detaches those ~140 patch sites mid-#1081 — the "big-bang" the issue's Technical Notes rule out. The four other oversized modules from #2314's list ship in #2487 as real package splits. The remaining step for this file — moving the phase methods out behind the now-explicit seams once #1081 settles — is now mechanical.

Verification

• Every moved block is byte-preserved modulo the state. renames, checked with an undefined-name AST walk over the module (it caught the one real transcription slip — a log line reading two now-out-of-scope names, repointed at the payload).
• All 89 test files that touch task_execution_service / execute_task: 1,844 passed, 8 skipped. The single failure is test_2467_turn_integrity's fleet-list arm — the dev-wide hardcoded-timestamp time bomb (fires after 2026-09-02T10:00Z on every branch), fixed separately in fix(tests): defuse the hardcoded-timestamp time bomb in the #2467 fleet-list test #2488.

Closes #2314

🤖 Generated with Claude Code

https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

[obasilakis]

/validate-pr: APPROVE

The terminal/CAS contract is preserved by construction, which is the thing that had to be proven for a refactor of this file. Verification run against PR head:

• AST function-level diff, base ↔ head: every pre-existing function is byte-identical. Only execute_task changed (907 → 350 lines) plus 9 private phase methods added. apply_result and _write_terminal_and_gate — the two owners of the CAS won gate, spawn_task_terminal_event (#1578) and close_execution_activity (#1804) — are untouched.
• Flattened, order-sensitive call sequence (orchestrator with phase calls inlined) is identical to base. No reordering, no dropped or added side effect. Counts match exactly: spawn_task_terminal_event 3/3, close_execution_activity 3/3, record_outcome 3/3, update_execution_status 9/9, _write_terminal_and_gate 6/6.
• activity_id is threaded into all 5 exception paths; _breaker_fast_fail (step 3b, post-activity) correctly routes through _write_terminal_and_gate with activity_id and agent_name (task_execution_service.py:1257-1263).
• The CancelledError handler keeps its if won: gate on the close (:1190); the finally slot release is still gated on slot_acquired and not async_handoff (:1206), both flags initialised before the try.
• _AttemptState mutation sites map 1:1 to base's retry locals — 10 sites, same order and operators.
• tests/unit/test_1804_terminal_activity_parity.py runs green against PR head (6 passed). The new _admission_gate allowlist entry is justified correctly: its three terminals all fire from capacity.acquire at step 2, before step 3 opens the dispatch activity.

All 24 checks pass.

Warnings

• #2314 AC#1 ("no resulting file over 800 lines") is explicitly unmet — the file grew 2,202 → 2,397 lines — and the PR uses "Related to #2314" rather than a closing keyword, so the issue stays status-in-progress. Both are honestly disclosed in the body, and the remaining move is described as mechanical post-#1081. Worth a note on #2314 that #2489 and #2487 are partial delivery, so the P-16 debt slot doesn't read as stalled.

Suggestions

• _AttemptState is constructed before the try (:984), whereas base assigned the retry locals just above the agent call. That's a small positive behaviour delta in a "pure refactor" — it removes a latent NameError if steps 2–4a ever raised into except httpx.HTTPError. Disclosed in the body and no reachable path changes, so no objection.
• execution_envelope.py:29 carries a vestigial @dataclass on class TaskExecutionErrorCode(str, Enum), moved verbatim from base. A leaf-module carve-out is the cheapest moment to drop it.

---

Findings produced by /validate-pr (Claude Code).