Repository navigation
Keep each turn's outcome through engine finalization and use it for workflow and cron completion - #481
Conversation
Internal second-model review: adjudication log (click to expand)Pre-publication review by an independent model (engine: codex; 3 findings) plus my own review.
Severity: ❌ blocker / Session id: cron:clickhouse-review-slot-8:20261001-135100 |
|
@groeneai is it still relevant? If so - resolve the conflicts |
…orkflow and cron completion AgentEngine.run() returns normally for every turn that does not raise out of the engine, so its callers could not tell how a turn ended. Cron logged "success" for any normal return, and workflow runs inferred failure from sessions.status, which a failed turn deliberately leaves (or restores to) "active". A max-turns, API-error, aborted or crashed turn was therefore reported as successful automation. The engine now records a TurnOutcome (completed / failed / interrupted, plus the reason) where each persisted turn ends: - run_cron / run_persistent_cron return TurnResult(text, outcome) for the invocation's own turn (via _run_turn, which holds run()'s old body; run() still returns str), and cron logs "error" with the reason when the outcome is not completed. - watch_turns(session_id) reports the latest outcome of a session's persisted turns, including autonomous background-continuation turns. A workflow run watches its own session over the launch and the existing quiescence loop and ends "failed" when that outcome is not completed; a missing outcome fails closed. The session-status inference is gone, so turn failure no longer depends on session health. - The outcome is published only after the turn is persisted (_finalize_turn wraps the old body, now _persist_turn); a persistence failure or a cancel while saving publishes failed / interrupted. - The autonomous drain exits that persist a partial turn (idle timeout, stream end, cancel) set the matching status instead of "completed". - Every abnormal backend end gets an inline "Turn <status>[: reason]" notice (previously only failed turns with an error did) and one WARNING log line. Scope follows the owner's choice of option A on ClickHouse#292: a parked isolated cron run is logged from its launching turn, content-less autonomous turns are not tracked, and the pre-query drain desync is left alone. Closes ClickHouse#292 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c1db878 to
3bd7f48
Compare
|
Yes, it is still relevant: on current main (5133895) a cron run whose turn hits max turns, is aborted or crashes is still logged I rebased the branch onto main and resolved the conflicts; it is still one commit (3bd7f48). The main adaptation: |
Closes #292 (the remaining scope; #428 fixed the Claude translator).
What breaks today
AgentEngine.run()returns normally for a turn that hit max turns, got an API error, was aborted or crashed after streaming. Cron then logssuccess, and a workflow run endsdonebecause it readssessions.status, which such a turn leaves (or restores to)active. An aborted turn gets no inline notice, and abnormal backend results are not logged.Change
TurnOutcome(completed/failed/interrupted+ reason) where each turn ends. A normally finished turn publishes it only after it is saved; a failed save publishesfailed.run_cron/run_persistent_cronreturnTurnResult(text, outcome)for their own turn; cron logserrorwith the reason.run()still returnsstr.engine.watch_turns(session_id)reports the latest outcome of a session's turns, background continuations included. A workflow run watches its own session through the existing quiescence wait and endsfailedon an abnormal outcome (a missing one fails closed). The session-status check is gone, so turn failure stays separate from session health.Turn <status>[: reason]notice and one WARNING line. The engine's own crash, stop and timeout paths keep their existing notices and log lines.Scope is option A as agreed in #292 (#292 (comment)). Unchanged limits: a parked isolated cron run is logged from its launching turn; a workflow follow-up turn that ends without content does not change the result; the pre-query drain desync is untouched.
Behaviour changes
error, so they do not anchor the interval and are caught up once at restart. I can give interrupted runs their own status if you prefer.failed; a stop through Nerve is still recordedkilledby the stop listener.Validation
tests/test_turn_outcomes.py: 26 integration tests (real engine and database) over the engine, cron and workflow paths; each of 13 mutations of the change fails them. Full suite: 6438 passed (main: 6412).🤖 Generated with Claude Code