feat(generation): support user cancellation of a running session#48
Merged
Conversation
Cancelling a generation previously just called fail_generation_session, which sent an "❌ Run Failed" notification, marked the session FAILED (indistinguishable from a real failure), and never stopped the running workflow task or its agent subprocesses. Introduce a distinct terminal CANCELLED status and make cancellation actually stop in-flight work through two mechanisms: - A process-local generation_id → asyncio.Task registry so the owning pod calls task.cancel() for immediate teardown. The task registers itself via asyncio.current_task(), so no plumbing is needed at the create_task sites (initial run, retry, and boot recovery). - Cooperative raise_if_cancelled() checks (throttled DB reads) in the orchestrator step loop, execute_all_phases, the parallel executors, and the P10Y poll loop, raising GenerationCancelledError as a cross-pod-safe fallback. The cancel endpoint sets CANCELLED first, then cancels the task, so the injected CancelledError and any GenerationCancelledError unwind against an already-terminal state and neither re-fails nor notifies. The state machine cancel() mirrors interrupted_by_shutdown(): notification-free, sets cancelled_by_user/cancelled_at markers, ends the API-key slot, and retains workspaces (Commandment II) — generated code is preserved and reclaimed by scheduled_wipe. agent_query now aclose()s the SDK query stream so the Claude Code CLI subprocess is torn down on cancel. A CANCELLED session is terminal: boot recovery and the stuck detectors already exclude it, and retry now refuses it with a clear message.
raise_if_cancelled used 0.0 as the "never checked" throttle sentinel. time.monotonic() has no fixed epoch, so on a freshly-booted host (e.g. a CI runner) the clock can be smaller than min_interval_s, making now - 0.0 < min_interval_s true and throttling the very first read. Use a None sentinel so the first check for a generation always reads.
Replace the module-level global dict + functions with a GenerationTaskRegistry class. One instance is created at app startup and stored on app.state; the run/retry/cancel endpoints receive it via a get_generation_task_registry dependency, and it is threaded into the workflow task bodies (and boot recovery) as a parameter. This removes mutable import-time global state, aligns with the codebase's dependency-injection and OOP conventions, and lets tests inject the registry via dependency_overrides instead of reaching into module internals. Behavior is unchanged: same process-local fast cancel backed by the cooperative status check.
Wire the backend cancellation (PR #48) into the Textual TUI. Detail screen (DashboardScreen): add an `x` → cancel binding, shown only while the run is active (check_action gates it to pending/initializing/ running). Confirming pops a ConfirmScreen, then calls the existing DELETE /api/v1/generation-sessions/{id} for that specific session and refreshes in place — no terminal suspend, since cancellation is silent. Sessions list: a cancelled row now renders "⊘ CANCELLED BY USER". Recognize the status everywhere it matters: add a CANCELLED pill and put "cancelled" in TERMINAL_STATUSES (so the dashboard stops polling and the header pill renders), keep the cli_service mirror in sync, announce a cancel as a neutral "Generation cancelled" desktop milestone instead of a failure, and add CANCELLED to the mcp_server GenerationStatus mirror for client/backend parity.
Three cleanups to the cancellation wiring: - raise_if_cancelled now no-ops on a None db_adapter, so callers invoke it unconditionally. db_adapter is None only in DB-less unit tests; every real run wires a generation_session_service. Removes the guard from the execute_all_phases call site and centralizes it in one place. - task_registry is now a required parameter of _run_generation_session_workflow, rerun_generation_session, and recover_interrupted_sessions, dropping the "if task_registry is not None" guards. It is always injected from app.state in production; the optionality was only test convenience, so the tests now pass a registry explicitly. - Remove the P10Y cooperative-cancellation plumbing. By the time P10Y runs the code is generated and OUTPUTS_ARCHIVED (Steel Commandment XI), task.cancel() still interrupts its poll sleeps on the owning pod, and a cross-pod cancel during P10Y lands the session in CANCELLED with no spurious failure notification (fail() rejects the terminal state before it can notify). Reverts the extra params on trigger_and_poll_p10y_metrics / _process_single_workspace, the generation_args additions, the top-level check, and the estimation parallel executor's cancellation branch.
mkonopelski-gd
approved these changes
Jul 14, 2026
mkonopelski-gd
added a commit
that referenced
this pull request
Jul 14, 2026
… core Following the cancel flow (PR #48): retry validation belongs to the backend state machine (reset_for_retry accepts FAILED or stuck-PENDING), so the client-side status pre-check was a second validator — and it had drifted, refusing PENDING that the backend accepts. Drop the pre-check and the outcome sum type. services.retry.retry_generation_core is now a one-call helper: POST /retry, return the parsed dict, raise on failure. Each surface POSTs and renders success vs the backend's error message in its own style, mirroring _cancel_flow: - TUI _retry_flow: toast on success, MessageScreen with the backend reason on error. - CLI: print + exit 0/1. - MCP: chat_json; drops the client-derived `code`/pending-guidance fields in favour of the backend's message (single source of truth). No client status round-trip before the POST. Tests updated (core, CLI, MCP, TUI). make unit-tests: 671 passed.
mkonopelski-gd
added a commit
that referenced
this pull request
Jul 14, 2026
* fix(tui): give retry in-app feedback via a shared retry core Pressing retry on a failed generation suspended the full-screen TUI to the raw terminal (print + Enter prompt), which reads as a crash. The retry decision logic was also duplicated between the CLI handler and the MCP tool. Extract one print-free core, services.retry.retry_generation_core, returning a sum type (AlreadyRunning / PendingRejectedBeforeCodegen / PendingNotFailed / Queued / BackendError) — every field required, no Optionals. It operates on an already-resolved id; resolution and the "no session" case stay at each surface where they genuinely differ. - TUI: _retry_flow calls the core directly and renders in-app (toasts for queued/already-running, MessageScreen for pending/backend errors); no more suspend-to-terminal. actions.do_retry removed. - CLI (cmd_retry_generation) and MCP (retry_generation) become thin callers of the core, keeping their own print/exit-code and chat_json rendering. CLI now also guards the pending state and returns a clean exit 1 on backend error instead of crashing. - refresh_status now re-arms the poll timer when a run leaves a terminal state (failed -> retry -> pending), fixing a latent staleness bug. Tests: new test_retry_core.py and test_server_retry.py; CLI/TUI tests updated. make unit-tests: 673 passed. * refactor(retry): defer eligibility to the backend, collapse the retry core Following the cancel flow (PR #48): retry validation belongs to the backend state machine (reset_for_retry accepts FAILED or stuck-PENDING), so the client-side status pre-check was a second validator — and it had drifted, refusing PENDING that the backend accepts. Drop the pre-check and the outcome sum type. services.retry.retry_generation_core is now a one-call helper: POST /retry, return the parsed dict, raise on failure. Each surface POSTs and renders success vs the backend's error message in its own style, mirroring _cancel_flow: - TUI _retry_flow: toast on success, MessageScreen with the backend reason on error. - CLI: print + exit 0/1. - MCP: chat_json; drops the client-derived `code`/pending-guidance fields in favour of the backend's message (single source of truth). No client status round-trip before the POST. Tests updated (core, CLI, MCP, TUI). make unit-tests: 671 passed.
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
2 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
Adds user-initiated cancellation of a running generation across the whole stack. Cancelling stops all in-flight agents immediately, moves the session to a distinct terminal CANCELLED state that is never resumed on restart or retried, sends no error notification, and is exposed in the TUI on both the generation-detail and sessions-list screens.
Entrypoint
Backend:
cancel_generation_session(theDELETE /{generation_id}route) inbackend/app/api/v1/generation_sessions.py. TUI: the newx→ cancel binding and_cancel_flowonDashboardScreeninmcp_server/tui/app.py.Details
Cancellation uses two stop mechanisms: a process-local generation-id→
asyncio.Taskregistry (an injectedGenerationTaskRegistrycreated at app startup and threaded through the run/retry/cancel endpoints and boot recovery) for immediatetask.cancel()on the owning pod, plus a throttled cooperativeraise_if_cancelledcheck wired into the workflow, phase, parallel, and P10Y loops as a cross-pod-safe fallback that always runs on its first call for a generation. The endpoint setsCANCELLEDbefore cancelling the task so the injectedCancelledErrorandGenerationCancelledErrorunwind against an already-terminal state and never re-fail or notify; the state-machinecancel()mirrorsinterrupted_by_shutdown()and retains workspaces per Steel Commandment II, andagent_querynowaclose()s the SDK stream so the Claude Code CLI subprocess is torn down deterministically. The TUI recognizescancelledeverywhere it matters — a⊘ CANCELLEDpill,cancelledadded toTERMINAL_STATUSESso the dashboard stops polling, a "CANCELLED BY USER" label in the sessions list, and a neutral "Generation cancelled" desktop milestone instead of a failure. Boot recovery and the stuck detectors already excludeCANCELLED, and retry refuses it with a clear message.