fix: http_probe HEAD uses curl no-body mode; analyze_pdf follows its dependency

Dockerfile

@@ -4,7 +4,9 @@ WORKDIR /app
 
 # Install dependencies first for better layer caching
 COPY pyproject.toml .
-RUN pip install --no-cache-dir .
+# .[pdf] — analyze_pdf needs PyMuPDF; without it the catalog gate hides the
+# tool, so an official image would ship without a capability it advertises.
+RUN pip install --no-cache-dir ".[pdf]"
 
 # Copy application source
 COPY src/ src/

packaging/postinstall.sh

@@ -85,8 +85,11 @@ fi
 # Install Python dependencies from pyproject.toml
 echo "  Installing Python dependencies (this can take a few minutes)..."
 if [ -f "$APP_DIR/pyproject.toml" ]; then
-    "$APP_DIR/.venv/bin/pip" install --quiet "$APP_DIR" 2>/dev/null || \
-        echo "  Warning: pip install failed — run '$APP_DIR/.venv/bin/pip install $APP_DIR' manually"
+    # [pdf] installs PyMuPDF so the advertised analyze_pdf tool actually works.
+    # Without it the tool is hidden by the catalog gate, so an official package
+    # would silently ship without a capability it documents.
+    "$APP_DIR/.venv/bin/pip" install --quiet "$APP_DIR[pdf]" 2>/dev/null || \
+        echo "  Warning: pip install failed — run '$APP_DIR/.venv/bin/pip install $APP_DIR[pdf]' manually"
 fi
 
 # Install Playwright browsers for native browser support (optional feature)

scripts/incus-deploy.sh

@@ -107,7 +107,8 @@ done
 # Install Python dependencies
 echo "Installing Python dependencies..."
 incus exec "$INSTANCE" -- bash -c "
-    cd /app && pip install --no-cache-dir --break-system-packages . > /dev/null 2>&1
+    # .[pdf] — see packaging/postinstall.sh: analyze_pdf needs PyMuPDF.
+    cd /app && pip install --no-cache-dir --break-system-packages '.[pdf]' > /dev/null 2>&1
 "
 
 # Set ownership

src/discord/tool_catalog.py

@@ -12,10 +12,14 @@
 
 from __future__ import annotations
 
+import importlib.util
 from collections.abc import Callable
 
+from ..odin_log import get_logger
 from ..tools import get_tool_definitions
 
+log = get_logger("tools")
+
 
 class ToolCatalog:
     def __init__(self, *, get_config: Callable, skill_manager) -> None:
@@ -54,6 +58,20 @@ def merged_definitions(self) -> list[dict]:
 
         if not image_tool_available(config):
             builtin = [t for t in builtin if t["name"] != "generate_image"]
+        # analyze_pdf: PyMuPDF lives in the optional `pdf` extra, and no
+        # install path used to install extras — so the tool was advertised on
+        # every install while its dependency was present on none of them, and
+        # calls died with "No module named 'fitz'". Structural availability
+        # only; the handler still converts a load failure into a clean result,
+        # because find_spec proves the module is importable, not that the
+        # native library loads.
+        if importlib.util.find_spec("fitz") is None:
+            builtin = [t for t in builtin if t["name"] != "analyze_pdf"]
+            log.info(
+                "analyze_pdf hidden from the tool catalog: PyMuPDF is not "
+                "installed. Install the 'pdf' extra to enable it "
+                "(pip install '.[pdf]')."
+            )
         # Per-spawn agent model/effort catalogue: expose each axis's field +
         # clause on spawn_agent/spawn_loop_agents only when that agent config
         # axis is "auto" (operates on clones — never mutates the shared defs).

src/tools/handlers/files_docs.py

@@ -123,7 +123,18 @@ async def _handle_analyze_pdf(self, inp: dict) -> str:
             if is_url_blocked(url):
                 return "Error: blocked URL (localhost / private IP / cloud-metadata address)."
 
