Use control_request set_model for mid-session model switch instead of session restart (closes #852)#968
Closed
FidoCanCode wants to merge 0 commit into
Closed
Use control_request set_model for mid-session model switch instead of session restart (closes #852)#968FidoCanCode wants to merge 0 commit into
FidoCanCode wants to merge 0 commit into
Conversation
Collaborator
|
Just testing the webhook interrupts we added. Bark. |
86f4bdc to
2c73bd5
Compare
Owner
Author
|
Woof! 🐕 Interrupt received loud and clear — good to see the webhook preempt working. Back to chewing on this |
FidoCanCode
added a commit
that referenced
this pull request
Apr 25, 2026
…ry misclassified as task completion (closes #969) (#970) Fixes #969. ## Root cause: collision between #965 and #967 `Worker._execute_task`'s resume loop checked at worker.py:2387: ```python if not any( t["id"] == task["id"] and t.get("status") == TaskStatus.PENDING for t in current_task_list ): log.info("task externally completed — stopping retry ...") break ``` The intent was always *"task was marked COMPLETED by an external action"*, but the implementation was *"status is anything other than PENDING"*. Pre-#965 those were equivalent. #965 added `self._tasks.update(task_id, IN_PROGRESS)` at task start (worker.py:2362), so the running task is now IN_PROGRESS — the check breaks out immediately on every resume-loop iteration. The bug was latent until #967 added synchronous webhook preempt: a preempted turn returns empty without commits, `while head_before == head_after` runs, the check misclassifies, the worker breaks the resume loop, and downstream logic closes the in-flight PR. ## Fix Compare for COMPLETED directly. One-line change. ## Test Adds `test_does_not_treat_in_progress_task_as_externally_completed` that drives `execute_task` through one resume-loop retry with an IN_PROGRESS task and asserts the loop does not short-circuit. ## Trace from the wild ``` 14:42:02 task XXX → in_progress 14:42:18 webhook: preempting worker synchronously 14:42:18 ClaudeSession: cancelled — exiting turn early 14:42:18 session.prompt: turn complete (cancelled=True) 14:42:18 ClaudeClient.run_turn: preempted mid-flight — retry 1 14:42:18 session.prompt: turn complete (total=0.01s, result_len=0) ← empty 14:42:18 task done (session=) 14:42:18 task externally completed — stopping retry (id=XXX) ← bug fires 14:42:22 PR #968 closed by FidoCanCode ← self-close ``` Co-authored-by: Fido Can Code <190991155+FidoCanCode@users.noreply.github.com>
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.
Fixes #852.
Replaces the restart-based
switch_modelinClaudeSessionwith acontrol_requestset_modelmessage on the existing subprocess stdin, eliminating the kill/spawn cycle that caused init-handshake deadlocks. The subprocess stays up — same session, same pipes, same MCP connections — only the model identifier flips.Work queue
Completed (1)