Refactor tool availability checks into registration

[0xallam]

Summary

• move tool availability decisions from strix.tools.__init__ into register_tool(...)
• gate browser and web-search tools at registration time using mode-aware flags
• keep using while ensuring direct imports do not back-register agent-graph tools
• add focused tests covering sandbox vs non-sandbox registration behavior

[0xallam]

@greptileai

[greptile-apps]

Greptile Summary

This PR refactors tool availability gating from conditional block-imports in strix/tools/__init__.py into the register_tool decorator in registry.py, adding requires_browser_mode and requires_web_search_mode flags. The approach is architecturally cleaner — all submodules are now unconditionally imported and each tool self-describes its own availability requirements. The core logic in _should_register_tool is correct and consistent with the old conditional-import behavior.

Key observations:

• sandbox_mode = _is_sandbox_mode() is computed at the top of decorator but _should_register_tool (called immediately after) also calls _is_sandbox_mode() internally — causing a redundant env-var read on every decorated function before the early-exit guard.
• test_load_skill_does_not_register_agents_graph_when_imported_directly (line 61) evicts all strix.tools.* from sys.modules before the "direct import", which forces strix/tools/__init__.py to re-execute and unconditionally import every submodule. The test therefore does not validate direct-import isolation — create_agent is absent solely due to sandbox-mode filtering, and the assert "python_action" in names_before assertion would fail if strix.tools were already cached. The test name and intent are misleading.
• No test covers the case where browser is enabled in sandbox mode (i.e., browser_action should be registered).

Confidence Score: 3/5

• Functionally correct refactor, but the third test has a logic flaw that makes it unreliable as a regression guard for the claimed isolation behavior.
• The production logic (_should_register_tool, requires_browser_mode, requires_web_search_mode) is sound and consistent with the previous conditional-import behavior. The two smaller issues (redundant env-var call, misleading test) don't affect runtime correctness, but the test bug means the "direct import isolation" guarantee is not actually verified — a future refactor could silently break it.
• tests/tools/test_tool_registration_modes.py — the third test validates sandbox filtering rather than import isolation as claimed; strix/tools/registry.py — minor redundant _is_sandbox_mode() call worth cleaning up.

Important Files Changed

Filename	Overview
strix/tools/registry.py	Adds _should_register_tool gate and new flags (requires_browser_mode, requires_web_search_mode) to register_tool. Logic is correct, but sandbox_mode is computed redundantly before the early-exit check.
strix/tools/init.py	Replaces conditional block-imports with unconditional wildcard imports for all tool submodules, delegating availability decisions entirely to register_tool. Behaviorally correct; all module-level code now runs in all modes.
strix/tools/browser/browser_actions.py	Adds requires_browser_mode=True to @register_tool decorator; straightforward and correct.
strix/tools/web_search/web_search_actions.py	Adds requires_web_search_mode=True to @register_tool decorator; correctly gates Perplexity API key check at registration time.
tests/tools/test_tool_registration_modes.py	New test file covering sandbox vs. non-sandbox registration. The third test has a logic flaw: it evicts strix.tools from sys.modules, causing init.py to re-execute and defeating the "direct import isolation" claim; the assertion on python_action would fail if strix.tools were already cached.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: strix/tools/registry.py
Line: 197-204

Comment:
**Redundant `_is_sandbox_mode()` call**

`sandbox_mode = _is_sandbox_mode()` is evaluated at line 198, but `_should_register_tool` (called immediately after on line 199) also invokes `_is_sandbox_mode()` internally. This means `os.getenv` is read twice on every decorated function — once wastefully before the early-exit check.

The cleaner fix is to move `sandbox_mode = _is_sandbox_mode()` to *after* the `_should_register_tool` guard, or pass it into `_should_register_tool` as a parameter and return it alongside the bool result:

```python
def decorator(f: Callable[..., Any]) -> Callable[..., Any]:
    if not _should_register_tool(
        sandbox_execution=sandbox_execution,
        requires_browser_mode=requires_browser_mode,
        requires_web_search_mode=requires_web_search_mode,
    ):
        return f

    sandbox_mode = _is_sandbox_mode()  # only computed when we'll actually use it
    ...
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: tests/tools/test_tool_registration_modes.py
Line: 61-80

Comment:
**Test doesn't actually test "direct import" isolation**

The test is named `test_load_skill_does_not_register_agents_graph_when_imported_directly`, and the PR description states "direct imports do not back-register agent-graph tools." However, the test first evicts **all** `strix.tools.*` entries from `sys.modules` before importing `strix.tools.load_skill.load_skill_actions`. Because `strix.tools` itself was evicted, Python must re-execute `strix/tools/__init__.py` as part of resolving the parent package — which unconditionally imports *all* submodules including `from .agents_graph import *` and `from .python import *`.

The `assert "python_action" in names_before` at line 78 only holds because of this full `__init__.py` re-execution, not because `load_skill_actions` itself registers it. Similarly, `create_agent` is absent from the registry solely due to sandbox-mode filtering (`sandbox_execution=False`), not because of import isolation.

If `strix.tools` were already present in `sys.modules` (which is the "direct import" scenario the test claims to cover), importing `strix.tools.load_skill.load_skill_actions` would **not** re-run `__init__.py`, meaning `python_action` would be absent from `names_before`. The test would then fail on line 78.

To genuinely test direct-import isolation, either (a) avoid evicting `strix.tools` from `sys.modules`, or (b) remove the misleading assertion on `python_action` and update the test name/comment to accurately describe what is being tested (sandbox-mode filtering).

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "refactor: move tool ..."}