fix: http_probe HEAD uses curl no-body mode; analyze_pdf follows its dependency - #240
Merged
Merged
Conversation
…s dependency
Two pre-existing defects found in Odin's v3.65.0 smoke test. Neither was
introduced by the workspace change; both were designed with him first.
1. http_probe HEAD hung for 5s and reported a spurious error. The builder
appended `-X {method}` for any non-GET, so HEAD became `curl -X HEAD` —
which sends the HEAD token but leaves libcurl expecting a response body.
Measured against a healthy server: 5.1s / exit 18 ("transfer closed with
741 bytes remaining") versus 0.065s / exit 0 for `-I`.
HEAD now takes curl's native no-body path: `-I`, no `-X` override, and the
unconditional `-i` suppressed because `-I` already routes headers to output
and combining them makes the output contract depend on curl's version. `-L`
is kept — `curl -I -L` performs HEAD across the chain, matching the
requested method and the existing follow-redirects contract; the final
result comes from the `-w` values, which parse from the existing
`---PROBE-RESULTS---` sentinel rather than from body termination.
HEAD is the ONLY affected method: POST/PUT/PATCH/DELETE/OPTIONS may return
zero-length bodies and curl frames those normally (verified, `-X OPTIONS`
exits 0 in 0.067s). Live-verified all four shapes: plain HEAD 0.07s/1 header
block, redirecting HEAD 0.10s/2 blocks with final 200, no-follow HEAD 301,
GET unchanged.
CONTRACT CHANGE: a nonempty body on HEAD is now REJECTED rather than
silently dropped (Odin's call). Data flags plus `-I` make curl's method
selection ambiguous, and silently discarding hid a caller mistake. An empty
body is still accepted. The pre-existing test asserting the drop is updated.
2. analyze_pdf was advertised on every install while its dependency was
installed on none. PyMuPDF lives in the optional `pdf` extra, and
postinstall, incus-deploy and Dockerfile all ran a bare `pip install .` —
so every call died with "No module named 'fitz'".
Both halves, per Odin: the catalog now hides analyze_pdf when the module is
not importable (joining claude_code, the email tools, issue_tracker and
generate_image in tool_catalog's existing backend-gating), AND all three
packaged paths install `.[pdf]` so an official install genuinely provides
what it advertises and a rebuild does not undo a manual repair. PyMuPDF is
deliberately NOT promoted to a base dependency — it is heavy and native, and
`pip install .` stays minimal with `.[pdf]` as the source opt-in.
find_spec is treated as structural availability only: the handler now
converts any import/load failure into a clean "PDF support unavailable"
message naming the remedy, because find_spec proves importability, not that
the native library loads. Startup logs why the tool was hidden.
The gating-arithmetic contract test computed its expectation from a fixed
constant. Because this gate depends on the ENVIRONMENT rather than Config,
that would have passed on a machine with the extra and failed on one
without — the local-green/CI-red trap. It now derives the expectation from
actual availability and asserts visibility tracks the dependency either way.
Mutation-verified: restoring `-X HEAD`, dropping the HEAD-body rejection,
removing the catalog gate, or restoring the raw `import fitz` each fails its
test. Suite run as a normal user AND as root: 7647/7644 passed. Gates: lint 0,
types 0, coverage findings 0, diff --check clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two pre-existing defects found during the v3.65.0 smoke test. Neither was introduced by the workspace change, and both were design-discussed with Odin before any code.
1.
http_probeHEAD hung for 5s and reported a spurious errorbuild_http_probe_commandappended-X {method}for any non-GET, so HEAD becamecurl -X HEAD— which sends the HEAD token but leaves libcurl expecting a response body.Measured against a healthy server:
HEAD now takes curl's native no-body path:
-I, and no-Xoverride-isuppressed for HEAD —-Ialready routes headers to output, and combining them makes the output contract depend on the curl version-Lkept:curl -I -Lperforms HEAD across the chain, matching the requested method and the existing follow-redirects contract. The final result comes from the-wvalues, which parse from the existing---PROBE-RESULTS---sentinel rather than from body termination.HEAD is the only affected method. POST/PUT/PATCH/DELETE/OPTIONS may return zero-length bodies and curl frames those normally — verified,
-X OPTIONSexits 0 in 0.067s.Live-verified all four shapes against odin-bot.net:
-L)follow_redirects=falseContract change
A nonempty body on HEAD is now rejected rather than silently dropped. Data flags plus
-Imake curl's method selection ambiguous, and silently discarding hid a caller mistake. An empty body is still accepted. The pre-existing test that asserted the silent drop is updated to the new contract.Also note: the previous
test_headasserted"-X HEAD" in cmd— it pinned the bug, so it is replaced rather than extended.2.
analyze_pdfwas advertised everywhere and installed nowherePyMuPDF lives in the optional
pdfextra, andpackaging/postinstall.sh,scripts/incus-deploy.shandDockerfileall ran a barepip install .. So the tool was advertised on every install path while its dependency was present on none of them, and calls died withNo module named 'fitz'.Both halves, per Odin's design call:
analyze_pdfwhen the module is not importable, joiningclaude_code, the email tools,issue_trackerandgenerate_imageintool_catalog's existing backend-gating. Startup logs why it was hidden..[pdf], so an official install genuinely provides what it advertises and a rebuild does not undo a manual repair.PyMuPDF is deliberately not promoted to a base dependency: it is heavy and native with its own platform and security-update burden.
pip install .stays minimal;.[pdf]is the source opt-in.find_specis treated as structural availability only — it proves importability, not that the native library loads — so the handler converts any import/load failure into a cleanPDF support unavailablemessage naming the remedy.A trap worth calling out
TestBackendGatedVisibilityasserts exact arithmetic:len(catalog) == len(EXPECTED_TOOL_ORDER) - len(GATED). The seven existing gated tools are config-gated, so that constant is deterministic. This gate is environment-gated, andfitzis absent from the dev venv but present on the live install — so a fixed constant would have passed on one machine and failed on the other. That is the local-green/CI-red failure mode from PR #239. The test now derives its expectation from actual availability and separately asserts that visibility tracks the dependency in either direction.Verification
-X HEAD, dropping the HEAD-body rejection, removing the catalog gate, or restoring the rawimport fitzeach fails its test.git diff --checkclean.🤖 Generated with Claude Code
https://claude.ai/code/session_011BDkRoJv6pTMTTYrh1SPzy