-        import fitz
+        # Structural gating hides this tool when PyMuPDF is missing, but the
+        # handler must still degrade cleanly: find_spec proves the module is
+        # importable, not that its native library loads, and a direct call can
+        # reach here on an install whose catalog was built elsewhere.
+        try:
+            import fitz
+        except Exception as exc:
+            return (
+                "PDF support unavailable: PyMuPDF could not be loaded "
+                f"({type(exc).__name__}: {exc}). Install the 'pdf' extra "
+                "(pip install '.[pdf]') and restart Odin."
+            )
 
         pdf_bytes: bytes | None = None
 

src/tools/http_probe_ops.py

@@ -108,17 +108,37 @@ def build_http_probe_command(params: dict) -> str:
             f"Invalid HTTP method: {method}. Allowed: {', '.join(sorted(ALLOWED_METHODS))}"
         )
 
+    # HEAD needs curl's native no-body mode, not a method override. `-X HEAD`
+    # sends the HEAD token but leaves libcurl expecting a response body, so it
+    # blocks until the timeout and exits 18 ("transfer closed with N bytes
+    # remaining"): measured 5.1s/exit-18 versus 0.065s/exit-0 for `-I` against
+    # a healthy server. HEAD is the ONLY affected method — POST/PUT/PATCH/
+    # DELETE/OPTIONS may legitimately return zero-length bodies and curl frames
+    # those normally (verified: `-X OPTIONS` exits 0 in 0.067s).
+    is_head = method == "HEAD"
+
+    # A request body on HEAD is rejected rather than silently dropped: data
+    # flags combined with -I make curl's method selection ambiguous, and HEAD
+    # request-body semantics are not worth preserving here.
+    if is_head and params.get("body"):
+        raise ValueError("HTTP method HEAD does not accept a request body")
+
     parts = ["curl", "-sS"]
 
     # Timing output format
     parts.append(f"-w {_sq(_TIMING_FORMAT)}")
 
-    # Include response headers in output
-    parts.append("-i")
-
-    # HTTP method
-    if method != "GET":
-        parts.append(f"-X {method}")
+    if is_head:
+        # -I already routes response headers to output; adding -i as well is
+        # redundant and makes the output contract depend on how a given curl
+        # version coalesces the two.
+        parts.append("-I")
+    else:
+        # Include response headers in output
+        parts.append("-i")
+        # HTTP method
+        if method != "GET":
+            parts.append(f"-X {method}")
 
     # Timeout
     timeout = _clamp_int(params.get("timeout"), DEFAULT_TIMEOUT, 1, MAX_TIMEOUT)

tests/characterization/test_tool_parity.py

@@ -182,6 +182,20 @@ class TestBackendGatedVisibility:
     GATED = {"claude_code", "email_send", "email_search", "email_read",
              "email_list_recent", "issue_tracker", "generate_image"}
 
+    @staticmethod
+    def _dependency_gated() -> set[str]:
+        """Tools gated by an installed DEPENDENCY rather than by config.
+
+        analyze_pdf needs PyMuPDF, which lives in the optional `pdf` extra, so
+        whether it is visible depends on the environment rather than the Config
+        under test. Computing this instead of hardcoding keeps the arithmetic
+        below true on a machine with the extra AND on one without — a fixed
+        constant would pass locally and fail in CI, or vice versa.
+        """
+        import importlib.util
+
+        return set() if importlib.util.find_spec("fitz") else {"analyze_pdf"}
+
     def _catalog_names(self, **config_kwargs) -> set[str]:
         from src.config.schema import Config
         from src.discord.client import OdinBot
@@ -196,8 +210,25 @@ def test_unconfigured_backends_are_invisible(self):
         names = self._catalog_names()
         leaked = self.GATED & names
         assert not leaked, f"backend-gated tools visible without config: {sorted(leaked)}"
