-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix: prevent premature subagent termination, add attack planning framework, and fix TUI lag #702
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -730,6 +730,9 @@ def __init__(self, args: argparse.Namespace): | |
| ] | ||
| self._dot_animation_timer: Any | None = None | ||
|
|
||
| self._event_buffer: list[tuple[str, Any]] = [] | ||
| self._event_flush_timer: Any | None = None | ||
|
|
||
| self._setup_cleanup_handlers() | ||
|
|
||
| def _build_scan_config(self, args: argparse.Namespace) -> dict[str, Any]: | ||
|
|
@@ -1292,7 +1295,7 @@ def _get_animated_verb_text(self, agent_id: str, verb: str) -> Text: # noqa: AR | |
|
|
||
| def _start_dot_animation(self) -> None: | ||
| if self._dot_animation_timer is None: | ||
| self._dot_animation_timer = self.set_interval(0.06, self._animate_dots) | ||
| self._dot_animation_timer = self.set_interval(0.15, self._animate_dots) | ||
|
|
||
| def _stop_dot_animation(self) -> None: | ||
| if self._dot_animation_timer is not None: | ||
|
|
@@ -1335,9 +1338,13 @@ def _agent_vulnerability_count(self, agent_id: str) -> int: | |
| if vuln.get("agent_id") == agent_id | ||
| ) | ||
|
|
||
| MAX_RENDER_EVENTS = 200 | ||
|
|
||
| def _gather_agent_events(self, agent_id: str) -> list[dict[str, Any]]: | ||
| events = self.live_view.events_for_agent(agent_id) | ||
| events.sort(key=lambda e: (e["timestamp"], e["id"])) | ||
| if len(events) > self.MAX_RENDER_EVENTS: | ||
| events = events[-self.MAX_RENDER_EVENTS:] | ||
| return events | ||
|
|
||
| def watch_selected_agent_id(self, _agent_id: str | None) -> None: | ||
|
|
@@ -1407,10 +1414,25 @@ def scan_target() -> None: | |
| self._scan_thread.start() | ||
|
|
||
| def _capture_sdk_event(self, agent_id: str, event: Any) -> None: | ||
| self._event_buffer.append((agent_id, event)) | ||
| if self._event_flush_timer is None: | ||
| self._event_flush_timer = self.set_timer(0.15, self._flush_event_buffer) | ||
|
|
||
| def _flush_event_buffer(self) -> None: | ||
| self._event_flush_timer = None | ||
| if not self._event_buffer: | ||
| return | ||
| batch = self._event_buffer[:] | ||
| self._event_buffer.clear() | ||
| try: | ||
| self.call_from_thread(self._record_sdk_event, agent_id, event) | ||
| self.call_from_thread(self._record_sdk_events_batch, batch) | ||
| except RuntimeError: | ||
| self._record_sdk_event(agent_id, event) | ||
| for agent_id, event in batch: | ||
| self._record_sdk_event(agent_id, event) | ||
|
Comment on lines
+1421
to
+1431
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: strix/interface/tui/app.py
Line: 1421-1431
Comment:
**Flush timer always takes the RuntimeError fallback path**
`_flush_event_buffer` is invoked by a Textual timer, which means it runs on the Textual main thread. Calling `call_from_thread` from the main thread always raises `RuntimeError`, so `_record_sdk_events_batch` is dead code — every flush goes through the per-event fallback loop. Separately, there is a race window between `batch = self._event_buffer[:]` (snapshot) and `self._event_buffer.clear()`: the background SDK thread can append an event into the buffer after the snapshot is taken but before `clear()` executes. That event is removed from the buffer without being in `batch`, silently dropping it from the TUI display.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| def _record_sdk_events_batch(self, batch: list[tuple[str, Any]]) -> None: | ||
| for agent_id, event in batch: | ||
| self.live_view.ingest_sdk_event(agent_id, event) | ||
|
|
||
| def _record_sdk_event(self, agent_id: str, event: Any) -> None: | ||
| self.live_view.ingest_sdk_event(agent_id, event) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,10 +14,14 @@ | |
| from strix.interface.tui.history import load_session_history | ||
|
|
||
|
|
||
| MAX_EVENTS_PER_AGENT = 500 | ||
|
|
||
|
|
||
| class TuiLiveView: | ||
| def __init__(self) -> None: | ||
| self.agents: dict[str, dict[str, Any]] = {} | ||
| self.events: list[dict[str, Any]] = [] | ||
| self._events_by_agent: dict[str, list[dict[str, Any]]] = {} | ||
| self._next_event_id = 1 | ||
| self._open_assistant_event_by_agent: dict[str, dict[str, Any]] = {} | ||
| self._tool_event_by_call_id: dict[str, dict[str, Any]] = {} | ||
|
|
@@ -115,10 +119,10 @@ def ingest_sdk_event(self, agent_id: str, event: Any) -> None: | |
| self._record_tool_output(agent_id, item) | ||
|
|
||
| def events_for_agent(self, agent_id: str) -> list[dict[str, Any]]: | ||
| return [event for event in self.events if event.get("agent_id") == agent_id] | ||
| return list(self._events_by_agent.get(agent_id, [])) | ||
|
|
||
| def has_events_for_agent(self, agent_id: str) -> bool: | ||
| return any(event.get("agent_id") == agent_id for event in self.events) | ||
| return bool(self._events_by_agent.get(agent_id)) | ||
|
|
||
| def _ingest_raw_response_event(self, agent_id: str, data: Any) -> None: | ||
| data_type = getattr(data, "type", "") | ||
|
|
@@ -277,6 +281,16 @@ def _append_event( | |
| } | ||
| self._next_event_id += 1 | ||
| self.events.append(event) | ||
| agent_events = self._events_by_agent.setdefault(agent_id, []) | ||
| agent_events.append(event) | ||
| if len(agent_events) > MAX_EVENTS_PER_AGENT: | ||
| dropped = agent_events[:len(agent_events) - MAX_EVENTS_PER_AGENT] | ||
| for d in dropped: | ||
| try: | ||
| self.events.remove(d) | ||
| except ValueError: | ||
| pass | ||
| del agent_events[:len(agent_events) - MAX_EVENTS_PER_AGENT] | ||
|
Comment on lines
+286
to
+293
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After the migration to Prompt To Fix With AIThis is a comment left during a code review.
Path: strix/interface/tui/live_view.py
Line: 286-293
Comment:
**`self.events.remove(d)` is O(n) overhead on an otherwise-unused list**
After the migration to `_events_by_agent`, `self.events` is no longer queried anywhere — `events_for_agent` and `has_events_for_agent` both use the new index. The per-prune `list.remove()` call is an O(n) linear scan across the entire global event list just to keep `self.events` in sync with a structure nothing reads. For long scans with many active agents the list can grow to `MAX_EVENTS_PER_AGENT × num_agents` entries, making each pruning step increasingly expensive.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| return event | ||
|
|
||
| @staticmethod | ||
|
|
||
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.
set_timercalled from a background thread_capture_sdk_eventis passed as theevent_sinkcallback and is invoked from the background asyncio scan loop, not the Textual main thread.set_timeris not documented as thread-safe in Textual; the supported path for scheduling work from a background thread iscall_from_thread. Also, theif self._event_flush_timer is Noneguard and the subsequent assignment are not atomic, so two rapid callback invocations from the scan loop can both observeNoneand register duplicate flush timers.Prompt To Fix With AI