From 202320cab5603a26f2b17e4c8946b63b9c9c7eeb Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Tue, 11 Aug 2026 18:05:40 +0200 Subject: [PATCH 1/7] Support listing and deleting recorded agent actions --- .../bug-fix/hackbot_agents/bug_fix/config.py | 4 + .../hackbot_agents/bug_fix/prompts/system.md | 2 +- .../hackbot_agents/frontend_triage/config.py | 7 +- .../frontend_triage/prompts/system.md | 4 +- .../hackbot_runtime/actions/__init__.py | 9 ++- .../hackbot_runtime/actions/bugzilla.py | 2 +- .../hackbot_runtime/actions/claude_sdk.py | 9 ++- .../hackbot_runtime/actions/phabricator.py | 2 +- .../actions/recorded_actions.py | 48 ++++++++++++ .../hackbot_runtime/actions/recorder.py | 61 +++++++++++++-- .../hackbot_runtime/actions/slack.py | 2 +- .../hackbot_runtime/actions/testrail.py | 2 +- libs/hackbot-runtime/tests/test_claude_sdk.py | 69 ++++++++++++++++- libs/hackbot-runtime/tests/test_recorder.py | 77 +++++++++++++++++++ libs/hackbot-runtime/tests/test_runtime.py | 29 +++++++ .../tests/test_slack_actions.py | 2 +- .../tests/test_testrail_action.py | 2 +- 17 files changed, 308 insertions(+), 23 deletions(-) create mode 100644 libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py diff --git a/agents/bug-fix/hackbot_agents/bug_fix/config.py b/agents/bug-fix/hackbot_agents/bug_fix/config.py index 293c283682..3441d986ac 100644 --- a/agents/bug-fix/hackbot_agents/bug_fix/config.py +++ b/agents/bug-fix/hackbot_agents/bug_fix/config.py @@ -19,6 +19,8 @@ # Action types that the agent may record during triage/fix runs. TRIAGE_AND_FIX_ACTIONS = [ + "recorded_actions.list_actions", + "recorded_actions.remove_action", "bugzilla.update_bug", "bugzilla.add_comment", "bugzilla.add_attachment", @@ -28,6 +30,8 @@ # Action types that the agent may record during follow-up runs on a revision. PHABRICATOR_FOLLOW_UP_ACTIONS = [ + "recorded_actions.list_actions", + "recorded_actions.remove_action", "bugzilla.update_bug", "bugzilla.add_attachment", "bugzilla.create_bug", diff --git a/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md b/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md index 49201b2d9c..4939906576 100644 --- a/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md +++ b/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md @@ -67,7 +67,7 @@ When you spawn an investigator via the Task tool, write a complete, self-contain # Recording actions -The `actions` MCP tools (`bugzilla_update_bug`, `bugzilla_add_comment`, and the Phabricator actions this run enables) do **not** mutate Bugzilla or Phabricator directly. Use `phabricator_add_comment`, when available, to reply on a Differential revision (for example, to answer a question when no code change is needed); use `phabricator_submit_patch` to deliver a code fix as a new revision, or `phabricator_update_patch` to deliver it as a new diff on the existing revision. Only the actions listed in your toolset are available: if one is missing, it does not apply to this run. They record an intended action into the run's `summary.json` for a human reviewer (or a downstream apply step) to enact. Treat each recorded action as a final, irrevocable proposal — once recorded it appears in the run output verbatim. +The `actions` MCP tools (`bugzilla_update_bug`, `bugzilla_add_comment`, and the Phabricator actions this run enables) do **not** mutate Bugzilla or Phabricator directly. Use `phabricator_add_comment`, when available, to reply on a Differential revision (for example, to answer a question when no code change is needed); use `phabricator_submit_patch` to deliver a code fix as a new revision, or `phabricator_update_patch` to deliver it as a new diff on the existing revision. Only the actions listed in your toolset are available: if one is missing, it does not apply to this run. They record an intended action into the run's `summary.json` for a human reviewer (or a downstream apply step) to enact. Recorded actions remain proposals until the run finishes; use `recorded_actions_list_actions` to review them and `recorded_actions_remove_action` to retract an inaccurate proposal. When you record `phabricator_submit_patch` to create a new revision, do not also record `bugzilla_add_comment` in the same run. The patch description already appears as a Bugzilla comment, so another comment would be redundant. diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py index 9364c0a901..6b99413633 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py @@ -29,11 +29,10 @@ ] -# Recordable action types the agent may take, by dotted id. This agent triages -# and plans only: it records a comment with its findings/plan and, at high -# confidence, may propose field updates (e.g. keyword/severity). It never -# creates bugs or attaches files. +# Actions-server tools available to the agent, by dotted id. ENABLED_ACTION_TYPES = [ + "recorded_actions.list_actions", + "recorded_actions.remove_action", "bugzilla.add_comment", "bugzilla.update_bug", ] diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md index 11201a7d89..6c541c289d 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -80,7 +80,9 @@ When you spawn an investigator via the Task tool, write a complete, self-contain # Recording actions -The `actions` MCP tools (`bugzilla_add_comment`, `bugzilla_update_bug`) do **not** mutate Bugzilla directly. They record an intended action into the run's `summary.json` for a human reviewer (or a downstream apply step) to enact. Treat each recorded action as a final, irrevocable proposal. +The `actions` MCP tools (`bugzilla_add_comment`, `bugzilla_update_bug`) do **not** mutate Bugzilla directly. They record an intended action for the run's `summary.json`, where a human reviewer or downstream apply step can enact it. + +Recorded actions remain proposals until the run finishes. Use `recorded_actions_list_actions` when you need to recover or verify their complete details. If you discover that a proposal is inaccurate, retract exactly that action with `recorded_actions_remove_action`, then record a corrected action if needed. Before calling any action tool, state in your response: diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/__init__.py b/libs/hackbot-runtime/hackbot_runtime/actions/__init__.py index 409476d90f..b9f0979c0d 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/__init__.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/__init__.py @@ -7,7 +7,13 @@ claude-sdk adapter is ``hackbot_runtime.actions.claude_sdk.actions_server_for``. """ -from hackbot_runtime.actions import bugzilla, phabricator, slack, testrail +from hackbot_runtime.actions import ( + bugzilla, + phabricator, + recorded_actions, + slack, + testrail, +) from hackbot_runtime.actions.recorder import ActionHook, ActionsRecorder ACTIONS_SERVER_NAME = "actions" @@ -18,6 +24,7 @@ "ActionsRecorder", "bugzilla", "phabricator", + "recorded_actions", "slack", "testrail", ] diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py b/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py index 47379331e7..cb43248878 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py @@ -32,7 +32,7 @@ def _confirm(recorder: ActionsRecorder, action_type: str) -> str: - return f"Recorded {action_type} (#{len(recorder.actions) - 1})." + return f"Recorded {action_type} as {recorder.last_action_id}." @tool diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/claude_sdk.py b/libs/hackbot-runtime/hackbot_runtime/actions/claude_sdk.py index 1166042e4f..8a21848bb9 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/claude_sdk.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/claude_sdk.py @@ -15,6 +15,7 @@ from hackbot_runtime.actions import ACTIONS_SERVER_NAME from hackbot_runtime.actions import bugzilla as _bugzilla from hackbot_runtime.actions import phabricator as _phabricator +from hackbot_runtime.actions import recorded_actions as _recorded_actions from hackbot_runtime.actions import slack as _slack from hackbot_runtime.actions import testrail as _testrail from hackbot_runtime.actions.recorder import ActionsRecorder @@ -35,7 +36,13 @@ def actions_server_for( """ if recorder is None: recorder = ActionsRecorder(artifacts_dir=fallback_artifacts_dir) - tools = _bugzilla.TOOLS + _phabricator.TOOLS + _testrail.TOOLS + _slack.TOOLS + tools = ( + _recorded_actions.TOOLS + + _bugzilla.TOOLS + + _phabricator.TOOLS + + _testrail.TOOLS + + _slack.TOOLS + ) if types is not None: wanted = set(types) tools = [t for t in tools if t.dotted in wanted] diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py b/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py index 687e3fbd0f..312fd72e23 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py @@ -32,7 +32,7 @@ def _confirm(recorder: ActionsRecorder, action_type: str) -> str: - return f"Recorded {action_type} (#{len(recorder.actions) - 1})." + return f"Recorded {action_type} as {recorder.last_action_id}." @tool diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py new file mode 100644 index 0000000000..e3ae280734 --- /dev/null +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py @@ -0,0 +1,48 @@ +"""Agent-facing tools for inspecting and retracting recorded actions.""" + +from __future__ import annotations + +from typing import Annotated + +from agent_tools.registry import tool, tools_in +from pydantic import Field + +from hackbot_runtime.actions.recorder import ActionsRecorder + + +@tool +async def list_actions(recorder: ActionsRecorder) -> list[dict]: + """List every action currently proposed by this agent run. + + Returns each action's stable in-run ID and its complete recorded payload, + including parameters, reasoning, references, and attachment metadata. Use + this when earlier action details are no longer present in your context or + before deciding whether a proposal needs to be retracted. + """ + return recorder.list_actions() + + +@tool +async def remove_action( + recorder: ActionsRecorder, + action_id: Annotated[ + str, + Field( + description=( + "Exact stable ID returned when the action was recorded or by " + "list_actions (for example, action-2)." + ) + ), + ], +) -> dict: + """Retract one proposed action from this agent run. + + The removed action will not appear in the final run summary and cannot be + applied. This operation accepts exactly one action ID and has no cascade or + force mode. + """ + removed = recorder.remove_action(action_id) + return {"removed": removed, "remaining_count": recorder.action_count} + + +TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py index dffca6e641..7108fc154d 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py @@ -1,6 +1,9 @@ +import copy from collections.abc import Callable, Mapping, Sequence from pathlib import Path +from agent_tools.registry import ToolError + from hackbot_runtime.artifacts import publish_file from hackbot_runtime.uploader import SignedPolicyUploader @@ -37,7 +40,12 @@ def __init__( artifacts_dir: Path | None = None, hooks: Mapping[str, Sequence[ActionHook]] = {}, ) -> None: - self._actions: list[dict] = [] + # IDs are deliberately separate from the serialized action payload. They + # are handles for managing proposals while the agent is running, not a + # new field in the summary/API contract. + self._actions: dict[str, dict] = {} + self._next_action_sequence = 0 + self._last_action_id: str | None = None self._uploader = uploader self._artifacts_dir = artifacts_dir self._hooks = { @@ -68,11 +76,12 @@ def record( ``phabricator.create_revision``). ``params`` is action-specific data the apply step will need. ``attachments`` maps a logical name to a local file path; each file is preserved under the stable key - ``attachments//``: uploaded via the runtime + ``attachments//``: uploaded via the runtime uploader when one is configured, otherwise copied into the local artifacts directory (so it is retrievable from compose/direct runs). - The recorded action references it by that key; the original local - path is not persisted (it disappears with the container). + The sequence is never reused, even after action removal. The recorded + action references it by that key; the original local path is not + persisted (it disappears with the container). ``ref`` optionally labels this action so a *later* action in the same run can reference its apply-time result (e.g. a Bugzilla comment's @@ -88,7 +97,9 @@ def record( recording leaves nothing behind: the action the hooks see carries no ``attachments`` key yet. """ - idx = len(self._actions) + sequence = self._next_action_sequence + self._next_action_sequence += 1 + action_id = f"action-{sequence}" action: dict = { "type": action_type, "params": params, @@ -106,15 +117,49 @@ def record( key = publish_file( self._uploader, self._artifacts_dir, - f"attachments/{idx}/{name}", + f"attachments/{sequence}/{name}", path, ) recorded_attachments.append({"name": name, "uploaded_key": key}) action["attachments"] = recorded_attachments - self._actions.append(action) + self._actions[action_id] = action + self._last_action_id = action_id return action + def list_actions(self) -> list[dict]: + """Return complete copies of the current actions with stable in-run IDs.""" + return [ + {**copy.deepcopy(action), "action_id": action_id} + for action_id, action in self._actions.items() + ] + + def remove_action(self, action_id: str) -> dict: + """Remove one action. + + The returned payload includes the stable ID and is detached from recorder + state. Removing an action only changes the proposals that will be written + to ``summary.json``; an attachment already uploaded for it may remain as + an unreferenced artifact until normal storage cleanup. + """ + action = self._actions.get(action_id) + if action is None: + raise ToolError(f"No recorded action with ID {action_id!r}.") + + removed = {**copy.deepcopy(action), "action_id": action_id} + del self._actions[action_id] + return removed + + @property + def last_action_id(self) -> str: + if self._last_action_id is None: + raise RuntimeError("No action has been recorded.") + return self._last_action_id + + @property + def action_count(self) -> int: + return len(self._actions) + @property def actions(self) -> list[dict]: - return list(self._actions) + return list(self._actions.values()) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/slack.py b/libs/hackbot-runtime/hackbot_runtime/actions/slack.py index 591d6ae851..6abc152052 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/slack.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/slack.py @@ -65,7 +65,7 @@ async def post_message( Recorded into the run summary for human review -- does not post to Slack. """ recorder.record(ACTION_TYPE, _params(channel, text), reasoning=reasoning) - return f"Recorded {ACTION_TYPE} (#{len(recorder.actions) - 1})." + return f"Recorded {ACTION_TYPE} as {recorder.last_action_id}." def record_message( diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py index d009dd2974..d63014e247 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py @@ -88,7 +88,7 @@ def feature_must_not_be_blank(cls, value: str) -> str: def _confirm(recorder: ActionsRecorder, action_type: str) -> str: - return f"Recorded {action_type} (#{len(recorder.actions) - 1})." + return f"Recorded {action_type} as {recorder.last_action_id}." def _validated_params(feature: str, generated_test_cases: list[Any]) -> dict[str, Any]: diff --git a/libs/hackbot-runtime/tests/test_claude_sdk.py b/libs/hackbot-runtime/tests/test_claude_sdk.py index 2c59444588..9dc836aa30 100644 --- a/libs/hackbot-runtime/tests/test_claude_sdk.py +++ b/libs/hackbot-runtime/tests/test_claude_sdk.py @@ -1,11 +1,18 @@ """Tests for the actions MCP server (built via agent-tools' adapter).""" +import json + import mcp.server.lowlevel.server as low from hackbot_runtime.actions import ActionsRecorder -from hackbot_runtime.actions.claude_sdk import actions_server_for +from hackbot_runtime.actions.claude_sdk import ( + actions_server_for, + actions_to_tool_names, +) from mcp.types import CallToolRequest, CallToolRequestParams, ListToolsRequest _ALL = [ + "recorded_actions.list_actions", + "recorded_actions.remove_action", "bugzilla.update_bug", "bugzilla.add_comment", "bugzilla.add_attachment", @@ -50,6 +57,8 @@ async def test_lists_expected_tools_without_recorder(): srv = _server(ActionsRecorder()) tools = await _list(srv) assert {t.name for t in tools} == { + "recorded_actions_list_actions", + "recorded_actions_remove_action", "bugzilla_update_bug", "bugzilla_add_comment", "bugzilla_add_attachment", @@ -115,3 +124,61 @@ async def test_actions_server_exposes_selected_testrail_tool(): ) tools = await _list(config["instance"]) assert {t.name for t in tools} == {"testrail_submit_test_plan"} + + +async def test_recorded_actions_tools_list_and_remove_complete_action(): + recorder = ActionsRecorder() + recorder.record( + "bugzilla.update_bug", + {"bug_id": 7, "changes": {"severity": "S2"}}, + reasoning="rule X", + ) + srv = _server(recorder) + + listed_result = await _call(srv, "recorded_actions_list_actions", {}) + listed = json.loads(listed_result.content[0].text) + assert listed == [ + { + "type": "bugzilla.update_bug", + "params": {"bug_id": 7, "changes": {"severity": "S2"}}, + "reasoning": "rule X", + "action_id": "action-0", + } + ] + + removed_result = await _call( + srv, "recorded_actions_remove_action", {"action_id": "action-0"} + ) + removed = json.loads(removed_result.content[0].text) + assert removed["removed"] == listed[0] + assert removed["remaining_count"] == 0 + assert recorder.actions == [] + + +async def test_recorded_actions_remove_unknown_id_surfaces_is_error(): + srv = _server(ActionsRecorder()) + + result = await _call( + srv, "recorded_actions_remove_action", {"action_id": "action-404"} + ) + + assert result.isError is True + assert "No recorded action" in result.content[0].text + + +def test_actions_to_tool_names_maps_exactly_the_selected_tools(): + assert actions_to_tool_names( + [ + "recorded_actions.list_actions", + "recorded_actions.remove_action", + "bugzilla.update_bug", + ] + ) == [ + "mcp__actions__recorded_actions_list_actions", + "mcp__actions__recorded_actions_remove_action", + "mcp__actions__bugzilla_update_bug", + ] + + assert actions_to_tool_names(["bugzilla.update_bug"]) == [ + "mcp__actions__bugzilla_update_bug" + ] diff --git a/libs/hackbot-runtime/tests/test_recorder.py b/libs/hackbot-runtime/tests/test_recorder.py index 933662673b..3edd2a10d7 100644 --- a/libs/hackbot-runtime/tests/test_recorder.py +++ b/libs/hackbot-runtime/tests/test_recorder.py @@ -3,6 +3,7 @@ from pathlib import Path import pytest +from agent_tools.registry import ToolError from hackbot_runtime.actions import ActionsRecorder @@ -217,3 +218,79 @@ def test_constructor_hooks_are_copied(): rec.record("bugzilla.update_bug", {"bug_id": 1}) assert len(rec.actions) == 1 + + +def test_list_actions_returns_stable_ids_and_complete_detached_payloads(): + rec = ActionsRecorder() + rec.record( + "phabricator.submit_patch", + {"bug_id": 1, "title": "Fix"}, + reasoning="verified fix", + ref="patch", + ) + rec.record( + "bugzilla.add_comment", + {"bug_id": 1, "text": "See {{actions.patch.url}}"}, + reasoning="announce the patch", + ) + + listed = rec.list_actions() + + assert [action["action_id"] for action in listed] == ["action-0", "action-1"] + assert listed[0] == { + "action_id": "action-0", + "type": "phabricator.submit_patch", + "params": {"bug_id": 1, "title": "Fix"}, + "reasoning": "verified fix", + "ref": "patch", + } + assert "action_id" not in rec.actions[0] + + listed[0]["params"]["title"] = "mutated copy" + assert rec.actions[0]["params"]["title"] == "Fix" + + +def test_remove_action_deletes_only_the_requested_action(): + rec = ActionsRecorder() + rec.record("bugzilla.update_bug", {"bug_id": 1}, reasoning="first") + rec.record("bugzilla.add_comment", {"bug_id": 1}, reasoning="second") + + removed = rec.remove_action("action-0") + + assert removed["action_id"] == "action-0" + assert removed["reasoning"] == "first" + assert rec.action_count == 1 + assert rec.list_actions()[0]["action_id"] == "action-1" + assert [action["type"] for action in rec.actions] == ["bugzilla.add_comment"] + + +def test_remove_action_rejects_unknown_or_already_removed_id(): + rec = ActionsRecorder() + rec.record("bugzilla.update_bug", {"bug_id": 1}) + rec.remove_action("action-0") + + with pytest.raises(ToolError, match="No recorded action"): + rec.remove_action("action-0") + + +def test_removed_action_id_and_attachment_key_are_not_reused(tmp_path): + first = tmp_path / "first.txt" + second = tmp_path / "second.txt" + first.write_text("first") + second.write_text("second") + rec = ActionsRecorder(artifacts_dir=tmp_path / "artifacts") + + rec.record("bugzilla.add_attachment", {"bug_id": 1}, attachments={"file": first}) + rec.remove_action("action-0") + rec.record("bugzilla.add_attachment", {"bug_id": 1}, attachments={"file": second}) + + assert rec.list_actions()[0]["action_id"] == "action-1" + assert rec.actions[0]["attachments"] == [ + {"name": "file", "uploaded_key": "attachments/1/file"} + ] + assert (tmp_path / "artifacts" / "attachments" / "0" / "file").read_text() == ( + "first" + ) + assert (tmp_path / "artifacts" / "attachments" / "1" / "file").read_text() == ( + "second" + ) diff --git a/libs/hackbot-runtime/tests/test_runtime.py b/libs/hackbot-runtime/tests/test_runtime.py index 3469283b42..f7e1483096 100644 --- a/libs/hackbot-runtime/tests/test_runtime.py +++ b/libs/hackbot-runtime/tests/test_runtime.py @@ -56,6 +56,35 @@ def test_summary_written_for_exception(tmp_path): assert "boom" in summary["error"] +def test_removed_action_is_absent_from_summary(tmp_path): + ctx = _ctx(tmp_path) + ctx.actions.record( + "bugzilla.update_bug", + {"bug_id": 1, "changes": {"severity": "S2"}}, + reasoning="inaccurate", + ) + ctx.actions.record( + "bugzilla.add_comment", + {"bug_id": 1, "text": "Corrected assessment"}, + reasoning="corrected", + ) + ctx.actions.remove_action("action-0") + + code = _finish(ctx, HackbotAgentResult(num_turns=1)) + + assert code == 0 + summary = json.loads( + (tmp_path / "artifacts" / "local-test" / "summary.json").read_text() + ) + assert summary["actions"] == [ + { + "type": "bugzilla.add_comment", + "params": {"bug_id": 1, "text": "Corrected assessment"}, + "reasoning": "corrected", + } + ] + + def test_non_result_return_is_contract_error(tmp_path): ctx = _ctx(tmp_path) # A bare dict (or None) is no longer accepted — only a HackbotAgentResult. diff --git a/libs/hackbot-runtime/tests/test_slack_actions.py b/libs/hackbot-runtime/tests/test_slack_actions.py index d80cd64762..6dd41cefd7 100644 --- a/libs/hackbot-runtime/tests/test_slack_actions.py +++ b/libs/hackbot-runtime/tests/test_slack_actions.py @@ -14,7 +14,7 @@ async def test_post_message_records_action(): text=" a test regressed ", reasoning="sheriffs decide on the backout", ) - assert "slack.post_message (#0)" in confirmation + assert confirmation == "Recorded slack.post_message as action-0." assert rec.actions == [ { "type": "slack.post_message", diff --git a/libs/hackbot-runtime/tests/test_testrail_action.py b/libs/hackbot-runtime/tests/test_testrail_action.py index 75fce89617..7c5a966a90 100644 --- a/libs/hackbot-runtime/tests/test_testrail_action.py +++ b/libs/hackbot-runtime/tests/test_testrail_action.py @@ -27,7 +27,7 @@ async def test_submit_test_plan_tool_records_deferred_action(): recorder, feature="Feature", generated_test_cases=_cases() ) - assert message == "Recorded testrail.submit_test_plan (#0)." + assert message == "Recorded testrail.submit_test_plan as action-0." assert recorder.actions[0]["type"] == ACTION_TYPE assert recorder.actions[0]["params"] == { "feature": "Feature", From c582040c521d9be962071b151a191f3523e06b90 Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Wed, 12 Aug 2026 13:10:45 +0200 Subject: [PATCH 2/7] Disable recorded action tools for agents --- agents/bug-fix/hackbot_agents/bug_fix/config.py | 4 ---- agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md | 2 +- .../frontend-triage/hackbot_agents/frontend_triage/config.py | 2 -- .../hackbot_agents/frontend_triage/prompts/system.md | 4 +--- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/agents/bug-fix/hackbot_agents/bug_fix/config.py b/agents/bug-fix/hackbot_agents/bug_fix/config.py index 3441d986ac..293c283682 100644 --- a/agents/bug-fix/hackbot_agents/bug_fix/config.py +++ b/agents/bug-fix/hackbot_agents/bug_fix/config.py @@ -19,8 +19,6 @@ # Action types that the agent may record during triage/fix runs. TRIAGE_AND_FIX_ACTIONS = [ - "recorded_actions.list_actions", - "recorded_actions.remove_action", "bugzilla.update_bug", "bugzilla.add_comment", "bugzilla.add_attachment", @@ -30,8 +28,6 @@ # Action types that the agent may record during follow-up runs on a revision. PHABRICATOR_FOLLOW_UP_ACTIONS = [ - "recorded_actions.list_actions", - "recorded_actions.remove_action", "bugzilla.update_bug", "bugzilla.add_attachment", "bugzilla.create_bug", diff --git a/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md b/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md index 4939906576..49201b2d9c 100644 --- a/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md +++ b/agents/bug-fix/hackbot_agents/bug_fix/prompts/system.md @@ -67,7 +67,7 @@ When you spawn an investigator via the Task tool, write a complete, self-contain # Recording actions -The `actions` MCP tools (`bugzilla_update_bug`, `bugzilla_add_comment`, and the Phabricator actions this run enables) do **not** mutate Bugzilla or Phabricator directly. Use `phabricator_add_comment`, when available, to reply on a Differential revision (for example, to answer a question when no code change is needed); use `phabricator_submit_patch` to deliver a code fix as a new revision, or `phabricator_update_patch` to deliver it as a new diff on the existing revision. Only the actions listed in your toolset are available: if one is missing, it does not apply to this run. They record an intended action into the run's `summary.json` for a human reviewer (or a downstream apply step) to enact. Recorded actions remain proposals until the run finishes; use `recorded_actions_list_actions` to review them and `recorded_actions_remove_action` to retract an inaccurate proposal. +The `actions` MCP tools (`bugzilla_update_bug`, `bugzilla_add_comment`, and the Phabricator actions this run enables) do **not** mutate Bugzilla or Phabricator directly. Use `phabricator_add_comment`, when available, to reply on a Differential revision (for example, to answer a question when no code change is needed); use `phabricator_submit_patch` to deliver a code fix as a new revision, or `phabricator_update_patch` to deliver it as a new diff on the existing revision. Only the actions listed in your toolset are available: if one is missing, it does not apply to this run. They record an intended action into the run's `summary.json` for a human reviewer (or a downstream apply step) to enact. Treat each recorded action as a final, irrevocable proposal — once recorded it appears in the run output verbatim. When you record `phabricator_submit_patch` to create a new revision, do not also record `bugzilla_add_comment` in the same run. The patch description already appears as a Bugzilla comment, so another comment would be redundant. diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py index 6b99413633..68fb274b67 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/config.py +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/config.py @@ -31,8 +31,6 @@ # Actions-server tools available to the agent, by dotted id. ENABLED_ACTION_TYPES = [ - "recorded_actions.list_actions", - "recorded_actions.remove_action", "bugzilla.add_comment", "bugzilla.update_bug", ] diff --git a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md index 6c541c289d..11201a7d89 100644 --- a/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md +++ b/agents/frontend-triage/hackbot_agents/frontend_triage/prompts/system.md @@ -80,9 +80,7 @@ When you spawn an investigator via the Task tool, write a complete, self-contain # Recording actions -The `actions` MCP tools (`bugzilla_add_comment`, `bugzilla_update_bug`) do **not** mutate Bugzilla directly. They record an intended action for the run's `summary.json`, where a human reviewer or downstream apply step can enact it. - -Recorded actions remain proposals until the run finishes. Use `recorded_actions_list_actions` when you need to recover or verify their complete details. If you discover that a proposal is inaccurate, retract exactly that action with `recorded_actions_remove_action`, then record a corrected action if needed. +The `actions` MCP tools (`bugzilla_add_comment`, `bugzilla_update_bug`) do **not** mutate Bugzilla directly. They record an intended action into the run's `summary.json` for a human reviewer (or a downstream apply step) to enact. Treat each recorded action as a final, irrevocable proposal. Before calling any action tool, state in your response: From 962b684c097edddc70becc669b1c01b7fbe067ee Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Mon, 17 Aug 2026 11:00:22 +0200 Subject: [PATCH 3/7] Simplify the recorded action removal response --- .../hackbot_runtime/actions/recorded_actions.py | 3 +-- libs/hackbot-runtime/hackbot_runtime/actions/recorder.py | 4 ---- libs/hackbot-runtime/tests/test_claude_sdk.py | 3 +-- libs/hackbot-runtime/tests/test_recorder.py | 1 - 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py index e3ae280734..6369e7a7d7 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py @@ -41,8 +41,7 @@ async def remove_action( applied. This operation accepts exactly one action ID and has no cascade or force mode. """ - removed = recorder.remove_action(action_id) - return {"removed": removed, "remaining_count": recorder.action_count} + return recorder.remove_action(action_id) TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py index 7108fc154d..89346138b7 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py @@ -156,10 +156,6 @@ def last_action_id(self) -> str: raise RuntimeError("No action has been recorded.") return self._last_action_id - @property - def action_count(self) -> int: - return len(self._actions) - @property def actions(self) -> list[dict]: return list(self._actions.values()) diff --git a/libs/hackbot-runtime/tests/test_claude_sdk.py b/libs/hackbot-runtime/tests/test_claude_sdk.py index 9dc836aa30..4e5a084c87 100644 --- a/libs/hackbot-runtime/tests/test_claude_sdk.py +++ b/libs/hackbot-runtime/tests/test_claude_sdk.py @@ -150,8 +150,7 @@ async def test_recorded_actions_tools_list_and_remove_complete_action(): srv, "recorded_actions_remove_action", {"action_id": "action-0"} ) removed = json.loads(removed_result.content[0].text) - assert removed["removed"] == listed[0] - assert removed["remaining_count"] == 0 + assert removed == listed[0] assert recorder.actions == [] diff --git a/libs/hackbot-runtime/tests/test_recorder.py b/libs/hackbot-runtime/tests/test_recorder.py index 3edd2a10d7..5f891fe220 100644 --- a/libs/hackbot-runtime/tests/test_recorder.py +++ b/libs/hackbot-runtime/tests/test_recorder.py @@ -259,7 +259,6 @@ def test_remove_action_deletes_only_the_requested_action(): assert removed["action_id"] == "action-0" assert removed["reasoning"] == "first" - assert rec.action_count == 1 assert rec.list_actions()[0]["action_id"] == "action-1" assert [action["type"] for action in rec.actions] == ["bugzilla.add_comment"] From 0f6dd279804e1cf50e0cbf6eb3b450400f9563fe Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Mon, 17 Aug 2026 11:29:39 +0200 Subject: [PATCH 4/7] Use stable action IDs in Try Server confirmations --- libs/hackbot-runtime/hackbot_runtime/actions/try_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py b/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py index 9b38e53496..f5e700304f 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py @@ -151,7 +151,7 @@ async def push( params["test_paths"] = validate_test_paths(tests) recorder.record(TRY_PUSH_ACTION_TYPE, params, reasoning=reasoning, ref=ref) - return f"Recorded {TRY_PUSH_ACTION_TYPE} (#{len(recorder.actions) - 1})." + return f"Recorded {TRY_PUSH_ACTION_TYPE} as {recorder.last_action_id}." TOOLS = tools_in(__name__) From c99c465900a92d76430e6e738ea1d87998ac004b Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Tue, 18 Aug 2026 00:16:26 +0200 Subject: [PATCH 5/7] Use opaque action IDs and return them atomically --- .../hackbot_runtime/actions/bugzilla.py | 20 ++++----- .../hackbot_runtime/actions/phabricator.py | 16 +++---- .../actions/recorded_actions.py | 5 +-- .../hackbot_runtime/actions/recorder.py | 36 +++++---------- .../hackbot_runtime/actions/slack.py | 4 +- .../hackbot_runtime/actions/testrail.py | 8 ++-- .../hackbot_runtime/actions/try_server.py | 4 +- libs/hackbot-runtime/tests/test_claude_sdk.py | 8 ++-- libs/hackbot-runtime/tests/test_recorder.py | 44 ++++++++++++------- libs/hackbot-runtime/tests/test_runtime.py | 4 +- .../tests/test_slack_actions.py | 6 ++- .../tests/test_testrail_action.py | 5 ++- 12 files changed, 80 insertions(+), 80 deletions(-) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py b/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py index f9a76defda..98c1b2532b 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/bugzilla.py @@ -26,8 +26,8 @@ ) -def _confirm(recorder: ActionsRecorder, action_type: str) -> str: - return f"Recorded {action_type} as {recorder.last_action_id}." +def _confirm(action: dict) -> str: + return f"Recorded {action['type']} (ID: {action['action_id']})." @tool @@ -61,12 +61,12 @@ async def update_bug( Recorded into the run summary for human review — does not modify Bugzilla. """ - recorder.record( + action = recorder.record( "bugzilla.update_bug", {"bug_id": bug_id, "changes": changes}, reasoning=reasoning, ) - return _confirm(recorder, "bugzilla.update_bug") + return _confirm(action) @tool @@ -91,12 +91,12 @@ async def add_comment( summary for human review — does not post to Bugzilla. """ text_with_footer = text.rstrip() + "\n\n---\n\n" + _COMMENT_FOOTER - recorder.record( + action = recorder.record( "bugzilla.add_comment", {"bug_id": bug_id, "text": text_with_footer, "is_private": is_private}, reasoning=reasoning, ) - return _confirm(recorder, "bugzilla.add_comment") + return _confirm(action) @tool @@ -181,13 +181,13 @@ async def add_attachment( if comment: params["comment"] = comment - recorder.record( + action = recorder.record( "bugzilla.add_attachment", params, reasoning=reasoning, attachments={"file": Path(file_path)}, ) - return _confirm(recorder, "bugzilla.add_attachment") + return _confirm(action) @tool @@ -231,8 +231,8 @@ async def create_bug( for k, v in (extra or {}).items(): body.setdefault(k, v) - recorder.record("bugzilla.create_bug", body, reasoning=reasoning) - return _confirm(recorder, "bugzilla.create_bug") + action = recorder.record("bugzilla.create_bug", body, reasoning=reasoning) + return _confirm(action) TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py b/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py index 5b9e29d251..5c70d80ed4 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py @@ -32,8 +32,8 @@ ) -def _confirm(recorder: ActionsRecorder, action_type: str) -> str: - return f"Recorded {action_type} as {recorder.last_action_id}." +def _confirm(action: dict) -> str: + return f"Recorded {action['type']} (ID: {action['action_id']})." def _validate_summary(summary: str | None) -> None: @@ -100,13 +100,13 @@ async def submit_patch( bug comment). """ _validate_summary(summary) - recorder.record( + action = recorder.record( "phabricator.submit_patch", {"bug_id": bug_id, "title": title, "summary": summary}, reasoning=reasoning, ref=ref, ) - return _confirm(recorder, "phabricator.submit_patch") + return _confirm(action) @tool @@ -141,12 +141,12 @@ async def update_patch( Only the diff changes: the revision keeps its title, summary, and bug association exactly as they are. """ - recorder.record( + action = recorder.record( "phabricator.update_patch", {"revision_id": revision_id}, reasoning=reasoning, ) - return _confirm(recorder, "phabricator.update_patch") + return _confirm(action) @tool @@ -167,12 +167,12 @@ async def add_comment( changes, use ``submit_patch`` instead. Recorded into the run summary for human review; nothing is posted to Phabricator during the run. """ - recorder.record( + action = recorder.record( "phabricator.add_comment", {"revision_id": revision_id, "text": text}, reasoning=reasoning, ) - return _confirm(recorder, "phabricator.add_comment") + return _confirm(action) TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py index 6369e7a7d7..299cd92a27 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py @@ -28,10 +28,7 @@ async def remove_action( action_id: Annotated[ str, Field( - description=( - "Exact stable ID returned when the action was recorded or by " - "list_actions (for example, action-2)." - ) + description="ID returned when the action was recorded or by list_actions." ), ], ) -> dict: diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py index 89346138b7..f26d9b007c 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorder.py @@ -1,4 +1,5 @@ import copy +import uuid from collections.abc import Callable, Mapping, Sequence from pathlib import Path @@ -40,12 +41,8 @@ def __init__( artifacts_dir: Path | None = None, hooks: Mapping[str, Sequence[ActionHook]] = {}, ) -> None: - # IDs are deliberately separate from the serialized action payload. They - # are handles for managing proposals while the agent is running, not a - # new field in the summary/API contract. self._actions: dict[str, dict] = {} self._next_action_sequence = 0 - self._last_action_id: str | None = None self._uploader = uploader self._artifacts_dir = artifacts_dir self._hooks = { @@ -70,7 +67,7 @@ def record( attachments: dict[str, Path] | None = None, ref: str | None = None, ) -> dict: - """Record an intended action. + """Record an action and return a detached copy with its ID. ``action_type`` uses ``.`` (e.g. ``bugzilla.update_bug``, ``phabricator.create_revision``). ``params`` is action-specific data @@ -99,7 +96,7 @@ def record( """ sequence = self._next_action_sequence self._next_action_sequence += 1 - action_id = f"action-{sequence}" + action_id = f"action-{uuid.uuid4().hex}" action: dict = { "type": action_type, "params": params, @@ -124,38 +121,29 @@ def record( action["attachments"] = recorded_attachments self._actions[action_id] = action - self._last_action_id = action_id - return action + return _detach(action_id, action) def list_actions(self) -> list[dict]: """Return complete copies of the current actions with stable in-run IDs.""" return [ - {**copy.deepcopy(action), "action_id": action_id} - for action_id, action in self._actions.items() + _detach(action_id, action) for action_id, action in self._actions.items() ] def remove_action(self, action_id: str) -> dict: - """Remove one action. - - The returned payload includes the stable ID and is detached from recorder - state. Removing an action only changes the proposals that will be written - to ``summary.json``; an attachment already uploaded for it may remain as - an unreferenced artifact until normal storage cleanup. - """ + """Remove and return an action.""" action = self._actions.get(action_id) if action is None: raise ToolError(f"No recorded action with ID {action_id!r}.") - removed = {**copy.deepcopy(action), "action_id": action_id} + removed = _detach(action_id, action) del self._actions[action_id] return removed - @property - def last_action_id(self) -> str: - if self._last_action_id is None: - raise RuntimeError("No action has been recorded.") - return self._last_action_id - @property def actions(self) -> list[dict]: return list(self._actions.values()) + + +def _detach(action_id: str, action: dict) -> dict: + """Return a detached copy of an action with its ID.""" + return {**copy.deepcopy(action), "action_id": action_id} diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/slack.py b/libs/hackbot-runtime/hackbot_runtime/actions/slack.py index 6abc152052..4fccbee380 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/slack.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/slack.py @@ -64,8 +64,8 @@ async def post_message( Recorded into the run summary for human review -- does not post to Slack. """ - recorder.record(ACTION_TYPE, _params(channel, text), reasoning=reasoning) - return f"Recorded {ACTION_TYPE} as {recorder.last_action_id}." + action = recorder.record(ACTION_TYPE, _params(channel, text), reasoning=reasoning) + return f"Recorded {ACTION_TYPE} (ID: {action['action_id']})." def record_message( diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py index d63014e247..7edcaf4a69 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py @@ -87,8 +87,8 @@ def feature_must_not_be_blank(cls, value: str) -> str: return value -def _confirm(recorder: ActionsRecorder, action_type: str) -> str: - return f"Recorded {action_type} as {recorder.last_action_id}." +def _confirm(action: dict) -> str: + return f"Recorded {action['type']} (ID: {action['action_id']})." def _validated_params(feature: str, generated_test_cases: list[Any]) -> dict[str, Any]: @@ -128,8 +128,8 @@ async def submit_test_plan( Nothing is sent to TestRail during the agent run. """ params = _validated_params(feature, generated_test_cases) - recorder.record(ACTION_TYPE, params) - return _confirm(recorder, ACTION_TYPE) + action = recorder.record(ACTION_TYPE, params) + return _confirm(action) def record_test_plan( diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py b/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py index f5e700304f..5723e6248d 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/try_server.py @@ -150,8 +150,8 @@ async def push( if tests: params["test_paths"] = validate_test_paths(tests) - recorder.record(TRY_PUSH_ACTION_TYPE, params, reasoning=reasoning, ref=ref) - return f"Recorded {TRY_PUSH_ACTION_TYPE} as {recorder.last_action_id}." + action = recorder.record(TRY_PUSH_ACTION_TYPE, params, reasoning=reasoning, ref=ref) + return f"Recorded {TRY_PUSH_ACTION_TYPE} (ID: {action['action_id']})." TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/tests/test_claude_sdk.py b/libs/hackbot-runtime/tests/test_claude_sdk.py index 4e5a084c87..57146b0509 100644 --- a/libs/hackbot-runtime/tests/test_claude_sdk.py +++ b/libs/hackbot-runtime/tests/test_claude_sdk.py @@ -128,11 +128,11 @@ async def test_actions_server_exposes_selected_testrail_tool(): async def test_recorded_actions_tools_list_and_remove_complete_action(): recorder = ActionsRecorder() - recorder.record( + action_id = recorder.record( "bugzilla.update_bug", {"bug_id": 7, "changes": {"severity": "S2"}}, reasoning="rule X", - ) + )["action_id"] srv = _server(recorder) listed_result = await _call(srv, "recorded_actions_list_actions", {}) @@ -142,12 +142,12 @@ async def test_recorded_actions_tools_list_and_remove_complete_action(): "type": "bugzilla.update_bug", "params": {"bug_id": 7, "changes": {"severity": "S2"}}, "reasoning": "rule X", - "action_id": "action-0", + "action_id": action_id, } ] removed_result = await _call( - srv, "recorded_actions_remove_action", {"action_id": "action-0"} + srv, "recorded_actions_remove_action", {"action_id": action_id} ) removed = json.loads(removed_result.content[0].text) assert removed == listed[0] diff --git a/libs/hackbot-runtime/tests/test_recorder.py b/libs/hackbot-runtime/tests/test_recorder.py index 5f891fe220..b72dc68024 100644 --- a/libs/hackbot-runtime/tests/test_recorder.py +++ b/libs/hackbot-runtime/tests/test_recorder.py @@ -22,6 +22,7 @@ def test_record_basic_shape(): {"bug_id": 1, "changes": {"severity": "S2"}}, reasoning="rule X", ) + assert returned.pop("action_id").startswith("action-") assert returned == rec.actions[0] assert rec.actions == [ { @@ -109,7 +110,7 @@ def second(action): assert calls == ["first", "second"] assert returned["params"] == {"bug_id": 1, "priority": "P1", "seen": "P1"} - assert rec.actions[0] == returned + assert rec.actions[0]["params"] == returned["params"] def test_hooks_only_run_for_their_action_type(): @@ -222,13 +223,13 @@ def test_constructor_hooks_are_copied(): def test_list_actions_returns_stable_ids_and_complete_detached_payloads(): rec = ActionsRecorder() - rec.record( + patch = rec.record( "phabricator.submit_patch", {"bug_id": 1, "title": "Fix"}, reasoning="verified fix", ref="patch", ) - rec.record( + comment = rec.record( "bugzilla.add_comment", {"bug_id": 1, "text": "See {{actions.patch.url}}"}, reasoning="announce the patch", @@ -236,9 +237,12 @@ def test_list_actions_returns_stable_ids_and_complete_detached_payloads(): listed = rec.list_actions() - assert [action["action_id"] for action in listed] == ["action-0", "action-1"] + assert [action["action_id"] for action in listed] == [ + patch["action_id"], + comment["action_id"], + ] assert listed[0] == { - "action_id": "action-0", + "action_id": patch["action_id"], "type": "phabricator.submit_patch", "params": {"bug_id": 1, "title": "Fix"}, "reasoning": "verified fix", @@ -252,24 +256,24 @@ def test_list_actions_returns_stable_ids_and_complete_detached_payloads(): def test_remove_action_deletes_only_the_requested_action(): rec = ActionsRecorder() - rec.record("bugzilla.update_bug", {"bug_id": 1}, reasoning="first") - rec.record("bugzilla.add_comment", {"bug_id": 1}, reasoning="second") + first = rec.record("bugzilla.update_bug", {"bug_id": 1}, reasoning="first") + second = rec.record("bugzilla.add_comment", {"bug_id": 1}, reasoning="second") - removed = rec.remove_action("action-0") + removed = rec.remove_action(first["action_id"]) - assert removed["action_id"] == "action-0" + assert removed["action_id"] == first["action_id"] assert removed["reasoning"] == "first" - assert rec.list_actions()[0]["action_id"] == "action-1" + assert rec.list_actions()[0]["action_id"] == second["action_id"] assert [action["type"] for action in rec.actions] == ["bugzilla.add_comment"] def test_remove_action_rejects_unknown_or_already_removed_id(): rec = ActionsRecorder() - rec.record("bugzilla.update_bug", {"bug_id": 1}) - rec.remove_action("action-0") + action_id = rec.record("bugzilla.update_bug", {"bug_id": 1})["action_id"] + rec.remove_action(action_id) with pytest.raises(ToolError, match="No recorded action"): - rec.remove_action("action-0") + rec.remove_action(action_id) def test_removed_action_id_and_attachment_key_are_not_reused(tmp_path): @@ -279,11 +283,17 @@ def test_removed_action_id_and_attachment_key_are_not_reused(tmp_path): second.write_text("second") rec = ActionsRecorder(artifacts_dir=tmp_path / "artifacts") - rec.record("bugzilla.add_attachment", {"bug_id": 1}, attachments={"file": first}) - rec.remove_action("action-0") - rec.record("bugzilla.add_attachment", {"bug_id": 1}, attachments={"file": second}) + removed_id = rec.record( + "bugzilla.add_attachment", {"bug_id": 1}, attachments={"file": first} + )["action_id"] + rec.remove_action(removed_id) + kept_id = rec.record( + "bugzilla.add_attachment", {"bug_id": 1}, attachments={"file": second} + )["action_id"] + + assert kept_id != removed_id + assert rec.list_actions()[0]["action_id"] == kept_id - assert rec.list_actions()[0]["action_id"] == "action-1" assert rec.actions[0]["attachments"] == [ {"name": "file", "uploaded_key": "attachments/1/file"} ] diff --git a/libs/hackbot-runtime/tests/test_runtime.py b/libs/hackbot-runtime/tests/test_runtime.py index f7e1483096..4a638a79a0 100644 --- a/libs/hackbot-runtime/tests/test_runtime.py +++ b/libs/hackbot-runtime/tests/test_runtime.py @@ -58,7 +58,7 @@ def test_summary_written_for_exception(tmp_path): def test_removed_action_is_absent_from_summary(tmp_path): ctx = _ctx(tmp_path) - ctx.actions.record( + inaccurate = ctx.actions.record( "bugzilla.update_bug", {"bug_id": 1, "changes": {"severity": "S2"}}, reasoning="inaccurate", @@ -68,7 +68,7 @@ def test_removed_action_is_absent_from_summary(tmp_path): {"bug_id": 1, "text": "Corrected assessment"}, reasoning="corrected", ) - ctx.actions.remove_action("action-0") + ctx.actions.remove_action(inaccurate["action_id"]) code = _finish(ctx, HackbotAgentResult(num_turns=1)) diff --git a/libs/hackbot-runtime/tests/test_slack_actions.py b/libs/hackbot-runtime/tests/test_slack_actions.py index 6dd41cefd7..b925754c23 100644 --- a/libs/hackbot-runtime/tests/test_slack_actions.py +++ b/libs/hackbot-runtime/tests/test_slack_actions.py @@ -14,7 +14,9 @@ async def test_post_message_records_action(): text=" a test regressed ", reasoning="sheriffs decide on the backout", ) - assert confirmation == "Recorded slack.post_message as action-0." + assert confirmation == ( + f"Recorded slack.post_message (ID: {rec.list_actions()[0]['action_id']})." + ) assert rec.actions == [ { "type": "slack.post_message", @@ -39,7 +41,7 @@ def test_record_message_supports_a_ref_for_later_reference(): action = slack.record_message(rec, "sheriffs", "backout recommended", ref="notice") assert action["ref"] == "notice" assert action["params"]["channel"] == "sheriffs" - assert rec.actions == [action] + assert rec.actions[0]["ref"] == "notice" def test_tools_are_exposed_under_the_slack_namespace(): diff --git a/libs/hackbot-runtime/tests/test_testrail_action.py b/libs/hackbot-runtime/tests/test_testrail_action.py index 7c5a966a90..4eecba5419 100644 --- a/libs/hackbot-runtime/tests/test_testrail_action.py +++ b/libs/hackbot-runtime/tests/test_testrail_action.py @@ -27,7 +27,10 @@ async def test_submit_test_plan_tool_records_deferred_action(): recorder, feature="Feature", generated_test_cases=_cases() ) - assert message == "Recorded testrail.submit_test_plan as action-0." + assert message == ( + f"Recorded testrail.submit_test_plan " + f"(ID: {recorder.list_actions()[0]['action_id']})." + ) assert recorder.actions[0]["type"] == ACTION_TYPE assert recorder.actions[0]["params"] == { "feature": "Feature", From de9fab7adac9033af27654085aea40bd2485fc53 Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Tue, 18 Aug 2026 00:52:46 +0200 Subject: [PATCH 6/7] Format recorded actions as a Markdown table --- .../actions/recorded_actions.py | 29 ++++++++++++++----- libs/hackbot-runtime/tests/test_claude_sdk.py | 19 ++++++------ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py index 299cd92a27..f2138a801d 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py @@ -10,16 +10,31 @@ from hackbot_runtime.actions.recorder import ActionsRecorder +def _table_cell(value: str | None) -> str: + """Format one value for a Markdown table cell.""" + if value is None: + return "" + return "
".join(value.splitlines()).replace("|", r"\|") + + @tool -async def list_actions(recorder: ActionsRecorder) -> list[dict]: - """List every action currently proposed by this agent run. +async def list_actions(recorder: ActionsRecorder) -> str: + """List the actions currently proposed by this agent run. - Returns each action's stable in-run ID and its complete recorded payload, - including parameters, reasoning, references, and attachment metadata. Use - this when earlier action details are no longer present in your context or - before deciding whether a proposal needs to be retracted. + Returns a Markdown table with each action's ID, type, and reasoning. """ - return recorder.list_actions() + actions = recorder.list_actions() + if not actions: + return "No recorded actions." + + rows = ["| ID | Action | Reasoning |", "| --- | --- | --- |"] + rows.extend( + f"| {_table_cell(action['action_id'])} " + f"| {_table_cell(action['type'])} " + f"| {_table_cell(action.get('reasoning'))} |" + for action in actions + ) + return "\n".join(rows) @tool diff --git a/libs/hackbot-runtime/tests/test_claude_sdk.py b/libs/hackbot-runtime/tests/test_claude_sdk.py index 57146b0509..07cecf7b6f 100644 --- a/libs/hackbot-runtime/tests/test_claude_sdk.py +++ b/libs/hackbot-runtime/tests/test_claude_sdk.py @@ -136,21 +136,22 @@ async def test_recorded_actions_tools_list_and_remove_complete_action(): srv = _server(recorder) listed_result = await _call(srv, "recorded_actions_list_actions", {}) - listed = json.loads(listed_result.content[0].text) - assert listed == [ - { - "type": "bugzilla.update_bug", - "params": {"bug_id": 7, "changes": {"severity": "S2"}}, - "reasoning": "rule X", - "action_id": action_id, - } + assert listed_result.content[0].text.splitlines() == [ + "| ID | Action | Reasoning |", + "| --- | --- | --- |", + f"| {action_id} | bugzilla.update_bug | rule X |", ] removed_result = await _call( srv, "recorded_actions_remove_action", {"action_id": action_id} ) removed = json.loads(removed_result.content[0].text) - assert removed == listed[0] + assert removed == { + "type": "bugzilla.update_bug", + "params": {"bug_id": 7, "changes": {"severity": "S2"}}, + "reasoning": "rule X", + "action_id": action_id, + } assert recorder.actions == [] From 83547bc28d0378c0a8735186ced6cdf1669d99a5 Mon Sep 17 00:00:00 2001 From: ayoubdiourin7 Date: Tue, 18 Aug 2026 11:26:45 +0200 Subject: [PATCH 7/7] Simplify recorded action tool descriptions --- .../hackbot_runtime/actions/recorded_actions.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py index f2138a801d..b5960277de 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/recorded_actions.py @@ -42,17 +42,10 @@ async def remove_action( recorder: ActionsRecorder, action_id: Annotated[ str, - Field( - description="ID returned when the action was recorded or by list_actions." - ), + Field(description="ID of the action to remove."), ], ) -> dict: - """Retract one proposed action from this agent run. - - The removed action will not appear in the final run summary and cannot be - applied. This operation accepts exactly one action ID and has no cascade or - force mode. - """ + """Remove and return a recorded action.""" return recorder.remove_action(action_id)