refactor(exec): fire-and-forget dispatch — a hung turn holds zero backend resource (#1083)#1273
Merged
Conversation
…endpoint (#1083 PR1) PR1 of fire-and-forget dispatch — a pure refactor plus a fully-hardened, fail-closed result-callback endpoint that rejects all live traffic until PR2 sets the durable async marker. Zero behavior change to current execution paths. - Extract TaskExecutionService.apply_result — the single terminal applier shared by the inline sync path and the future result-callback. Moves the SUCCESS and the httpx #678-salvage FAILED terminal writes (sanitize, cost rollup, context, CAS write, activity completion, breaker outcome, optional slot release) behind one normalized TerminalEnvelope. Every side effect is gated on the CAS bool (Codex #1/#12): a CAS-lost write does nothing — no double activity close, breaker churn, or slot drain. Producer-side classification (SUB-003, error-code, timeout-terminate) stays in execute_task. - slot_service.release_slot: gate the BACKLOG-001 drain on the ZREM result so a replayed/no-op release can't admit a backlog row past max_parallel_tasks (Codex #12). Every legitimate drain trigger removes a present member. - POST /api/agents/{name}/executions/{id}/result — agent's own MCP-key auth (mirrors heartbeat authorize_heartbeat) + ownership + a durable async-marker gate (RUNNING rows must carry claude_session_id='dispatched_async', else 409) + idempotent replay (terminal → {replayed:true}) + body-size 413 caps. - db.get_open_activity_id_for_execution: filtered by related_execution_id AND chat/schedule_start AND state='started' so a shared-eid tool_call row can't be cross-closed (Codex #8). mark_execution_dispatched gains async_dispatch=True. - config: DISPATCH_ASYNC flag + ASYNC_DISPATCH_ELIGIBLE_TRIGGERS={schedule,webhook} + dispatch_async_eligible(); compose/.env forwarding (backend-only, default off). Tests: apply_result golden + CAS-gate matrix, callback auth/ownership/replay/ marker/size matrix, DB seams; existing CAS-gate + #678 auto-retry parity green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#1083 PR2 lane A) Backend half of the fire-and-forget cutover (agent-server half follows). Inert until DISPATCH_ASYNC=true AND a Claude-runtime agent on a new base image ACKs 202. - execute_task: for an async-eligible trigger ({schedule,webhook}) under DISPATCH_ASYNC, send async_result=true, write the durable 'dispatched_async' marker, and on a 202 ACK return RUNNING/dispatched_async immediately, handing the slot lease to the result callback (skip the `finally` release). Any non-202 response (200 / old image / non-Claude runtime) falls through to today's synchronous handling — the safe mixed-fleet fallback. The runtime gate is enforced agent-side (decision 5). - cleanup_service lease reaper: after fail_stale_slot_execution, tag the FAILED message with the lease_expired code and close the open dispatch activity via the filtered get_open_activity_id_for_execution (the absent fire-and-forget coroutine `finally` no longer closes it). Adds TaskExecutionErrorCode.LEASE_EXPIRED. - Finding 1: _sweep_stale_executions now uses each agent's timeout+SLOT_TTL_BUFFER window (mark_stale_executions_failed gains agent_timeouts+buffer_seconds) instead of the flat 120-min default, so a legitimately-running max-timeout async turn isn't failed ~5 min before the slot reaper / canary E-01. agent_timeouts=None reproduces the prior flat behaviour exactly. Tests: dispatch-return matrix (202→RUNNING/no-release, non-202 fallback, trigger scope), stale-sweep per-agent boundary REGRESSION (timeout+buffer±ε), lease activity close; existing CAS-gate / cleanup / observability tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#1083 PR2 lane B) Agent-server half of the fire-and-forget cutover. When the backend sends async_result=true AND this agent runs the Claude runtime, /api/task accepts with 202 and runs the turn in a detached task that reports the typed terminal to the backend's result-callback endpoint. Inert for non-Claude runtimes / old behaviour (async_result defaults false). - services/result_callback.py: try_spawn_async gates on async_result + Claude runtime + execution_id + callback creds (else the caller runs synchronously — the non-202 fallback). _run_and_report runs the headless turn, builds a typed envelope (success → completed; HTTPException → status-mapped error_code/ terminal_reason, with metadata salvaged from the structured 502 body), persists it atomically to ~/.trinity/pending-results/<eid>.json, and delivers it with capped backoff up to the slot-lease deadline (dispatch + timeout + SLOT_TTL_BUFFER), deleting on a 2xx or a permanent 4xx. A strong-ref _inflight set defeats the asyncio weak-ref GC footgun. - main.py: startup sweep re-sends any envelope left on disk by a crash/restart, so completed work isn't lost to a phantom LEASE_EXPIRED (a late SUCCESS still overwrites it via the backend CAS). - chat.py /api/task: 202 branch before the synchronous path; models.py: ParallelTaskRequest.async_result flag. v1 limitation (T8 / #1201, P2 fast-follow): only the 502 empty-result failure path carries cost/context metadata on the async callback; 504/503 write a null-cost row until execute_headless_task exposes ctx.metadata on those paths. Tests: envelope mapping, eligibility gating, persist/resend roundtrip, and the retry-to-deadline delivery loop (2xx, permanent-4xx no-retry, transient-5xx retry, deadline). agent-server regression subset green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ult-callback endpoint (#1083) architecture.md gains a "Fire-and-Forget Dispatch (#1083)" cross-cutting block (apply_result CAS-gating, durable async marker, callback endpoint, agent-side persist/retry/sweep, lease reaper + stale-sweep buffer fix, v1 boundaries), the result-callback endpoint row, and a pointer from the task_execution_service catalog entry. feature-flows index gains the #1083 Recent Updates row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#1083, Codex #2) The result-callback's idempotent-replay short-circuit fired for ALL terminal statuses, which would block a genuinely-late SUCCESS callback from reaching the CAS after the lease reaper had already FAILed the row (LEASE_EXPIRED) — directly contradicting the plan's accepted "SUCCESS overwrites a phantom FAILED" behavior. Now only the authoritative terminals (SUCCESS/CANCELLED/SKIPPED) short-circuit as a replay ACK; a FAILED row falls through to apply_result, whose CAS lets a late SUCCESS overwrite it (a duplicate FAILED is harmlessly CAS-blocked → no side-effect re-run). The async-marker gate still rejects a FAILED *sync* row (marker != dispatched_async), so the cross-path guard holds for terminals too. +2 callback tests (FAILED-async fall-through, FAILED-sync 409); architecture.md clarified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Defense-in-depth: result_callback builds a pending-results filesystem path
(~/.trinity/pending-results/{execution_id}.json) and the backend callback URL
from the backend-supplied execution_id. Add a strict allowlist guard
(_SAFE_EXECUTION_ID = ^[A-Za-z0-9_-]{1,128}$) enforced at try_spawn_async — a
value outside the token_urlsafe / UUID charset now falls back to synchronous
handling and never reaches the path build or the callback URL.
Adds parametrized unit tests and archives the CSO --diff audit report for the
branch.
Refs #1083
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try: | ||
| _PENDING_DIR.mkdir(parents=True, exist_ok=True) | ||
| tmp = _pending_path(execution_id).with_suffix(".json.tmp") | ||
| tmp.write_text(json.dumps(record)) |
| _PENDING_DIR.mkdir(parents=True, exist_ok=True) | ||
| tmp = _pending_path(execution_id).with_suffix(".json.tmp") | ||
| tmp.write_text(json.dumps(record)) | ||
| tmp.replace(_pending_path(execution_id)) |
|
|
||
| def _delete(execution_id: str) -> None: | ||
| try: | ||
| _pending_path(execution_id).unlink(missing_ok=True) |
CodeQL py/path-injection flagged _persist/_delete: a caller-side regex guard in try_spawn_async is not recognized as a barrier for the callee sink. Move the sanitizer into _pending_path itself — resolve() + is_relative_to() containment against _PENDING_DIR (mirrors jsonl_recovery), raising ValueError on escape. The regex stays as the belt; this is the suspenders on the path-build dataflow, so a hostile execution_id can never traverse out of the pending dir. Refs #1083 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1083, CodeQL) CodeQL py/path-injection did not accept resolve()+is_relative_to() as a barrier and still flagged the path build in _pending_path/_persist/_delete. Switch to the canonical CWE-022 sanitizer: os.path.basename strips any directory components from execution_id before the join, so the write is confined to _PENDING_DIR. The resolve()+is_relative_to() containment stays as suspenders; the regex guard in try_spawn_async remains the belt. Refs #1083 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#1083, CodeQL) resolve()+is_relative_to() and os.path.basename were both ignored by CodeQL's py/path-injection model (same dead end #950 hit before switching). Mirror the guard that actually cleared the alert there: os.path.normpath collapses any '..', an inline startswith prefix-check confirms containment under _PENDING_DIR, and the normalized value is flowed downstream to write/replace/unlink. Raises on escape; the try_spawn_async regex belt still rejects such ids upstream. Refs #1083 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # docs/memory/feature-flows.md
vybe
approved these changes
Jun 19, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
Validated via /validate-pr: P1 reliability refactor (#1083). CSO --diff archived CLEAR (0 CRIT/HIGH/MED); callback endpoint hardened (agent-own-MCP-key→403, ownership→404 no-enum, dispatched_async marker→409 so it's inert until engaged, 413 caps, execution_id allowlist + #950 normpath/startswith containment). DISPATCH_ASYNC flag default-OFF + non-202 sync fallback → mixed-fleet safe; apply_result CAS-gates all side-effects; config properly packaged (both compose + .env.example); no schema change; ~120 unit tests; docs complete. Conflict was docs-index-only (feature-flows.md), resolved keeping both #1082/#1083 rows. Approving.
vybe
enabled auto-merge (squash)
June 19, 2026 12:55
| _PENDING_DIR.mkdir(parents=True, exist_ok=True) | ||
| tmp = _pending_path(execution_id).with_suffix(".json.tmp") | ||
| tmp.write_text(json.dumps(record)) | ||
| tmp.replace(_pending_path(execution_id)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements fire-and-forget dispatch (#1083): eligible autonomous turns are dispatched to the agent with a 202 ACK and run in the agent's background, then POST their terminal back to a new callback endpoint that finalizes the row.
execute_taskreturns right after the ACK, so a wedged turn holds zero backend coroutine and the slot becomes a lease (released by the callback or reclaimed by the existing TTL reaper).DISPATCH_ASYNC(default OFF) AND Claude-runtime only. Non-202 responses (old image / non-Claude / flag off) fall through to today's synchronous path — the safe mixed-fleet fallback.{schedule, webhook}only.loop/fan_outstay sync;eventbypassesexecute_task.What's in this PR
task_execution_service.apply_result) — single point that finalizes an execution, shared by the inline sync path and the callback; gates all side-effects on the CAS bool so a replay / late callback completes no activity, records no breaker outcome, releases no slot.claude_session_id='dispatched_async') + lease reaper incleanup_service(expired lease → FAIL + close activity, using each agent'stimeout + SLOT_TTL_BUFFERwindow).POST /api/agents/{name}/executions/{id}/result: agent's own MCP key + ownership (404) + marker gate (409) + idempotent replay + body-size 413 cap. A late SUCCESS can still overwrite a reaperLEASE_EXPIREDvia the CAS (Codex Feature/gemini runtime support #2).result_callback.py) —try_spawn_asyncruns the headless turn, builds the typed terminal envelope, persists it to~/.trinity/pending-results/<eid>.json, delivers with capped backoff up to the lease deadline; a startup sweep re-sends leftover envelopes after a crash/restart.execution_idallowlist (^[A-Za-z0-9_-]{1,128}$) enforced attry_spawn_asyncso a malformed/hostile id can never traverse out of the pending dir or inject into the callback URL (defense-in-depth; falls back to sync). CSO--diffaudit report archived underdocs/security-reports/— 0 CRITICAL/HIGH/MEDIUM, the one LOW finding resolved in-branch.v1 boundaries
lease-expiry = FAIL (not re-queue); async empty-result = FAIL (the #678 inline auto-retry stays sync-only); SUB-003 auto-switch stays inline-only; 504/503 async failures write a null-cost row until
execute_headless_taskexposesctx.metadataon those paths (#1201 fast-follow).Tests
Unit:
test_1083_apply_result,test_1083_callback_endpoint,test_1083_dispatch_return,test_1083_lease_reaper,test_1083_db_seams,test_1083_result_callback(incl. parametrized unsafe-execution_idcoverage). Plus integrationtest_1083_callback_endpoint.Closes #1083
🤖 Generated with Claude Code