-        # exact arithmetic: full registry minus the six gated tools
-        assert len(names) == len(EXPECTED_TOOL_ORDER) - len(self.GATED)
+        dependency_gated = self._dependency_gated()
+        assert not (dependency_gated & names), (
+            f"tools with a missing dependency are advertised: {sorted(dependency_gated & names)}"
+        )
+        # exact arithmetic: full registry minus config-gated minus
+        # dependency-gated tools
+        assert len(names) == len(EXPECTED_TOOL_ORDER) - len(self.GATED) - len(dependency_gated)
+
+    def test_analyze_pdf_follows_its_dependency(self):
+        """analyze_pdf must be advertised exactly when PyMuPDF can be imported.
+
+        It was previously advertised unconditionally while no install path
+        installed the `pdf` extra, so every call failed with
+        "No module named 'fitz'" (found in the v3.65.0 smoke test).
+        """
+        import importlib.util
+
+        visible = "analyze_pdf" in self._catalog_names()
+        assert visible is (importlib.util.find_spec("fitz") is not None)
 
     def test_configured_claude_code_is_visible(self):
         names = self._catalog_names(

tests/test_handlers_files_docs.py

@@ -224,3 +224,26 @@ async def test_truncation(self):
             out = await _tools(exec_ret=(0, b64))._handle_analyze_pdf(
                 {"host": "s", "path": "/p"})
             assert "truncated" in out
+
+
+async def test_analyze_pdf_degrades_cleanly_without_pymupdf(monkeypatch):
+    """find_spec proves the module is importable, not that its native library
+    loads — and a direct call can reach the handler on an install whose catalog
+    was built elsewhere. Either way the caller gets a clean, actionable result
+    rather than a raw ImportError (v3.65.0 smoke test: "No module named 'fitz'").
+    """
+    import builtins
+
+    real_import = builtins.__import__
+
+    def _no_fitz(name, *args, **kwargs):
+        if name == "fitz":
+            raise ImportError("No module named 'fitz'")
+        return real_import(name, *args, **kwargs)
+
+    monkeypatch.setattr(builtins, "__import__", _no_fitz)
+
+    tools = _tools()
+    result = await tools._handle_analyze_pdf({"host": "localhost", "path": "/tmp/x.pdf"})
+    assert "PDF support unavailable" in result
+    assert "pdf" in result and "install" in result.lower(), "must name the remedy"

tests/test_http_probe_ops.py

@@ -222,9 +222,85 @@ def test_patch(self):
         cmd = build_http_probe_command({"url": "https://example.com", "method": "PATCH"})
         assert "-X PATCH" in cmd
 
-    def test_head(self):
+    def test_head_uses_native_no_body_mode(self):
+        """HEAD must use curl's -I, never -X HEAD.
+
+        `-X HEAD` sends the HEAD token but leaves libcurl expecting a response
+        body, so it blocks until the timeout and exits 18 ("transfer closed
+        with N bytes remaining"). Measured against a healthy server: 5.1s and
+        exit 18 with -X HEAD, versus 0.065s and exit 0 with -I. This test
+        previously asserted "-X HEAD" in cmd — it pinned the bug.
+        """
         cmd = build_http_probe_command({"url": "https://example.com", "method": "HEAD"})
-        assert "-X HEAD" in cmd
+        assert "-I" in cmd.split(), cmd
+        assert "-X HEAD" not in cmd, "the -X override reintroduces the hang"
+
+    def test_head_suppresses_the_header_include_flag(self):
+        """-I already routes response headers to output; adding -i as well is
+        redundant and makes the output contract depend on how a given curl
+        version coalesces the two."""
+        cmd = build_http_probe_command({"url": "https://example.com", "method": "HEAD"})
+        assert "-i" not in cmd.split(), cmd
+
+    def test_non_head_methods_keep_the_include_flag_and_override(self):
+        """The fix must stay HEAD-specific: other methods may legitimately
+        return zero-length bodies and curl frames those normally (verified:
+        -X OPTIONS exits 0 in 0.067s)."""
+        for method in ("POST", "PUT", "PATCH", "DELETE", "OPTIONS"):
+            cmd = build_http_probe_command({"url": "https://example.com", "method": method})
+            assert f"-X {method}" in cmd, method
+            assert "-i" in cmd.split(), method
+            assert "-I" not in cmd.split(), method
+
+    def test_get_is_unchanged(self):
+        cmd = build_http_probe_command({"url": "https://example.com", "method": "GET"})
+        assert "-i" in cmd.split()
+        assert "-I" not in cmd.split()
+        assert "-X" not in cmd.split()
+
+    def test_head_keeps_follow_redirects(self):
+        """curl -I -L performs HEAD across the redirect chain, matching the
+        requested method and the existing follow-redirects contract; dropping
+        -L only for HEAD would make its behaviour inconsistent."""
+        cmd = build_http_probe_command({"url": "https://example.com", "method": "HEAD"})
+        assert "-L" in cmd.split()
+
+    def test_head_without_follow_redirects_omits_follow_flag(self):
+        cmd = build_http_probe_command({
+            "url": "https://example.com", "method": "HEAD", "follow_redirects": False,
+        })
+        assert "-L" not in cmd.split()
+        assert "-I" in cmd.split()
+
+    def test_head_keeps_the_timing_trailer_and_its_sentinel(self):
+        """The timing block is parsed from a distinct sentinel rather than from
+        body termination, so it survives HEAD's empty body."""
+        cmd = build_http_probe_command({"url": "https://example.com", "method": "HEAD"})
+        assert "---PROBE-RESULTS---" in cmd
+        assert "%{http_code}" in cmd
+        assert "%{time_total}" in cmd
+
+    def test_head_honours_the_timeout(self):
+        cmd = build_http_probe_command({
+            "url": "https://example.com", "method": "HEAD", "timeout": 7,
+        })
+        assert "--max-time 7" in cmd
+
+    def test_head_rejects_a_request_body(self):
+        """Rejected, not silently dropped: data flags combined with -I make
+        curl's method selection ambiguous, and HEAD request-body semantics are
+        not worth preserving."""
+        with pytest.raises(ValueError, match="HEAD"):
+            build_http_probe_command({
+                "url": "https://example.com", "method": "HEAD", "body": "x=1",
+            })
+
+    def test_head_with_empty_body_is_accepted(self):
+        """Only a NONEMPTY body is a conflict."""
+        cmd = build_http_probe_command({
+            "url": "https://example.com", "method": "HEAD", "body": "",
+        })
+        assert "-I" in cmd.split()
 
     def test_options(self):
         cmd = build_http_probe_command({"url": "https://example.com", "method": "OPTIONS"})
@@ -327,13 +403,21 @@ def test_delete_body_ignored(self):
         })
         assert "-d" not in cmd
 
-    def test_head_body_ignored(self):
-        cmd = build_http_probe_command({
-            "url": "https://example.com",
-            "method": "HEAD",
-            "body": "should not appear",
-        })
-        assert "-d" not in cmd
+    def test_head_body_rejected(self):
+        """CONTRACT CHANGE: a body on HEAD used to be silently dropped; it is
+        now rejected.
+
+        Silently discarding it hid a caller mistake, and data flags combined
+        with curl's -I make method selection ambiguous. Rejecting is the
+        deliberate choice (Odin's design call on this fix); the previous
+        assertion here was that the body simply never reached the command.
+        """
+        with pytest.raises(ValueError, match="HEAD"):
+            build_http_probe_command({
+                "url": "https://example.com",
+                "method": "HEAD",
+                "body": "should not appear",
+            })
 
     def test_empty_body_not_added(self):
         cmd = build_http_probe_command({