-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat(agent): structured task format and workflow improvements #384
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 |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| from typing import Any | ||
|
|
||
| import html | ||
|
|
||
| from strix.agents.base_agent import BaseAgent | ||
| from strix.llm.config import LLMConfig | ||
|
|
||
|
|
@@ -57,31 +59,45 @@ async def execute_scan(self, scan_config: dict[str, Any]) -> dict[str, Any]: # | |
| elif target_type == "ip_address": | ||
| ip_addresses.append(details["target_ip"]) | ||
|
|
||
| task_parts = [] | ||
| target_lines = [] | ||
|
|
||
| if repositories: | ||
| task_parts.append("\n\nRepositories:") | ||
| if repositories: | ||
| for repo in repositories: | ||
| if repo["workspace_path"]: | ||
| task_parts.append(f"- {repo['url']} (available at: {repo['workspace_path']})") | ||
| target_lines.append(f' <target type="repository">{html.escape(repo["url"])} (code at: {html.escape(repo["workspace_path"])})</target>') | ||
| else: | ||
| task_parts.append(f"- {repo['url']}") | ||
| target_lines.append(f' <target type="repository">{html.escape(repo["url"])}</target>') | ||
|
|
||
| if local_code: | ||
| task_parts.append("\n\nLocal Codebases:") | ||
| task_parts.extend( | ||
| f"- {code['path']} (available at: {code['workspace_path']})" for code in local_code | ||
| ) | ||
| for code in local_code: | ||
| target_lines.append(f' <target type="local_code">{html.escape(code["path"])} (code at: {html.escape(code["workspace_path"])})</target>') | ||
|
|
||
| if urls: | ||
| task_parts.append("\n\nURLs:") | ||
| task_parts.extend(f"- {url}" for url in urls) | ||
| for url in urls: | ||
| target_lines.append(f' <target type="url">{html.escape(url)}</target>') | ||
|
|
||
| if ip_addresses: | ||
| task_parts.append("\n\nIP Addresses:") | ||
| task_parts.extend(f"- {ip}" for ip in ip_addresses) | ||
|
|
||
| task_description = " ".join(task_parts) | ||
| for ip in ip_addresses: | ||
| target_lines.append(f' <target type="ip">{html.escape(ip)}</target>') | ||
|
|
||
| targets_block = "\n".join(target_lines) | ||
|
|
||
| has_code = bool(repositories or local_code) | ||
| has_urls = bool(urls or ip_addresses) | ||
| if has_code and has_urls: | ||
| mode = "COMBINED MODE (code + deployed target)" | ||
| elif has_code: | ||
| mode = "WHITE-BOX (source code provided)" | ||
| else: | ||
| mode = "BLACK-BOX (URL/domain targets)" | ||
|
|
||
| task_description = ( | ||
| f"<scan_task>\n" | ||
| f"<targets>\n{targets_block}\n</targets>\n" | ||
| f"<mode>{mode}</mode>\n" | ||
| f"<action>Begin security assessment NOW. Your first tool call must be create_agent to spawn context-gathering subagents for the targets listed above. Do NOT call wait_for_message — the targets are already specified.</action>\n" | ||
| f"</scan_task>" | ||
| ) | ||
|
|
||
| if user_instructions: | ||
| task_description += f"\n\nSpecial instructions: {user_instructions}" | ||
|
Comment on lines
+94
to
103
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import asyncio | ||
| import contextlib | ||
| import html | ||
| import logging | ||
| from typing import TYPE_CHECKING, Any, Optional | ||
|
|
||
|
|
@@ -410,6 +411,13 @@ async def _process_iteration(self, tracer: Optional["Tracer"]) -> bool | None: | |
| if actions: | ||
| return await self._execute_actions(actions, tracer) | ||
|
|
||
| corrective_message = ( | ||
| "You responded with plain text instead of a tool call. " | ||
| "While the agent loop is running, EVERY response MUST be a tool call. " | ||
| "Do NOT send plain text messages. Act via your available tools. " | ||
| "Review your task and take action now." | ||
| ) | ||
| self.state.add_message("user", corrective_message) | ||
|
Comment on lines
+414
to
+420
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.
The corrective message references Consider making this message generic (e.g., "Use an available tool to proceed") or overriding Prompt To Fix With AIThis is a comment left during a code review.
Path: strix/agents/base_agent.py
Line: 401-411
Comment:
**Corrective message hardcodes StrixAgent-specific tool names in BaseAgent**
The corrective message references `create_agent`, `terminal_execute`, and `wait_for_message` — tools that are specific to `StrixAgent`. Since this logic lives in `BaseAgent`, other subclasses that respond with plain text will receive guidance for tools they don't have, which could confuse those agents or cause them to attempt non-existent tool calls.
Consider making this message generic (e.g., "Use an available tool to proceed") or overriding `_process_iteration` in `StrixAgent` to provide the agent-specific corrective message.
How can I resolve this? If you propose a fix, please make it concise. |
||
| return None | ||
|
|
||
| async def _execute_actions(self, actions: list[Any], tracer: Optional["Tracer"]) -> bool: | ||
|
|
@@ -484,33 +492,18 @@ def _check_agent_messages(self, state: AgentState) -> None: # noqa: PLR0912 | |
| sender_name = "User" | ||
| state.add_message("user", message.get("content", "")) | ||
| else: | ||
| sender_name = sender_id or "Unknown" | ||
| if sender_id and sender_id in _agent_graph.get("nodes", {}): | ||
| sender_name = _agent_graph["nodes"][sender_id]["name"] | ||
|
|
||
| message_content = f"""<inter_agent_message> | ||
| <delivery_notice> | ||
| <important>You have received a message from another agent. You should acknowledge | ||
| this message and respond appropriately based on its content. However, DO NOT echo | ||
| back or repeat the entire message structure in your response. Simply process the | ||
| content and respond naturally as/if needed.</important> | ||
| </delivery_notice> | ||
| <sender> | ||
| <agent_name>{sender_name}</agent_name> | ||
| <agent_id>{sender_id}</agent_id> | ||
| </sender> | ||
| <message_metadata> | ||
| <type>{message.get("message_type", "information")}</type> | ||
| <priority>{message.get("priority", "normal")}</priority> | ||
| <timestamp>{message.get("timestamp", "")}</timestamp> | ||
| </message_metadata> | ||
| <content> | ||
| {message.get("content", "")} | ||
| </content> | ||
| <delivery_info> | ||
| <note>This message was delivered during your task execution. | ||
| Please acknowledge and respond if needed.</note> | ||
| </delivery_info> | ||
| </inter_agent_message>""" | ||
| content = message.get("content", "") | ||
| message_content = f"""<agent_message | ||
| from="{html.escape(sender_name)}" | ||
| id="{html.escape(str(sender_id))}" | ||
| type="{html.escape(message.get("message_type", "information"))}" | ||
| priority="{html.escape(message.get("priority", "normal"))}"> | ||
| <![CDATA[{content}]]> | ||
| </agent_message>""" | ||
|
Comment on lines
+500
to
+506
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.
The verbose Prompt To Fix With AIThis is a comment left during a code review.
Path: strix/agents/base_agent.py
Line: 490-496
Comment:
**Message content not escaped in XML attribute/element context**
`sender_name`, `sender_id`, and `message.get("content", "")` are embedded into the `<agent_message>` XML without any escaping. If `sender_name` or `sender_id` contains a double-quote character, the XML attributes will be broken. More importantly, if the message `content` itself contains the literal string `</agent_message>`, the new compact format would cause the regex in `clean_content()` (which matches `<agent_message\b[^>]*>.*?</agent_message>`) to terminate prematurely, leaving the remainder of the content unexpectedly visible in traces.
The verbose `<inter_agent_message>` format wrapped `content` in a `<content>` child element which made it somewhat more resilient to this (though not fully). The new inline format is more susceptible. Consider CDATA-wrapping or escaping the content before embedding.
How can I resolve this? If you propose a fix, please make it concise. |
||
| state.add_message("user", message_content.strip()) | ||
|
|
||
| message["read"] = True | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -44,9 +44,7 @@ def increment_iteration(self) -> None: | |||||
| self.iteration += 1 | ||||||
| self.last_updated = datetime.now(UTC).isoformat() | ||||||
|
|
||||||
| def add_message( | ||||||
| self, role: str, content: Any, thinking_blocks: list[dict[str, Any]] | None = None | ||||||
| ) -> None: | ||||||
| def add_message(self, role: str, content: Any, thinking_blocks: list | None = None) -> None: | ||||||
|
||||||
| def add_message(self, role: str, content: Any, thinking_blocks: list | None = None) -> None: | |
| def add_message(self, role: str, content: Any, thinking_blocks: list[dict[str, Any]] | None = None) -> None: |
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.
These f-strings create very long physical lines (notably the
<action>…</action>line), which is likely to trip the repo’s Ruff/Black 100-char line-length enforcement (E501). Please wrap/split the string across multiple lines (e.g., implicit concatenation) so formatting/lint passes consistently.