-
Notifications
You must be signed in to change notification settings - Fork 329
feat(agent): add agent interrupt support #1930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@cubic review |
@jordan-umusu I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 19 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="tracecat/agent/executor/loopback.py">
<violation number="1" location="tracecat/agent/executor/loopback.py:242">
P2: Using `asyncio.wait_for` around the full `read_message` can cancel a read mid‑payload, which can desynchronize the framed protocol and corrupt subsequent messages. Consider implementing timeout polling without cancelling an in‑flight read (e.g., keep a `read_message` task across polls or only apply the timeout while waiting for the header).</violation>
</file>
<file name="tracecat/agent/session/activities.py">
<violation number="1" location="tracecat/agent/session/activities.py:185">
P2: Avoid catching all exceptions here; it swallows unexpected failures and prevents the workflow/activity from failing fast. Either let exceptions propagate or only catch specific, expected errors.
(Based on your team's feedback about avoiding broad `except Exception` blocks.) [FEEDBACK_USED]</violation>
</file>
<file name="frontend/src/components/chat/chat-session-pane.tsx">
<violation number="1" location="frontend/src/components/chat/chat-session-pane.tsx:462">
P2: The updated disabled condition enables the button during non-streaming in-flight states (e.g., status="submitted"), so it becomes a submit button while a response is pending. This can trigger overlapping sendMessage calls and out-of-order responses. Keep the button disabled for non-streaming statuses while still allowing interrupts during streaming.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| try: | ||
| _msg_type, payload_bytes = await read_message( | ||
| reader, expected_type=MessageType.EVENT | ||
| _, payload_bytes = await asyncio.wait_for( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Using asyncio.wait_for around the full read_message can cancel a read mid‑payload, which can desynchronize the framed protocol and corrupt subsequent messages. Consider implementing timeout polling without cancelling an in‑flight read (e.g., keep a read_message task across polls or only apply the timeout while waiting for the header).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tracecat/agent/executor/loopback.py, line 242:
<comment>Using `asyncio.wait_for` around the full `read_message` can cancel a read mid‑payload, which can desynchronize the framed protocol and corrupt subsequent messages. Consider implementing timeout polling without cancelling an in‑flight read (e.g., keep a `read_message` task across polls or only apply the timeout while waiting for the header).</comment>
<file context>
@@ -210,16 +231,31 @@ async def _process_runtime_events(self, reader: asyncio.StreamReader) -> None:
try:
- _msg_type, payload_bytes = await read_message(
- reader, expected_type=MessageType.EVENT
+ _, payload_bytes = await asyncio.wait_for(
+ read_message(reader, expected_type=MessageType.EVENT),
+ timeout=INTERRUPT_POLL_INTERVAL_SEC,
</file context>
| session_id=input.session_id, | ||
| status=input.status, | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Avoid catching all exceptions here; it swallows unexpected failures and prevents the workflow/activity from failing fast. Either let exceptions propagate or only catch specific, expected errors.
(Based on your team's feedback about avoiding broad except Exception blocks.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tracecat/agent/session/activities.py, line 185:
<comment>Avoid catching all exceptions here; it swallows unexpected failures and prevents the workflow/activity from failing fast. Either let exceptions propagate or only catch specific, expected errors.
(Based on your team's feedback about avoiding broad `except Exception` blocks.) </comment>
<file context>
@@ -158,9 +158,43 @@ async def load_session_activity(input: LoadSessionInput) -> LoadSessionResult:
+ session_id=input.session_id,
+ status=input.status,
+ )
+ except Exception as e:
+ logger.error(
+ "Failed to update session status",
</file context>
| )} | ||
| <PromptInputSubmit | ||
| disabled={isReadonly || !input || !!status} | ||
| disabled={isReadonly || (!input && !status)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The updated disabled condition enables the button during non-streaming in-flight states (e.g., status="submitted"), so it becomes a submit button while a response is pending. This can trigger overlapping sendMessage calls and out-of-order responses. Keep the button disabled for non-streaming statuses while still allowing interrupts during streaming.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/chat/chat-session-pane.tsx, line 462:
<comment>The updated disabled condition enables the button during non-streaming in-flight states (e.g., status="submitted"), so it becomes a submit button while a response is pending. This can trigger overlapping sendMessage calls and out-of-order responses. Keep the button disabled for non-streaming statuses while still allowing interrupts during streaming.</comment>
<file context>
@@ -451,8 +459,9 @@ export function ChatSessionPane({
)}
<PromptInputSubmit
- disabled={isReadonly || !input || !!status}
+ disabled={isReadonly || (!input && !status)}
status={status}
+ onInterrupt={interrupt}
</file context>
| disabled={isReadonly || (!input && !status)} | |
| disabled={isReadonly || (status && status !== "streaming") || (!input && !status)} |
|
Found 14 test failures on Blacksmith runners: Failures
|
Summary by cubic
Adds end-to-end agent interrupt support so users can stop a running session from the chat. Sessions now track lifecycle status and streams close cleanly on interrupt.
New Features
Migration
Written for commit b195983. Summary will update on new commits.