You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Keep intentional Claude SDK interrupt teardown from marking the session as crashed when the terminated CLI reports ProcessError.
Add a regression test proving a stopped turn can be followed by a successful turn in the same Omnigent session.
ELI5: Stop removes the active client on purpose. If that client then reports that it was terminated, Omnigent now recognizes the termination as part of Stop instead of treating the conversation as permanently broken.
Stop -> remove client -> CLI exits with SIGTERM -> do not set crash marker
-> next turn creates a client -> succeeds
Live E2E with a local Omnigent server and host in the Browser: started a sleep 30 command, clicked Interrupt, observed the Claude runner report ProcessError with exit code 143, then sent a follow-up in the same session and received SECOND_OK.
Demo
N/A
Type of change
Bug fix
Feature
UI / frontend change
Refactor / chore
Docs
Test / CI
Breaking change
Test coverage
Unit tests added / updated
Integration tests added / updated
E2E tests added / updated
Manual verification completed
Existing tests cover this change
Not applicable
Coverage notes
The regression test deterministically reproduces interrupt teardown followed by a process error and asserts that a second turn recovers. Manual verification exercised the same sequence through the live Browser UI against a local Omnigent server and host.
Changelog
Stopping a Claude SDK turn no longer prevents follow-up messages in the same session.
This PR fixes a backend flow that was previously broken/stuck — a Stop poisoned the session so no follow-up message could succeed — which is exactly the "backend bug that was stuck and is now fixed" case that warrants a before/after demonstration. The Demo section is N/A and no image/video was found. The Test Plan already describes the exact reproduction through the live Browser (start sleep 30 → Interrupt → observe exit 143 → follow-up returns SECOND_OK); please attach a short screen recording of that sequence so a reviewer can see recovery without checking out the branch. (The deterministic regression test partially mitigates this, so treat it as a request, not a hard blocker.)
Blocking issues
None. The change is correct and well-scoped:
On interrupt, interrupt_session → close_session → _close_live_client pops session_key from self._clientsbefore the in-flight turn's receive_response() raises the SIGTERM-induced ProcessError. Guarding the crash marker on session_key in self._clients therefore correctly distinguishes intentional teardown (client already removed → no crash mark) from a genuine mid-turn crash (client still present → crash mark set).
Genuine crashes are unaffected: the client is added in _get_or_create_client and is only removed via deliberate close/interrupt/cancel paths, so a real fault still leaves session_key in self._clients and still poisons the session as before.
The subsequent _close_live_client(session_key) call is a harmless no-op when the client was already popped.
Security vulnerabilities
None. No auth, deserialization, injection, or boundary changes; no dependency/lockfile changes.
Non-blocking notes
The interrupted turn still yields an ExecutorError (with the ProcessError stderr/diagnostics) to the caller after teardown — only the persistent crash marker is suppressed. That matches prior behavior, but if the runner surfaces that ExecutorError to the user it will read like a failure even though the Stop was intentional. Worth confirming the runner treats a post-interrupt ExecutorError as expected teardown rather than a hard error.
The test relies on ordering between interrupt_session popping the client and the turn's error propagating. It's deterministic here via the process_terminated event, but the guard's correctness depends on close_session always completing the pop before the boundary runs on the real path — which holds given interrupt_session awaits close_session before returning.
Summary
A minimal, correct fix that stops intentional interrupt teardown from permanently poisoning a Claude SDK session, backed by a solid deterministic regression test that reproduces "stop → ProcessError → recover in a fresh client." Logic and semantics check out and there are no security concerns. The only ask is a short screen recording of the interrupt-then-recover flow in the Browser to satisfy the visual-demonstration expectation for a previously-broken backend behavior.
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
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.
Related issue
N/A
Summary
ProcessError.ELI5: Stop removes the active client on purpose. If that client then reports that it was terminated, Omnigent now recognizes the termination as part of Stop instead of treating the conversation as permanently broken.
Test Plan
python -m pytest tests/inner/test_claude_sdk_executor.py(121 passed)sleep 30command, clicked Interrupt, observed the Claude runner reportProcessErrorwith exit code 143, then sent a follow-up in the same session and receivedSECOND_OK.Demo
N/A
Type of change
Test coverage
Coverage notes
The regression test deterministically reproduces interrupt teardown followed by a process error and asserts that a second turn recovers. Manual verification exercised the same sequence through the live Browser UI against a local Omnigent server and host.
Changelog
Stopping a Claude SDK turn no longer prevents follow-up messages in the same session.