fix(doctor): skip tool checks when tools block is empty (#5301)

* fix(doctor): skip tool checks when tools block is empty

Follow-up to #5296, which fixed this for `models:`. The same defect
remains for `tools:`: `.get("tools", [])` returns None when the key is
present but empty, because the default only applies when the key is
absent. Iterating that None raises TypeError, which the surrounding
broad handler renders as a check result:

    ! web search configured  ('NoneType' object is not iterable)
    ! web fetch configured  ('NoneType' object is not iterable)
    ! web capture configured  ('NoneType' object is not iterable)
    ! image search configured  ('NoneType' object is not iterable)
    ✗ sandbox configured  ('NoneType' object is not iterable)

Line 476 is reached by all four web/image checks through the shared
check_web_tool helper, and line 645 by check_sandbox.

Unlike the models case, a default install does not hit this: `make
config` ships ten real tool entries, so a user has to empty or comment
out that block first.

The web checks now fall through to their normal "no tool in config"
warning and the sandbox check evaluates normally. Parentheses on the
comprehension are for readability; `or` already binds correctly there.

Regression tests use the commented-out `tools:` shape that reproduces
the failure, matching the tests added in #5296.

Fixes #5300

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(doctor): skip non-mapping tool entries, tighten regression tests

Review follow-ups on the line this PR already changes.

A `tools:` list holding a scalar (`tools:\n  - web_search`) reached
`t.get("name")` and raised AttributeError, which the broad handler
rendered as the check result:

    ! web search configured  ('str' object has no attribute 'get')

That is the same leakage this PR removes for the null case, so it is
fixed here rather than deferred. `check_sandbox` already guards the same
way via `isinstance(tool, dict)`.

The empty-tools test asserted that "NoneType" was absent from the
detail, which pins the failure mode rather than the behaviour — it would
still pass if the detail became some other internal error text. Both
tests now assert the expected message directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(doctor): assert sandbox outcome instead of the failure mode

Review follow-up, same class as the web-tool assertion fixed earlier in
this PR. The sandbox regression test still asserted that "NoneType" was
absent from the detail, which pins the failure mode rather than the
outcome — it would keep passing if some other internal error text leaked
out of the broad handler.

On this config the path is deterministic: an empty `tools:` means no
bash tool, so exactly one result. Assert the fields directly
(`CheckResult` has no `__eq__`, so whole instances cannot be compared by
value).

Verified against `main`'s scripts/doctor.py, where the same config
yields status=fail and detail="'NoneType' object is not iterable", so
the new assertions are red there and green here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

backend/tests/test_doctor.py

@@ -275,6 +275,24 @@ class TestCheckWebSearch:
         assert result.status == "ok"
         assert "DuckDuckGo" in result.detail
 
+    def test_commented_out_tools_block_warns_without_traceback(self, tmp_path):
+        # config.example.yaml ships a `tools:` key whose entries can all be
+        # commented out, so it parses as None rather than an empty list.
+        cfg = tmp_path / "config.yaml"
+        cfg.write_text("config_version: 5\ntools:\n  # - name: web_search\n")
+        result = doctor.check_web_search(cfg)
+        assert result.status == "warn"
+        assert result.detail == "no web_search tool in config"
+
+    def test_scalar_tools_entry_warns_without_traceback(self, tmp_path):
+        # A bare string entry is not a mapping; `t.get("name")` used to raise
+        # AttributeError, which the broad handler rendered as the check result.
+        cfg = tmp_path / "config.yaml"
+        cfg.write_text("config_version: 5\ntools:\n  - web_search\n")
+        result = doctor.check_web_search(cfg)
+        assert result.status == "warn"
+        assert result.detail == "no web_search tool in config"
+
     def test_tavily_with_key_ok(self, tmp_path, monkeypatch):
         monkeypatch.setenv("TAVILY_API_KEY", "tvly-test")
         cfg = tmp_path / "config.yaml"
@@ -659,6 +677,17 @@ class TestCheckSandbox:
         results = doctor.check_sandbox(cfg)
         assert results[0].status == "fail"
 
+    def test_commented_out_tools_block_reports_no_traceback(self, tmp_path):
+        # Regression: iterating a null `tools:` raised TypeError, which the
+        # broad handler rendered as "('NoneType' object is not iterable)".
+        cfg = tmp_path / "config.yaml"
+        cfg.write_text("config_version: 5\nsandbox:\n  use: deerflow.sandbox.local:LocalSandboxProvider\ntools:\n  # - name: bash\n")
+        results = doctor.check_sandbox(cfg)
+        # Empty `tools:` means no bash tool, so the path is deterministic.
+        assert len(results) == 1
+        assert results[0].status == "ok"
+        assert results[0].detail == "Local sandbox"
+
     def test_local_sandbox_with_disabled_host_bash_warns(self, tmp_path):
         cfg = tmp_path / "config.yaml"
         cfg.write_text("config_version: 5\nsandbox:\n  use: deerflow.sandbox.local:LocalSandboxProvider\n  allow_host_bash: false\ntools:\n  - name: bash\n    use: deerflow.sandbox.tools:bash_tool\n")

scripts/doctor.py

@@ -473,7 +473,7 @@ def check_web_tool(config_path: Path, *, tool_name: str, label: str) -> CheckRes
 
         data = _load_yaml_file(config_path)
 
-        tool_entries = [t for t in data.get("tools", []) if t.get("name") == tool_name]
+        tool_entries = [t for t in (data.get("tools") or []) if isinstance(t, dict) and t.get("name") == tool_name]
         if not tool_entries:
             return CheckResult(
                 label,
@@ -642,7 +642,7 @@ def check_sandbox(config_path: Path) -> list[CheckResult]:
             ]
 
         sandbox_use = sandbox.get("use", "")
-        tools = data.get("tools", [])
+        tools = data.get("tools") or []
         tool_names = {tool.get("name") for tool in tools if isinstance(tool, dict)}
         results: list[CheckResult] = []