fix(core): explicit lifecycle contract — plain text never ends a run; respond_to_user / wait_for_agents split - #954
Conversation
Interactive turns ended by plain text left the agent parked in 'waiting' forever. Require an explicit lifecycle tool in both modes and nudge a text-only turn back into a tool call, bounded by a recovery limit.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryThe PR makes agent lifecycle transitions explicit: plain-text turns are retried rather than ending runs, while separate tools now represent waiting for users and waiting for agents.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported parent-stranding issue is addressed because the stalled child now queues a notice and wakes its waiting parent. Important Files Changed
Reviews (4): Last reviewed commit: "docs(prompts): text-only turns no longer..." | Re-trigger Greptile |
An exhausted agent parked in 'waiting' got a fresh nudge budget on every 600s auto-resume, so a wedged agent could nudge-park-nudge indefinitely. Track the count on the coordinator, snapshot it, and reset it only on real input or an explicit lifecycle tool.
Runtime verification — lifecycle contract + Phase 3 durabilityHarness-driven runtime testing of the real Phase 3: auto-resume no longer hands out a fresh nudge budget (the key result)Same unmodified test, pre-Phase-3 vs this branch:
A genuine user message still restores a full budget of 3 — so this doesn't over-correct into "a real user can no longer un-wedge the agent". Original regression: text-only turn no longer strands the runPre-fix This branch, identical script: Durability, yielding, bounds, autonomous mode, resume semantics
Caveat: the |
Live E2E verification — real sandbox, real model, real TUIFollow-up to the earlier harness testing: a live interactive scan against The original bug was provoked live — and the recovery path caught itAsking the root agent a plain conversational question produced a bare-text turn, exactly the condition that used to strand a scan forever. The nudge fired and the model yielded properly 1.6 s later: The agent woke, answered in the TUI, and re-parked — scan stayed alive: Notable finding: Bounded recovery, whole-run counts
One agent exhausted its budget and parked rather than looping or crashing: Golden path and --resumeThe run reached a clean After killing the scan mid-flight, Caveat: No product defects found. |
A mutual wait between two agents resolves only when both hit their cap, so the ceiling is the worst-case idle burn. Name the constants instead of repeating the literal, and align the interactive auto-resume timeout.
Parking is self-service only for the root, which the user is watching. A parked child owes its parent a report it can no longer send, so the parent would wait out its full timeout for nothing.
The user can message any agent from the TUI, not only the root, so the justification is that the parent is an agent with no other way to learn the child parked - not that the child has no human resumer.
…or_agents One tool was doing three jobs (wait on the user, wait on other agents, and - wrongly - wait for a long-running command), so the driver had to guess which one an agent meant and used parent_id as the proxy: the root waits for a human, everyone else waits for agents. That proxy is wrong, since the user can message any agent from the TUI's agent tree. Tool identity now carries the intent, and the coordinator records it as a wait_kind that survives snapshot/restore: respond_to_user -> wait_kind="user", never auto-resumed (root or not) wait_for_agents -> wait_kind="agents", auto-resumed on a 300s timer recovery exhaust -> wait_kind="stalled" respond_to_user fuses the message and the yield into one call, so there is no way to answer and then forget to stop - the two-step that gpt-4o-mini skipped 2/2 in live testing. Plain text still renders as before. Auto-resume is also bounded now: an agent that re-parks after every timeout burned a model turn every 300s for the rest of the scan (and, since parked children notify their parent, spammed the parent's inbox on the same cycle). After _MAX_IDLE_AUTO_RESUMES it stays parked until a real message arrives.
Live E2E verification —
|
| Metric | Count |
|---|---|
respond_to_user / wait_for_agents / wait_for_message |
8 / 28 / 0 |
| Nudge WARNINGs (1/3, 2/3, 3/3, 4/3) | 20, 5, 1, 0 |
Exhaustion→stalled parks |
1 |
Idle auto-resume (agents kind) |
1 (idle_resume_counts={'d35fe68b':1}) |
finish_scan / MaxTurnsExceeded / strix.core errors |
4 / 0 / 0 |
No wait_kind="user" agent was ever auto-resumed (the regression this design prevents).
--resume restored the 25-agent tree + full conversation, with both new fields persisted:
Resume: restored coordinator with 25 agent(s); root=24808c39
wait_kinds={"24808c39":"user","f22192b2":"user"} idle_resume_counts={"d35fe68b":1}
Caveats: the idle-resume cap→stalled (3× auto-resume) wasn't reached live (needs ~15 min idle; covered by unit tests), and the nudge's respond_to_user wording is verified from source since injected model input isn't logged.
|
@greptileai review |
Summary
In interactive mode, a model turn that ended with plain text and no tool call silently killed the run. The Agents SDK treats a no-tool message as a final output, and
_settle_run_resultthen flipped the still-runningagent towaiting, so the outer loop blocked oncoordinator.wait_for_message()— for the root agent withtimeout=None, i.e. forever. An autonomous scan was stranded mid-work because the model forgot to call a tool, not because it was done.Non-interactive mode already handled this exact slip (
_run_noninteractive_until_lifecycle: detect status stillrunning, inject a "call a tool" nudge, retry, bounded). This makes that the contract for both modes: status transitions only happen via an explicit lifecycle tool call, and plain text is inert for lifecycle purposes.Plain text is still streamed to the user exactly as before — this changes what text means, not whether it's shown.
finish_scan(root) /agent_finish(subagent)completedrespond_to_userwaiting, indefinitelywait_for_agentswaiting, boundedexec_command/write_stdinLoop (
strix/core/execution.py)_run_noninteractive_until_lifecyclebecomes_run_until_lifecycle(..., interactive), and both the first cycle and every post-user-message cycle route through it:Two deliberate asymmetries remain, both about who can resume:
_INTERACTIVE_TOOL_RECOVERY_LIMIT = 3; autonomous keepsmax(1, max_turns).waitingwith a logged warning — a human is present, so the scan stays resumable by messaging the parked agent. Autonomous still raisesMaxTurnsExceededand markscrashed, since nobody is there to resume it.A parked subagent additionally notifies its parent, mirroring
_notify_parent_on_terminalon the autonomous path:The human can revive any parked agent from the TUI, but the parent is an agent, not a watcher: nothing else tells it the child stopped, so it burns its whole
wait_for_agentstimeout on a completion report that is never coming. This is a separate notice from_TERMINAL_NOTICEbecause the child is not terminal — a message can still revive it — so the parent is told to either send a concrete next step or stop waiting, rather than the terminal notices' flat "stop waiting on this child"._settle_run_resultis deleted outright: it was the only path that producedwaitingwithout an explicit yield tool, which is exactly the bug.One wait tool split into two, and a
wait_kindto drive timeoutswait_for_messagewas doing three jobs — wait on the user, wait on other agents, and (silently wrongly) wait for a long-running command. Since one tool call could mean any of them, the driver had to guess which, and it usedparent_idas the proxy: the root waits for a human forever, everyone else gets a 300s re-check. That proxy is false, becauseStrixTUI._send_user_messagesends toself.selected_agent_id— the user can message any agent in the tree, including a parked child.So the intent now rides on tool identity, which the model cannot get wrong the way it can get a parameter wrong, and the coordinator records it:
_plain_waiting_timeoutreads that instead of the tree:wait_kindsandidle_resume_countsgo into theagents.jsonsnapshot, so a resumed scan still knows what each parked agent was waiting on.respond_to_user(new,strix/tools/respond/tool.py, registered only wheninteractive=True) fuses the message and the yield into a single call. The old contract asked the model to do two things — emit the answer, then remember to call the wait tool — and the model's job feels finished after the first;gpt-4o-miniskipped step two 2/2 in the live run below. With one call there is no way to answer and forget to stop, and no way to stop without having answered. Plain text still renders as it always did, so narrating mid-task costs nothing;respond_to_useris specifically the act of waiting.wait_for_agents(renamed fromwait_for_message, no alias) keeps the old peer/child semantics and now documents what it is not: it never watches a process, so using it to wait out a long-running command burns the full timeout even if the command exited a second later. The right answer is the shell tool's own polling —exec_commandreturns a session id andwrite_stdin(chars="")returns the moment there is new output or an exit.Bounded auto-resume. An agent that re-parks after every timeout used to burn one model turn every 300s for the rest of the scan; with the persisted recovery counter it re-parks after a single cycle each time, and with the new stall notice it also re-spammed its parent's inbox on that cadence. After
_MAX_IDLE_AUTO_RESUMESconsecutive resumes with no message, it is left parked (wait_kind="stalled") until something real arrives. A genuine message resets the budget.Durability: the counter is coordinator state, not loop-local
A loop-local counter resets whenever the driver is re-entered — including the
_plain_waiting_timeoutauto-resume for subagents. A wedged agent would then nudge → park → auto-resume → get a fresh budget → nudge again, forever (bounded only by scan budget), and would also forget its progress across--resume.So the count lives on
AgentCoordinator(recovery_counts) and is included in theagents.jsonsnapshot. It resets only on genuine progress:An agent restored at its cap therefore parks again after one further text-only cycle rather than replaying the whole budget. Existing resume semantics needed no change —
start_parked=bool(interactive and is_resume and root_status != "running")inrunner.pyalready continues a restoredrunningagent and leaves a deliberatelywaitingone parked, which is exactly the behavior the new contract wants.Wait ceiling halved to 300s
Two agents waiting on each other resolve only when both hit their cap, so the ceiling is the worst-case idle burn — 10 minutes was too generous.
_WAIT_DEFAULT_TIMEOUT_S = 300with_WAIT_HARD_CEILING_S = 301enforced by the SDK around the whole tool call (so it also caps an oversizedtimeout_secondsthe model asks for), and_WAITING_AUTO_RESUME_TIMEOUT_Sdrops to match. Named constants instead of the repeated literal.Prompt / tool contract, and the UI
The interactive prompt previously asserted the opposite of the new runtime behavior ("A message WITHOUT a tool call IMMEDIATELY STOPS your entire execution", "reply in plain text and stop"), so it had to be inverted in the same change or prompt and runtime would disagree. It now states that plain text never ends a turn, that
respond_to_useris the only way to yield to the user, and thatwait_for_agentsis for peers and children only. The autonomous section drops its yield-to-user language entirely, since no user is attached there.Tool names are hardcoded in the renderers, so both surfaces are updated: a TUI
RespondToUserRendererand a viewerRespondRendererrender the reply as the agent's own markdown prose rather than as a tool call, and the viewer's prebuilt bundle is rebuilt (npm run build) so the rename doesn't silently fall back to the generic renderer.Testing
make check-all(ruff + format + mypy) and all pre-commit hooks pass;npx tsc --noEmitclean for the viewer. Full suite: 651 passed, 1 failed —test_finish_scan_bypasses_active_agent_guard_after_reserve, which fails identically on a cleanmain(ReportStatehas no attributerun_record, pre-existing and unrelated).New coverage on top of the nine lifecycle tests:
respond_to_userparks withwait_kind="user", is rejected in an autonomous run, and takes an already-arrived message instead of parking; it is registered only in interactive mode whilewait_for_agentsis registered in both; an agent awaiting a human is never auto-resumed whether or not it is the root; an agent awaiting agents is; idle auto-resumes stop after their budget and a real message restores it; andwait_kind+ the idle counter survive a snapshot round-trip.One real regression was caught by
tests/test_e2e_budget_lifecycle.pyrather than by a new test: its fake runner never calls a tool, so under the new contract every turn is a forgotten-tool-call turn and the agents burned 3 turns per wake, blowing past the 90% sub-agent reserve straight into the 100% cap. The fixture now parks explicitly after each turn, standing in for the yield tool a real turn ends with — the budget lifecycle it actually tests is unchanged.Beyond unit tests the earlier phases were verified two ways, both in PR comments below:
run_agent_loop/_run_until_lifecyclewas run against older commits: pre-fixmainstrands the run (1 cycle,waiting, still blocked after 15s) where this branch reachescompleted; and pre-Phase-3 hands out a fresh nudge budget on every auto-resume (3.00 cycles per auto-resume vs 1.00 here).--resume. The bug was provoked live: a plain question produced a bare-text turn, the nudge fired, and the model yielded properly 1.6s later instead of stranding the scan. 20 nudges across the run, none exceeding the cap, one clean exhaustion park, zero tracebacks, cleanfinish_scan.Two caveats worth stating plainly. In that live run
gpt-4o-minianswered with bare text 2/2 times, i.e. it did not obey the prompt's instruction to yield via a tool — the loop guardrail is what kept the run alive, so the safety net is load-bearing rather than redundant. And the tool split landed after that run:respond_to_useris designed to remove exactly that two-step failure, but it has not itself been exercised against a live model.Link to Devin session: https://app.devin.ai/sessions/fd12121ff02b4056a72a1e284b31c1d3
Requested by: @0xallam