Clear Working state when a model call fails before a response - #120
Merged
Conversation
A provider/HTTP error, a task-join error, or apply_model_response itself returning Err before its own transition logic ran all funnel into drain_pending_prompt's outcome_err handling in crates/forge-tui/src/app/turn.rs. Unlike the success path (which goes through apply_model_response's evaluator and always terminates the lifecycle one way or another), this error path only cleared the TUI's local `busy` flag and left the session's TaskLifecycle stuck at Working. Since the message queue's dispatch gate and start_new_task both refuse to act while Working, a single failed request permanently soft-locked the whole session: every later message queued and never sent, even after switching to a completely different, healthy provider. Only killing and restarting the process cleared it. Add AgentSession::mark_model_call_failed (mirrors mark_cancelled's shape: a lifecycle-only transition, no synthetic assistant message, since the TUI already shows the error via its own banner) and call it from the non-cancel branch of outcome_err handling. Adds a regression test that fails a turn then sends a second message and asserts it dispatches immediately rather than queuing, plus a lifecycle assertion on the pre-existing partial-stream-interruption test (which exercises the same code path but never checked this).
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
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
apply_model_responseever produces aModelResponse), the session'sTaskLifecyclestayed stuck atWorkingforever. The header showed a permanently-animating "Working" spinner, and every later message silently joined the queue and never dispatched — confirmed to survive switching to a completely different, healthy provider. Only killing and restarting the process cleared it.drain_pending_promptincrates/forge-tui/src/app/turn.rsfunnels every non-success outcome (provider/HTTP error, task-join error, orapply_model_responseitself returningErr) into a sharedoutcome_errvariable. The success path (apply_model_responsereturningOk) has full lifecycle-transition logic; this error path only cleared the TUI-localbusyflag and calledreport_errorfor the on-screen banner — nothing ever movedAgentSession::active_task.lifecycleout ofWorking. Since the message queue's dispatch gate (classify_input) andstart_new_taskboth refuse to act whilelifecycle == Working, and provider switches never touch that field either, the session was permanently soft-locked.AgentSession::mark_model_call_failed(crates/forge-core/src/lib.rs), mirroring the existingmark_cancelled's shape — a lifecycle-only transition (Working|Waiting -> Failed,TransitionReason::TurnFailure) with no synthetic assistant message, since the TUI'sreport_erroralready shows the failure to the operator and duplicating it into the transcript would just be noise. Wired into the non-cancel branch ofoutcome_errhandling inturn.rs.Test plan
cargo test --package forge-tui— 671/671, including a new regression testedge_provider_error_unsticks_session_for_the_next_message(fails a turn, then sends a second message and asserts it dispatches immediately rather than queuing) and a new lifecycle assertion added to the pre-existingedge_network_stream_interruption_preserves_partial_response(which exercises the same code path but never checked the session lifecycle before).cargo fmt --all -- --check,cargo clippy --workspace --all-targets --locked -- -D warnings,cargo test --workspace --all-targets --locked— all clean.cache_controlP0 on OpenAI Codex (header went✗ Failed · Model call failedinstead of stuckWorking), then switched to Ollama and sent two more messages through two more real errors (gemma3:4b does not support tools, then the dangling-tool-call catch from Fail turns whose model text looks like an unparsed tool call #119) — every message dispatched immediately, none silently queued, no stuck state at any point.