test: sweep sibling tests stale on the tool renames + deferral default

The rename sweep in the base commit missed the sibling-test blast radius
(18 red files on CI). Three classes, all fixed:

1. Stale old names in tests (todo/cronjob/process/tour/tip) — updated to
   todo_list/cronjob_manage/process_manage/gui_tour/show_tip at every
   registry.get_entry/dispatch/coerce/preview/allowlist call site, plus
   the coding-brief sentence in agent/coding_context.py now names
   todo_list (and its gating test).
2. Missed rename in production: AGENT_RUNTIME_POST_HOOK_TOOL_NAMES still
   held 'tour' — post-hook ownership would have double-emitted for
   gui_tour via the bridge path.
3. Tests pinning pre-deferral assembly (blank-slate surface, modal
   sandbox resolution, desktop diet, HUD note) now pin their ACTUAL
   contract under the legacy defer:[] override, or assert on granted
   tool names instead of visible schemas.

Also fixes a pre-existing ordering flake surfaced by the sweep:
test_holds_exactly_the_gui_affordances depended on whether an earlier
test had imported apply_layout_tool (registry-registered, not in the
static desktop_ui list) — now forces discovery and pins the full set.

649 tests green locally across all touched files, both orderings.
This commit is contained in:
Teknium
2026-08-29 18:22:47 -07:00
parent 03e66c8cba
commit c1762ff11c
20 changed files with 98 additions and 60 deletions
+1 -1
View File
@@ -108,7 +108,7 @@ def _ra():
AGENT_RUNTIME_POST_HOOK_TOOL_NAMES = frozenset(
{"todo_list", "session_search", "memory", "clarify", "read_terminal", "desktop_preview", "drive_preview", "annotate_preview", "read_window_below", "setup_mcp", "tour", "delegate_task"}
{"todo_list", "session_search", "memory", "clarify", "read_terminal", "desktop_preview", "drive_preview", "annotate_preview", "read_window_below", "setup_mcp", "gui_tour", "delegate_task"}
)
+2 -2
View File
@@ -253,7 +253,7 @@ CODING_AGENT_GUIDANCE = (
"paths for the same flaw and fix the class, not just the reported site.\n"
"- When fixing linter/type errors on a file, stop after about three "
"attempts on the same file and ask the user rather than looping.\n"
"- Track multi-step work with `todo`. Reference code as `path:line` instead "
"- Track multi-step work with `todo_list`. Reference code as `path:line` instead "
"of pasting whole files.\n"
"\n"
"Respect the user's repo: don't commit, push, or rewrite history unless "
@@ -549,7 +549,7 @@ class RuntimeMode:
brief = self.profile.guidance
if valid_tool_names is not None and "todo_list" not in valid_tool_names:
brief = brief.replace(
"- Track multi-step work with `todo`. Reference code as "
"- Track multi-step work with `todo_list`. Reference code as "
"`path:line` instead of pasting whole files.",
"- Reference code as `path:line` instead of pasting "
"whole files.",
+10 -10
View File
@@ -26,7 +26,7 @@ class TestTodoRead:
"""get_cute_tool_message(…, result=…) when todos_arg is None (read path)."""
def test_read_no_result(self):
msg = get_cute_tool_message("todo", {}, 0.5)
msg = get_cute_tool_message("todo_list", {}, 0.5)
assert "reading tasks" in msg
assert "0.5s" in msg
@@ -34,7 +34,7 @@ class TestTodoRead:
def test_read_zero_total(self):
"""Edge case: empty todo list returns summary with total=0."""
msg = get_cute_tool_message("todo", {}, 0.5,
msg = get_cute_tool_message("todo_list", {}, 0.5,
result=_todo_result(0, 0))
assert "reading tasks" in msg
@@ -46,7 +46,7 @@ class TestTodoCreate:
def test_create_default(self):
"""Brand-new plan: all pending, no result — plain count."""
msg = get_cute_tool_message("todo",
msg = get_cute_tool_message("todo_list",
{"todos": [
{"id": "a", "content": "x", "status": "pending"},
]}, 0.3)
@@ -58,7 +58,7 @@ class TestTodoCreate:
def test_create_with_result_zero_done(self):
"""New plan with 0 done — plain count, no progress fraction."""
msg = get_cute_tool_message("todo",
msg = get_cute_tool_message("todo_list",
{"todos": [
{"id": "a", "content": "x", "status": "pending"},
{"id": "b", "content": "y", "status": "pending"},
@@ -74,7 +74,7 @@ class TestTodoUpdate:
def test_update_no_result(self):
"""No result available — plain update N task(s)."""
msg = get_cute_tool_message("todo",
msg = get_cute_tool_message("todo_list",
{"todos": [{"id": "a", "status": "completed"}],
"merge": True}, 0.5)
assert "update 1 task(s)" in msg
@@ -82,7 +82,7 @@ class TestTodoUpdate:
def test_update_halfway(self):
"""2/4 — midpoint progress."""
msg = get_cute_tool_message("todo",
msg = get_cute_tool_message("todo_list",
{"todos": [{"id": "b", "status": "in_progress"}],
"merge": True},
0.7,
@@ -96,7 +96,7 @@ class TestTodoUpdate:
def test_update_total_not_in_summary(self):
"""Result summary missing total key."""
msg = get_cute_tool_message("todo",
msg = get_cute_tool_message("todo_list",
{"todos": [{"id": "a", "status": "completed"}],
"merge": True},
0.3,
@@ -111,7 +111,7 @@ class TestTodoEdgeCases:
def test_merge_default_value(self):
"""merge defaults to False in function signature, should be False when absent."""
msg = get_cute_tool_message("todo",
msg = get_cute_tool_message("todo_list",
{"todos": [{"id": "a", "content": "x", "status": "pending"}]},
1.0)
assert "1 task(s)" in msg
@@ -120,7 +120,7 @@ class TestTodoEdgeCases:
def test_large_task_count(self):
"""Many tasks should not break formatting."""
many = [{"id": str(i), "content": "x", "status": "pending"} for i in range(50)]
msg = get_cute_tool_message("todo", {"todos": many}, 0.5)
msg = get_cute_tool_message("todo_list", {"todos": many}, 0.5)
assert "50 task(s)" in msg
@@ -131,7 +131,7 @@ class TestTodoSkinIntegration:
"""
def test_default_skin_prefix(self):
msg = get_cute_tool_message("todo", {}, 0.5)
msg = get_cute_tool_message("todo_list", {}, 0.5)
assert msg.startswith("┊")
+3 -3
View File
@@ -65,8 +65,8 @@ class TestCodingBriefTodoGating:
return prefix[0]
def test_todo_kept_when_tool_available(self):
brief = self._brief({"todo", "terminal", "read_file"})
assert "Track multi-step work with `todo`" in brief
brief = self._brief({"todo_list", "terminal", "read_file"})
assert "Track multi-step work with `todo_list`" in brief
def test_todo_dropped_when_tool_missing(self):
brief = self._brief({"terminal", "read_file"})
@@ -76,7 +76,7 @@ class TestCodingBriefTodoGating:
def test_unknown_toolset_keeps_full_brief(self):
brief = self._brief(None)
assert "Track multi-step work with `todo`" in brief
assert "Track multi-step work with `todo_list`" in brief
class TestEssentialSkillsUndisableable:
+1 -1
View File
@@ -92,7 +92,7 @@ def test_arg_canonicalization_ignores_key_order():
def test_allowlisted_pollers_never_fire():
c = ToolCallGuardrailController()
for tool in ("process", "vendor_get_result", "job_poll"):
for tool in ("process_manage", "vendor_get_result", "job_poll"):
for _ in range(STALL_GUARD_IDENTICAL_CALL_THRESHOLD + 2):
assert c.observe_identical_call(tool, {"id": "j1"}, "Generating") is None
@@ -112,7 +112,7 @@ class TestBackstopWrapper:
"terminal", "read_file", "write_file", "search_files", "patch",
"browser_navigate", "web_search", "web_extract", "delegate_task",
"execute_code", "skill_view", "vision_analyze", "memory",
"cronjob", "process", "totally_unknown_tool",
"cronjob_manage", "process_manage", "totally_unknown_tool",
]
keys = ["command", "path", "content", "pattern", "url", "query",
"urls", "goal", "code", "name", "question", "action",
@@ -151,12 +151,12 @@ class TestDisplayPreviewTypeSafety:
def test_process_preview_non_string_data(self):
from agent.display import build_tool_preview
result = build_tool_preview(
"process", {"action": "submit", "session_id": "abc", "data": 42}
"process_manage", {"action": "submit", "session_id": "abc", "data": 42}
)
assert result == 'submit abc "42"'
def test_process_preview_none_action(self):
from agent.display import build_tool_preview
result = build_tool_preview("process", {"action": None, "session_id": "abc"})
result = build_tool_preview("process_manage", {"action": None, "session_id": "abc"})
assert isinstance(result, str)
+1 -1
View File
@@ -194,7 +194,7 @@ class TestCronjobToolReasoningEffort:
def _tool_handler(self):
import tools.cronjob_tools as mod
return mod.registry._tools["cronjob"].handler
return mod.registry._tools["cronjob_manage"].handler
def test_schema_does_not_expose_reasoning_effort(self):
"""Policy pin: the model-facing surface must NOT offer the
+2 -2
View File
@@ -17,11 +17,11 @@ class TestHermesApiServerToolset:
def test_toolset_includes_core_tools(self):
tools = resolve_toolset("hermes-api-server")
expected = [
"terminal", "process",
"terminal", "process_manage",
"read_file", "write_file", "patch", "search_files",
"vision_analyze", "image_generate",
"execute_code", "delegate_task",
"todo", "memory", "session_search", "cronjob",
"todo_list", "memory", "session_search", "cronjob_manage",
]
for tool in expected:
assert tool in tools, f"Missing expected tool: {tool}"
+9 -1
View File
@@ -53,6 +53,14 @@ class TestBlankSlateMinimalToolsets:
from tools.registry import registry as _tool_registry
_entry = _tool_registry.get_entry("vision_analyze")
monkeypatch.setattr(_entry, "check_fn", lambda: True)
# This test pins disabled_toolsets SUBTRACTION, not deferral policy —
# assemble with the legacy everything-eager override so the expected
# list stays deferral-independent (#97979 defers process_manage by
# default, which would swap it for the three bridge tools here).
from tools.tool_search import ToolSearchConfig
_legacy = ToolSearchConfig.from_raw({"enabled": "on", "defer": []})
monkeypatch.setattr("tools.tool_search.load_config", lambda: _legacy)
monkeypatch.setattr("tools.tool_search.load_config_readonly", lambda: _legacy)
from hermes_cli.tools_config import _get_platform_tools
cfg = {}
_blank_slate_minimal_toolsets(cfg)
@@ -67,7 +75,7 @@ class TestBlankSlateMinimalToolsets:
names = sorted(
{(d.get("function") or {}).get("name") or d.get("name") for d in defs}
)
assert names == ["patch", "process", "read_file", "search_files",
assert names == ["patch", "process_manage", "read_file", "search_files",
"skill_manage", "skill_view", "skills_list",
"terminal", "vision_analyze", "write_file"]
+4 -4
View File
@@ -2203,7 +2203,7 @@ class TestConcurrentToolExecution:
def test_invoke_tool_handles_agent_level_tools(self, agent):
"""_invoke_tool should handle todo tool directly."""
with patch("tools.todo_tool.todo_tool", return_value='{"ok":true}') as mock_todo:
result = agent._invoke_tool("todo", {"todos": []}, "task-1")
result = agent._invoke_tool("todo_list", {"todos": []}, "task-1")
mock_todo.assert_called_once()
assert "ok" in result
@@ -2295,7 +2295,7 @@ class TestConcurrentToolExecution:
"""Sequential and concurrent agent-level paths share post-hook ownership."""
from agent.agent_runtime_helpers import agent_runtime_owns_post_tool_hook
for tool_name in ("todo", "session_search", "memory", "clarify", "delegate_task"):
for tool_name in ("todo_list", "session_search", "memory", "clarify", "delegate_task"):
assert agent_runtime_owns_post_tool_hook(agent, tool_name) is True
agent._context_engine_tool_names = {"context_query"}
@@ -2441,7 +2441,7 @@ class TestAgentRuntimePostHookOwnershipSync:
"""Exercise post-hook ownership through both agent-runtime tool paths."""
_CASES = (
("todo", {"todos": []}),
("todo_list", {"todos": []}),
("session_search", {"query": "needle"}),
("memory", {"action": "view", "target": "memory"}),
("clarify", {"question": "Continue?"}),
@@ -2451,7 +2451,7 @@ class TestAgentRuntimePostHookOwnershipSync:
("annotate_preview", {"action": "clear"}),
("read_window_below", {}),
("setup_mcp", {"server": "linear", "action": "install"}),
("tour", {"action": "stop"}),
("gui_tour", {"action": "stop"}),
("delegate_task", {"goal": "Check the child path"}),
)
+1 -1
View File
@@ -244,5 +244,5 @@ class TestCoerceToolArgsNested:
"""Against the real todo schema from the registry."""
import json as _json
args = {"todos": [_json.dumps({"id": "1", "content": "x", "status": "pending"})]}
result = coerce_tool_args("todo", args)
result = coerce_tool_args("todo_list", args)
assert result["todos"][0] == {"id": "1", "content": "x", "status": "pending"}
+1 -1
View File
@@ -227,7 +227,7 @@ class TestHandleFunctionCall:
class TestAgentLoopTools:
def test_expected_tools_in_set(self):
assert "todo" in _AGENT_LOOP_TOOLS
assert "todo_list" in _AGENT_LOOP_TOOLS
assert "memory" in _AGENT_LOOP_TOOLS
assert "session_search" in _AGENT_LOOP_TOOLS
assert "delegate_task" in _AGENT_LOOP_TOOLS
+2 -2
View File
@@ -265,7 +265,7 @@ class TestResolveToolsetIncludeRegistry:
finally:
registry.deregister("__probe_registry_only_tool__")
assert static == {"terminal", "process"}, static
assert static == {"terminal", "process_manage"}, static
# Registered into 'terminal' but not part of the static definition — it
# must only appear in the merged view.
assert "__probe_registry_only_tool__" in merged
@@ -275,7 +275,7 @@ class TestResolveToolsetIncludeRegistry:
def test_static_view_threads_through_includes(self):
# 'debugging' has direct tools [terminal, process] and includes [web, file]
static = set(resolve_toolset("debugging", include_registry=False))
assert {"terminal", "process"} <= static
assert {"terminal", "process_manage"} <= static
assert "web_search" in static
assert "read_file" in static
+9 -9
View File
@@ -427,7 +427,7 @@ class TestAgentCannotSetModelPin:
updated = json.loads(
registry.dispatch(
"cronjob",
"cronjob_manage",
{
"action": "update",
"job_id": job_id,
@@ -461,7 +461,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
created = json.loads(
registry.dispatch(
"cronjob",
"cronjob_manage",
{
"action": "create",
"name": "Continuable cron canary",
@@ -478,7 +478,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
stored = get_job(created["job_id"])
assert stored is not None
assert stored.get("attach_to_session") is True
listing = json.loads(registry.dispatch("cronjob", {"action": "list"}))
listing = json.loads(registry.dispatch("cronjob_manage", {"action": "list"}))
listed = next(j for j in listing["jobs"] if j["job_id"] == created["job_id"])
assert listed.get("attach_to_session") is True
@@ -488,7 +488,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
created = json.loads(
registry.dispatch(
"cronjob",
"cronjob_manage",
{
"action": "create",
"name": "plain",
@@ -502,7 +502,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
updated = json.loads(
registry.dispatch(
"cronjob",
"cronjob_manage",
{
"action": "update",
"job_id": created["job_id"],
@@ -518,7 +518,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
disabled = json.loads(
registry.dispatch(
"cronjob",
"cronjob_manage",
{
"action": "update",
"job_id": created["job_id"],
@@ -531,7 +531,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
stored = get_job(created["job_id"])
assert stored is not None
assert stored.get("attach_to_session") is False
listing = json.loads(registry.dispatch("cronjob", {"action": "list"}))
listing = json.loads(registry.dispatch("cronjob_manage", {"action": "list"}))
listed = next(j for j in listing["jobs"] if j["job_id"] == created["job_id"])
assert listed.get("attach_to_session") is False
@@ -541,7 +541,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
created = json.loads(
registry.dispatch(
"cronjob",
"cronjob_manage",
{
"action": "create",
"schedule": "1h",
@@ -554,7 +554,7 @@ class TestRegisteredHandlerForwardsAttachToSession:
assert stored is not None
assert "attach_to_session" not in stored
# And the formatted list output must not invent the field either.
listed = json.loads(registry.dispatch("cronjob", {"action": "list"}))
listed = json.loads(registry.dispatch("cronjob_manage", {"action": "list"}))
formatted = next(
j for j in listed["jobs"] if j["job_id"] == created["job_id"]
)
+17 -7
View File
@@ -26,14 +26,24 @@ class TestConsolidatedToolsets(unittest.TestCase):
self.assertEqual(proj, ["desktop_project"])
def test_registry_serves_only_new_names(self):
from model_tools import get_tool_definitions
"""Post-#97979 the GUI surface defers by default, so assemble with
the legacy everything-eager override (defer: []) — the contract
pinned here is the RENAME (new names only, dead names gone), not
the deferral policy."""
from unittest.mock import patch as _patch
names = {
t["function"]["name"]
for t in get_tool_definitions(
quiet_mode=True, enabled_toolsets=["desktop_ui", "project"]
)
}
from model_tools import get_tool_definitions
from tools.tool_search import ToolSearchConfig
legacy = ToolSearchConfig.from_raw({"enabled": "on", "defer": []})
with _patch("tools.tool_search.load_config_readonly", return_value=legacy), \
_patch("tools.tool_search.load_config", return_value=legacy):
names = {
t["function"]["name"]
for t in get_tool_definitions(
quiet_mode=True, enabled_toolsets=["desktop_ui", "project"]
)
}
self.assertIn("desktop_preview", names)
self.assertIn("desktop_project", names)
for dead in (
+14 -5
View File
@@ -36,13 +36,22 @@ class TestToolResolution:
def test_terminal_and_file_toolsets_resolve_all_tools(self):
"""enabled_toolsets=['terminal', 'file'] should produce 6 tools."""
from unittest.mock import patch as _patch
from model_tools import get_tool_definitions
tools = get_tool_definitions(
enabled_toolsets=["terminal", "file"],
quiet_mode=True,
)
from tools.tool_search import ToolSearchConfig
# Pin the RESOLUTION contract independent of deferral policy —
# #97979 defers process_manage by default (legacy defer: [] override).
_legacy = ToolSearchConfig.from_raw({"enabled": "on", "defer": []})
with _patch("tools.tool_search.load_config", return_value=_legacy), \
_patch("tools.tool_search.load_config_readonly", return_value=_legacy):
tools = get_tool_definitions(
enabled_toolsets=["terminal", "file"],
quiet_mode=True,
)
names = {t["function"]["name"] for t in tools}
expected = {"terminal", "process", "read_file", "write_file", "search_files", "patch"}
expected = {"terminal", "process_manage", "read_file", "write_file", "search_files", "patch"}
assert expected == names, f"Expected {expected}, got {names}"
def test_terminal_tool_present(self):
+2 -2
View File
@@ -24,7 +24,7 @@ def emitted(monkeypatch):
def test_lives_in_the_gui_surface_toolset(monkeypatch):
"""Scoped by toolset, not by the backend's env — see AGENTS.md."""
monkeypatch.delenv("HERMES_DESKTOP", raising=False)
entry = registry.get_entry("tip")
entry = registry.get_entry("show_tip")
assert entry is not None
assert entry.toolset == "desktop_ui"
@@ -32,7 +32,7 @@ def test_lives_in_the_gui_surface_toolset(monkeypatch):
def test_is_ungated_like_tour():
"""The Appearance switch governs the app's idle rotation, not this."""
entry = registry.get_entry("tip")
entry = registry.get_entry("show_tip")
assert entry is not None
assert entry.check_fn is None
+1 -1
View File
@@ -14,7 +14,7 @@ def _run(**kwargs):
def test_lives_in_the_gui_surface_toolset(monkeypatch):
"""Scoped by toolset, not by the backend's env — see AGENTS.md."""
monkeypatch.delenv("HERMES_DESKTOP", raising=False)
entry = registry.get_entry("tour")
entry = registry.get_entry("gui_tour")
assert entry is not None
assert entry.toolset == "desktop_ui"
@@ -27,8 +27,8 @@ GUI_TOOLS = {
"read_window_below",
"react_to_message",
"setup_mcp",
"tip",
"tour",
"show_tip",
"gui_tour",
}
@@ -43,7 +43,13 @@ def no_desktop_env(monkeypatch):
class TestDesktopUiToolset:
def test_holds_exactly_the_gui_affordances(self):
assert set(resolve_toolset("desktop_ui")) == GUI_TOOLS
# apply_layout registers into desktop_ui via the registry (not the
# static toolsets.py list), so force discovery first — otherwise the
# result depends on which earlier test imported tool modules
# (pre-existing ordering flake, surfaced by the #97979 test sweep).
from tools.registry import discover_builtin_tools
discover_builtin_tools()
assert set(resolve_toolset("desktop_ui")) == GUI_TOOLS | {"apply_layout"}
def test_stays_off_the_core_tool_list(self):
"""Core ships on every API call — a GUI-only tool must not be there."""
+6 -1
View File
@@ -117,7 +117,12 @@ class TestTurnRouting:
assert assembled.activated
assert mcp_name not in names
assert server._hud_surface_note(_session(tools=names, client_surface="hud")) == (
# Production computes the note from agent.valid_tool_names — the
# GRANTED set — not from the visible post-assembly schemas. Under
# #97979 the HUD kit (read_window_below, computer_use) is deferred
# behind the bridge yet still granted/callable, so the note must
# survive assembly unchanged.
assert server._hud_surface_note(_session(tools=FULL_KIT, client_surface="hud")) == (
hud_surface_note(FULL_KIT)
)