fix(long-task): address code review findings

- Declare _scopes = {"core"} explicitly to prevent recursive nesting in subagent scope
- Document fragile coupling in _extract_file_changes: path extraction depends on
  write_file/edit_file detail format; add debug log for unexpected formats
- Align final-template threshold (max_steps - 2) with budget switch threshold
- Eliminate hasattr(self, "_state") in _reset_state by initializing in __init__
This commit is contained in:
chengyongru 2026-05-13 17:09:43 +08:00 committed by chengyongru
parent 5acae58a13
commit 5f5f3d5d97
2 changed files with 22 additions and 7 deletions

View File

@ -112,5 +112,5 @@ class ToolLoader:
if not is_plugin_source: if not is_plugin_source:
builtin_names.add(tool.name) builtin_names.add(tool.name)
except Exception: except Exception:
logger.error("Failed to register tool: %s", cls_label) logger.exception("Failed to register tool: %s", cls_label)
return registered return registered

View File

@ -79,6 +79,8 @@ class HandoffState:
class HandoffTool(Tool): class HandoffTool(Tool):
"""Signal that the step is done but the overall task continues.""" """Signal that the step is done but the overall task continues."""
_plugin_discoverable = False
def __init__(self, store: HandoffState) -> None: def __init__(self, store: HandoffState) -> None:
self._store = store self._store = store
@ -120,6 +122,8 @@ class HandoffTool(Tool):
class CompleteTool(Tool): class CompleteTool(Tool):
"""Signal that the entire long task is finished.""" """Signal that the entire long task is finished."""
_plugin_discoverable = False
def __init__(self, store: HandoffState) -> None: def __init__(self, store: HandoffState) -> None:
self._store = store self._store = store
@ -195,7 +199,7 @@ def _build_user_message(
goal=goal, goal=goal,
budget=budget, budget=budget,
) )
elif step >= max_steps - 3: elif step >= max_steps - 2:
prompt = render_template( prompt = render_template(
"agent/long_task/step_final.md", "agent/long_task/step_final.md",
step=step, step=step,
@ -238,6 +242,11 @@ def _extract_handoff_from_messages(messages: list[dict[str, Any]]) -> str:
return "" return ""
_FILE_EVENT_PREFIXES = ("Wrote ", "Edited ")
# NOTE: path extraction depends on write_file/edit_file detail format.
# If those tools change their output format, this mapping must be updated.
def _extract_file_changes( def _extract_file_changes(
tool_events: list[dict[str, Any]], tool_events: list[dict[str, Any]],
) -> tuple[list[str], list[str]]: ) -> tuple[list[str], list[str]]:
@ -251,13 +260,17 @@ def _extract_file_changes(
if status != "ok": if status != "ok":
continue continue
if name in ("write_file", "edit_file"): if name in ("write_file", "edit_file"):
# Try to extract file path from detail if detail.startswith(_FILE_EVENT_PREFIXES):
if detail.startswith("Wrote ") or detail.startswith("Edited "):
path = detail.split(" ", 1)[1].split(":")[0].strip() path = detail.split(" ", 1)[1].split(":")[0].strip()
if name == "write_file": if name == "write_file":
created.append(path) created.append(path)
else: else:
modified.append(path) modified.append(path)
else:
logger.debug(
"long_task: skipping file event with unexpected detail: {}",
detail[:80],
)
return created, modified return created, modified
@ -292,9 +305,13 @@ class LongTaskEvent:
class LongTaskTool(Tool): class LongTaskTool(Tool):
"""Execute a long-running task via a meta-ReAct loop of subagent steps.""" """Execute a long-running task via a meta-ReAct loop of subagent steps."""
# NOT available in subagent scope to prevent recursive long_task nesting.
_scopes: set[str] = {"core"}
def __init__(self, manager: SubagentManager) -> None: def __init__(self, manager: SubagentManager) -> None:
self._manager = manager self._manager = manager
self._hooks: dict[str, Any] = {} self._hooks: dict[str, Any] = {}
self._state: dict[str, Any] = {"signal_queue": []}
self._reset_state() self._reset_state()
def _reset_state(self) -> None: def _reset_state(self) -> None:
@ -303,9 +320,7 @@ class LongTaskTool(Tool):
Preserves any pending user corrections so inject_correction() can be Preserves any pending user corrections so inject_correction() can be
called before execute() starts. called before execute() starts.
""" """
existing_signals = ( existing_signals = self._state.get("signal_queue", [])
self._state.get("signal_queue", []) if hasattr(self, "_state") else []
)
self._state: dict[str, Any] = { self._state: dict[str, Any] = {
"current_step": 0, "current_step": 0,
"total_steps": 0, "total_steps": 0,