From eb8a30cc2f6416c8285ad5e5be7acd2d15ebc5e7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:27:01 -0700 Subject: [PATCH] =?UTF-8?q?simplify(compat):=20skills=5Fsync/skill=5Fmanag?= =?UTF-8?q?er/kanban/computer=5Fuse=20=E2=80=94=20drop=2037=20re-exports/a?= =?UTF-8?q?liases,=20repoint=208=20callers=20+=209=20test=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- agent/background_review.py | 2 +- hermes_cli/setup_quick.py | 3 ++- run_agent.py | 2 +- tests/agent/test_phantom_tool_references.py | 2 +- tests/hermes_cli/test_setup_blank_slate.py | 2 +- .../test_background_review_toolset_restriction.py | 4 ++-- tests/tools/test_skill_ledger.py | 2 +- tests/tools/test_skill_manager_tool.py | 8 ++++---- tests/tools/test_skills_list_modified_diff.py | 4 ++-- tests/tools/test_skills_sync.py | 11 +++++------ tests/tools/test_zombie_process_cleanup.py | 6 +++--- tools/approval.py | 2 +- tools/computer_use/__init__.py | 10 ---------- tools/file_tools.py | 2 +- tools/kanban_tools.py | 4 ++-- tools/skill_manager_tool.py | 14 +++++--------- tools/skills_sync.py | 14 +++----------- tools/skills_sync_optional.py | 2 +- tools/skills_tool_plugin.py | 2 +- 19 files changed, 37 insertions(+), 59 deletions(-) diff --git a/agent/background_review.py b/agent/background_review.py index a83382d032..5d40c35995 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -961,7 +961,7 @@ def _run_review_fork( ), ) with suppress(Exception): - from tools.skill_manager_tool import _reset_background_review_read_marks + from tools.skill_manager_guards import _reset_background_review_read_marks _reset_background_review_read_marks() try: diff --git a/hermes_cli/setup_quick.py b/hermes_cli/setup_quick.py index f9373bab28..b31dcfd22f 100644 --- a/hermes_cli/setup_quick.py +++ b/hermes_cli/setup_quick.py @@ -203,7 +203,8 @@ def _set_bundled_skills_opt_out(opt_out: bool, log_label: str, on_success=None, """Record the bundled-skills opt-out marker and sync (essential skills are always seeded); ``on_success(sync_result)`` / ``on_error(exc)`` report the outcome.""" try: - from tools.skills_sync import set_bundled_skills_opt_out, sync_skills + from tools.skills_sync import sync_skills + from tools.skills_sync_bundled_ops import set_bundled_skills_opt_out set_bundled_skills_opt_out(opt_out) result = sync_skills(quiet=True) if on_success is not None: diff --git a/run_agent.py b/run_agent.py index 3acea768bb..e8a803a514 100644 --- a/run_agent.py +++ b/run_agent.py @@ -955,7 +955,7 @@ class AIAgent( process_registry.kill_all(task_id=task_id) def release_computer_use() -> None: - from tools.computer_use import release_computer_use_session + from tools.computer_use.tool import release_computer_use_session release_computer_use_session(task_id) for step in (kill_processes, lambda: cleanup_vm(task_id), lambda: cleanup_browser(task_id), release_computer_use): diff --git a/tests/agent/test_phantom_tool_references.py b/tests/agent/test_phantom_tool_references.py index 836522827a..599f42c2b9 100644 --- a/tests/agent/test_phantom_tool_references.py +++ b/tests/agent/test_phantom_tool_references.py @@ -109,7 +109,7 @@ class TestEssentialSkillsUndisableable: assert cfg["skills"]["disabled"] == ["other"] def test_skill_manage_delete_refused(self): - from tools.skill_manager_tool import _pinned_guard + from tools.skill_manager_guards import _pinned_guard msg = _pinned_guard("hermes-agent") assert msg is not None assert "essential" in msg.lower() diff --git a/tests/hermes_cli/test_setup_blank_slate.py b/tests/hermes_cli/test_setup_blank_slate.py index e08d67c8e3..a3c909fc6e 100644 --- a/tests/hermes_cli/test_setup_blank_slate.py +++ b/tests/hermes_cli/test_setup_blank_slate.py @@ -116,7 +116,7 @@ class TestBlankSlateFork: monkeypatch.setattr(s, "_blank_slate_walkthrough", lambda cfg, home: walked.__setitem__("called", True)) opted_out = {"value": None} - monkeypatch.setattr("tools.skills_sync.set_bundled_skills_opt_out", + monkeypatch.setattr("tools.skills_sync_bundled_ops.set_bundled_skills_opt_out", lambda enabled: opted_out.__setitem__("value", enabled)) cfg = {} diff --git a/tests/run_agent/test_background_review_toolset_restriction.py b/tests/run_agent/test_background_review_toolset_restriction.py index 07e8e37aff..3175aa394c 100644 --- a/tests/run_agent/test_background_review_toolset_restriction.py +++ b/tests/run_agent/test_background_review_toolset_restriction.py @@ -155,7 +155,7 @@ def test_read_file_registers_background_review_read_mark(tmp_path): in this review turn" on the follow-up skill_manage patch (#61521). """ from tools.file_tools import read_file_tool - from tools.skill_manager_tool import ( + from tools.skill_manager_guards import ( _background_review_has_read, _reset_background_review_read_marks, ) @@ -190,7 +190,7 @@ def test_read_file_registers_background_review_read_mark(tmp_path): def test_read_file_outside_review_does_not_mark(tmp_path): """Foreground reads must not populate the review-fork read set.""" from tools.file_tools import read_file_tool - from tools.skill_manager_tool import ( + from tools.skill_manager_guards import ( _background_review_has_read, _reset_background_review_read_marks, ) diff --git a/tests/tools/test_skill_ledger.py b/tests/tools/test_skill_ledger.py index 7243c480ad..408b0c7b67 100644 --- a/tests/tools/test_skill_ledger.py +++ b/tests/tools/test_skill_ledger.py @@ -63,7 +63,7 @@ def test_background_review_patch_ledgers_and_rolls_back(ledger_env, monkeypatch) reset_current_write_origin, set_current_write_origin, ) - from tools.skill_manager_tool import mark_background_review_skill_read + from tools.skill_manager_guards import mark_background_review_skill_read token = set_current_write_origin(BACKGROUND_REVIEW) try: diff --git a/tests/tools/test_skill_manager_tool.py b/tests/tools/test_skill_manager_tool.py index 1d5bcb4629..bf4f4a03b8 100644 --- a/tests/tools/test_skill_manager_tool.py +++ b/tests/tools/test_skill_manager_tool.py @@ -835,7 +835,7 @@ class TestBackgroundOwnershipPolicyConsistency: @staticmethod def _bg_patch(tmp_path, name, old, new): - from tools.skill_manager_tool import mark_background_review_skill_read + from tools.skill_manager_guards import mark_background_review_skill_read from tools.skill_provenance import ( BACKGROUND_REVIEW, reset_current_write_origin, @@ -1108,7 +1108,7 @@ class TestCuratorConsolidationDeleteGuard: ): """A view in one tool worker authorizes a patch in the next worker.""" from tools.skills_tool import skill_view - from tools.skill_manager_tool import _reset_background_review_read_marks + from tools.skill_manager_guards import _reset_background_review_read_marks _reset_background_review_read_marks() with _curator_pass(tmp_path, monkeypatch=monkeypatch): @@ -1133,7 +1133,7 @@ class TestCuratorConsolidationDeleteGuard: ): """Copied tool contexts share only their own review's read marks.""" from tools.skills_tool import skill_view - from tools.skill_manager_tool import _reset_background_review_read_marks + from tools.skill_manager_guards import _reset_background_review_read_marks _reset_background_review_read_marks() with _curator_pass(tmp_path, monkeypatch=monkeypatch): @@ -1161,7 +1161,7 @@ class TestCuratorConsolidationDeleteGuard: def test_background_review_support_file_overwrite_requires_that_file_read(self, tmp_path, monkeypatch): from tools.skills_tool import skill_view - from tools.skill_manager_tool import _reset_background_review_read_marks + from tools.skill_manager_guards import _reset_background_review_read_marks _reset_background_review_read_marks() with _curator_pass(tmp_path, monkeypatch=monkeypatch): diff --git a/tests/tools/test_skills_list_modified_diff.py b/tests/tools/test_skills_list_modified_diff.py index 9df68656d3..2d382232f6 100644 --- a/tests/tools/test_skills_list_modified_diff.py +++ b/tests/tools/test_skills_list_modified_diff.py @@ -16,8 +16,8 @@ clears the modified state so the two stay consistent. from contextlib import ExitStack from unittest.mock import patch -from tools.skills_sync import ( - sync_skills, +from tools.skills_sync import sync_skills +from tools.skills_sync_bundled_ops import ( reset_bundled_skill, list_user_modified_bundled_skills, diff_bundled_skill, diff --git a/tests/tools/test_skills_sync.py b/tests/tools/test_skills_sync.py index 5a79faf5a4..b532881aac 100644 --- a/tests/tools/test_skills_sync.py +++ b/tests/tools/test_skills_sync.py @@ -17,9 +17,9 @@ from tools.skills_sync import ( _compute_relative_dest, _dir_hash, sync_skills, - reset_bundled_skill, - restore_official_optional_skill, ) +from tools.skills_sync_bundled_ops import reset_bundled_skill +from tools.skills_sync_optional import restore_official_optional_skill class TestReadWriteManifest: @@ -765,7 +765,7 @@ class TestOptOutToggleAndRemove: return bundled def test_marker_toggle(self, tmp_path): - from tools.skills_sync import set_bundled_skills_opt_out + from tools.skills_sync_bundled_ops import set_bundled_skills_opt_out home = tmp_path / "home" home.mkdir() marker = home / ".no-bundled-skills" @@ -783,9 +783,8 @@ class TestOptOutToggleAndRemove: assert not marker.exists() def test_remove_keeps_user_modified(self, tmp_path): - from tools.skills_sync import ( - sync_skills, remove_pristine_bundled_skills, - ) + from tools.skills_sync import sync_skills + from tools.skills_sync_bundled_ops import remove_pristine_bundled_skills bundled = self._setup_bundled(tmp_path) skills_dir = tmp_path / "user_skills" manifest_file = skills_dir / ".bundled_manifest" diff --git a/tests/tools/test_zombie_process_cleanup.py b/tests/tools/test_zombie_process_cleanup.py index 141f81c96e..14c5f070ce 100644 --- a/tests/tools/test_zombie_process_cleanup.py +++ b/tests/tools/test_zombie_process_cleanup.py @@ -110,7 +110,7 @@ class TestAgentCloseMethod: with patch("tools.process_registry.process_registry") as mock_registry, \ patch("run_agent.cleanup_vm") as mock_cleanup_vm, \ patch("run_agent.cleanup_browser") as mock_cleanup_browser, \ - patch("tools.computer_use.release_computer_use_session") as mock_cleanup_cua: + patch("tools.computer_use.tool.release_computer_use_session") as mock_cleanup_cua: agent.close() mock_registry.kill_all.assert_called_once_with( @@ -152,7 +152,7 @@ class TestAgentCloseMethod: "tools.process_registry.process_registry.kill_all", side_effect=RuntimeError("process cleanup failed"), ), patch( - "tools.computer_use.release_computer_use_session", + "tools.computer_use.tool.release_computer_use_session", ) as mock_cleanup_cua: agent.close() @@ -173,7 +173,7 @@ class TestAgentCloseMethod: agent.client = None with patch( - "tools.computer_use.release_computer_use_session", + "tools.computer_use.tool.release_computer_use_session", ) as mock_cleanup_cua: agent.release_clients() diff --git a/tools/approval.py b/tools/approval.py index 2755ebf708..c6bb88bc5e 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -237,7 +237,7 @@ def _release_permission_mode_dependents(session_key: str) -> None: sessions never load computer-use; releasing on BOTH edges makes enabling YOLO replace a standard backend and disabling it revoke a private unrestricted daemon immediately.""" try: - from tools.computer_use import release_computer_use_session + from tools.computer_use.tool import release_computer_use_session release_computer_use_session(session_key) except Exception: diff --git a/tools/computer_use/__init__.py b/tools/computer_use/__init__.py index 4de6c2bb6a..44b5ec7bc0 100644 --- a/tools/computer_use/__init__.py +++ b/tools/computer_use/__init__.py @@ -10,13 +10,3 @@ Modules: `tool.py` (handler, approval gate, response shaping), `backend.py` (abs `ComputerUseBackend` + result dataclasses), `cua_backend.py` (default MCP-over-stdio backend + `cua_backend_parse`/`_session`/`_daemon` siblings), `schema.py` (byte-frozen). """ - -from __future__ import annotations - -from tools.computer_use.tool import ( # noqa: F401 (public re-exports) - handle_computer_use, - release_computer_use_session, - set_approval_callback, - check_computer_use_requirements, - get_computer_use_schema, -) diff --git a/tools/file_tools.py b/tools/file_tools.py index 4fa930c5b8..c2f2624d3b 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -529,7 +529,7 @@ def _record_successful_read(task_data: dict, task_id: str, path: str, resolved_s # file is accepted. A partial read doesn't count — the guard requires the CURRENT full content # to have been seen. No-op outside review forks (mark_background_review_skill_read gates on # is_background_review). - from tools.skill_manager_tool import mark_background_review_skill_read + from tools.skill_manager_guards import mark_background_review_skill_read mark_background_review_skill_read(Path(resolved_str)) except Exception: logger.debug("background-review read-mark failed", exc_info=True) diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 35e1cb74c6..801025e198 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -19,8 +19,8 @@ from agent.redact import redact_sensitive_text from hermes_cli.goals import judge_goal from tools.registry import registry, tool_error from hermes_cli.config import cfg_get, load_config -from tools.kanban_tools_schemas import ( # noqa: F401 - re-exported for callers/tests - _DESC_BOARD, _DESC_TASK_ID_DEFAULT, _board_schema_prop, KANBAN_ATTACH_SCHEMA, +from tools.kanban_tools_schemas import ( + KANBAN_ATTACH_SCHEMA, KANBAN_ATTACH_URL_SCHEMA, KANBAN_ATTACHMENTS_SCHEMA, KANBAN_BLOCK_SCHEMA, KANBAN_COMMENT_SCHEMA, KANBAN_COMPLETE_SCHEMA, KANBAN_CREATE_SCHEMA, KANBAN_HEARTBEAT_SCHEMA, KANBAN_LINK_SCHEMA, KANBAN_LIST_SCHEMA, KANBAN_REQUEST_CHANGES_SCHEMA, KANBAN_REQUEST_REVIEW_SCHEMA, diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index e8e931cf63..3463f4c330 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -27,15 +27,11 @@ from agent.skill_utils import ( is_skill_description_truncated_for_prompt, parse_frontmatter as _parse_frontmatter, SKILL_PROMPT_DESC_LIMIT) -from tools.skill_manager_guards import ( # noqa: F401 — re-exported for callers/tests - _BackgroundReviewReadMarks, _background_review_has_read, _background_review_preflight, - _background_review_read_before_write_guard, _background_review_read_paths, - _background_review_write_guard, _containing_skills_root, _curator_consolidation_delete_guard, - _is_path_redirect, _maybe_auto_propose_org_edit, _org_mirror_write_guard, _pinned_guard, - _reset_background_review_read_marks, _validate_delete_target, _is_background_review, - mark_background_review_skill_read, _refusal as _err) -from tools.skill_manager_batch import ( # noqa: F401 - _BATCH_MAX_OPS, _BATCH_OP_ACTIONS, _skill_manage_batch) +from tools.skill_manager_guards import ( + _background_review_preflight, _background_review_read_before_write_guard, _background_review_write_guard, + _containing_skills_root, _curator_consolidation_delete_guard, _maybe_auto_propose_org_edit, + _org_mirror_write_guard, _pinned_guard, _validate_delete_target, _is_background_review, _refusal as _err) +from tools.skill_manager_batch import _skill_manage_batch from tools.skills_guard import scan_skill, should_allow_install, format_scan_report logger = logging.getLogger(__name__) diff --git a/tools/skills_sync.py b/tools/skills_sync.py index cb1f2b21af..ec6902a0e5 100644 --- a/tools/skills_sync.py +++ b/tools/skills_sync.py @@ -24,7 +24,9 @@ for _stream in (sys.stdout, sys.stderr): _stream.reconfigure(encoding="utf-8", errors="replace") from hermes_constants import get_bundled_skills_dir, get_hermes_home, get_optional_skills_dir from agent.skill_utils import ESSENTIAL_SKILLS, is_excluded_skill_path -from tools.skill_usage import _read_skill_name, read_suppressed_names # noqa: F401 (re-exported) +from tools.skill_usage import _read_skill_name, read_suppressed_names +from tools.skills_sync_bundled_ops import _is_tracked_user_modification +from tools.skills_sync_optional import _backfill_optional_provenance, _read_hub_install_paths from utils import atomic_write_text logger = logging.getLogger(__name__) @@ -419,16 +421,6 @@ def _rmtree_writable(path: Path) -> None: shutil.rmtree(path, onerror=_on_error) -# Re-exported so ``from tools.skills_sync import X`` / ``patch("tools.skills_sync.X")`` keep working. -from tools.skills_sync_bundled_ops import ( # noqa: E402,F401 - _is_tracked_user_modification, _read_for_diff, diff_bundled_skill, list_user_modified_bundled_skills, - remove_pristine_bundled_skills, reset_bundled_skill, set_bundled_skills_opt_out) -from tools.skills_sync_optional import ( # noqa: E402,F401 - _backfill_optional_provenance, _content_hash, _index_installed_skill_dirs_by_name, _move_to_restore_backup, - _optional_skill_index, _read_hub_install_paths, _safe_rel_install_path, _skill_file_list, - restore_official_optional_skill) - - if __name__ == "__main__": print("Syncing bundled skills into ~/.hermes/skills/ ...") result = sync_skills(quiet=False) diff --git a/tools/skills_sync_optional.py b/tools/skills_sync_optional.py index 01c713ee99..20bcf96936 100644 --- a/tools/skills_sync_optional.py +++ b/tools/skills_sync_optional.py @@ -15,7 +15,7 @@ logger = logging.getLogger("tools.skills_sync") def _ss(): - """Live ``tools.skills_sync`` module (imported lazily: it re-exports this module).""" + """Live ``tools.skills_sync`` module (imported lazily: it imports helpers from this module).""" from tools import skills_sync return skills_sync diff --git a/tools/skills_tool_plugin.py b/tools/skills_tool_plugin.py index a8b31b64bb..043c3b0cfb 100644 --- a/tools/skills_tool_plugin.py +++ b/tools/skills_tool_plugin.py @@ -104,7 +104,7 @@ def _serve_skill_file( def _mark_background_review_read(path: Path) -> None: try: - from tools.skill_manager_tool import mark_background_review_skill_read + from tools.skill_manager_guards import mark_background_review_skill_read mark_background_review_skill_read(path) except Exception: logger.debug("Could not record background-review skill read for %s", path, exc_info=True)