test: cover the request plumbing and decoding branches of the Python client

[dmccoystephenson]

Summary

The Python client's uncovered branches are covered by characterisation tests. No production code is changed, and no existing assertion is weakened.

Fifteen tests are added to python/tests/test_client.py, locking in behaviour that every public method inherits but that nothing asserted:

• Config normalisation — allowed_roots has its slashes and whitespace tidied, and a root that normalises to the empty string is dropped rather than kept. That last one is load-bearing: an empty root is a prefix of every path, so keeping one would silently open the whole repository. None and () are confirmed not to be collapsed into each other.
• Request plumbing — api_base (with and without a trailing slash), user_agent, and timeout are each confirmed to reach the outgoing request. The timeout case is asserted across every call a save makes, not just the first, because a request without a deadline hangs a request thread rather than failing visibly.
• Response decoding — an empty response body parses as {} rather than raising, which several GitHub endpoints require.
• Error surface — an error body that is not GitHub's JSON envelope (a gateway answering with HTML) falls back to GitHub API returned HTTP <code> and carries the status, and the gateway's own body is not quoted back into the message.
• Content decoding — get_file refuses a non-base64 envelope rather than decoding it into silent nonsense, and replaces undecodable bytes rather than raising.
• Path gate — the unmanaged-path refusal is confirmed to carry status=400 and to name the boundary, and to say "outside the repository" when no roots are configured. A path equal to a root is confirmed to be inside it.
• Listing — a trees entry with no size lists as 0 rather than failing the whole call.

FakeGitHub gains a timeouts list and a mutable tree, matching the mutable state it already exposes for branches and pull requests. The literal tree that was inlined in the handler is moved to __init__ unchanged.

Which half

python/ only. Nothing under js/ is touched.

Test plan

• PYTHONPATH=src python3 -m unittest discover -s tests -v — 46 tests, all passing (31 before this change). Executed locally on Python 3.8; the CI matrix runs the 3.9–3.13 floor-to-ceiling.
• Mutation-checked. Because these are characterisation tests rather than a bug fix, passing green proves nothing on its own. Nine targeted mutations were applied to python/src/github_docs/client.py one at a time, and each was detected by at least one of the new tests: the empty-body guard, the size default, the timeout kwarg, the api_base rstrip, the base64 encoding guard, the errors="replace" decode, the root-normalisation filter, the status=400 on the path refusal, and the HTTP-status fallback message. client.py was restored byte-for-byte afterwards; the diff contains one file.
• CI green on the head SHA — the js legs cannot be run locally (no node in this environment) and are left to CI. This change touches no file under js/.

Invariants

• dependencies = [] is untouched; no non-stdlib import is added.
• The non-credential-shaped TOKEN sentinel is unchanged.
• No production code is modified, so the redaction path, the result contract, and the export surface are all unchanged.

Issues

No tracking issue — this is a Stage B test-expansion cycle. A full documentation-accuracy sweep was run first across all seven sources of truth and found no drift, so that work mode was not selected.

The two open issues, #5 and #6, were both deferred this cycle for the same reason: neither can be resolved by a code change. #5 needs a publish decision plus an npm organisation secret, and #6 needs a pending publisher registered on PyPI and a decision about the distribution name — all operator actions on systems outside this repository. It is worth recording against #6 that its first listed prerequisite, a dry_run input on release-pypi.yml, has since been satisfied by #7.

---

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric

Scored adversarially against the diff and against command output, not against judgement.

Universal

• Scope: PASS — git diff --name-only origin/main...HEAD returns exactly one path, python/tests/test_client.py. No production file, manifest, workflow or README is touched. The only non-additive hunk is the FakeGitHub tree literal being moved from the handler body into __init__, unchanged line-for-line, which is what makes it settable per test.
• Tests-new: NOT APPLICABLE — no public method or function is added by this PR. Inverted for a test-expansion cycle and read as "does every new test exercise something real": nine mutations were applied to python/src/github_docs/client.py one at a time and each was caught, so every new test is load-bearing rather than tautological. The mutations were the empty-body guard, the size default, the timeout kwarg, the api_base rstrip, the base64 encoding guard, the errors="replace" decode, the root-normalisation filter, the status=400 on the path refusal, and the HTTP-status fallback message. client.py was byte-identical afterwards, confirmed by git status.
• Tests-fix: NO SIGNAL THIS CYCLE — nothing is fixed here. The stash-and-run experiment has no subject, and the mutation run above is the closest equivalent that applies: it establishes the FAIL half empirically rather than by reasoning.
• Sibling structure: PASS — no new file is created. The new cases were folded into the existing TestConfig, TestManagedPaths, TestListDocuments, TestGetFile and TestErrorSurface classes rather than parked in a new one, and the single new class, TestRequestPlumbing, follows the file's existing shape (a docstring or none, make_client(**overrides), mock.patch("urllib.request.urlopen", ...), prose test names).
• Sibling renames: PASS — nothing is renamed. _response and _http_error each gain an optional raw=None parameter, keeping every existing call site valid; both remain in parallel and both got the same parameter name and the same shape of guard.
• Docs: PASS — no behaviour changed, so no row in the sources-of-truth table needs to move. python/README.md's Development section still names the command these tests run under, and the counts it quotes are not version-pinned.
• Issue resolution: NOT APPLICABLE — no Closes #N is claimed. Deliberate: this is a Stage B cycle with no tracking issue, stated in the PR body.
• CI: PASS — all eight legs green on head a025eb0: js (18|20|22) and python (3.9|3.10|3.11|3.12|3.13), runs 34450010755 and 34450016365. The python (3.9) leg is the one that matters here, since it is the declared floor and the only leg that exercises the changed file under the oldest supported interpreter.

Repo-specific

• No-leak (js): NOT APPLICABLE — nothing under js/ is in the diff.
• No-leak (python): PASS — no GitHubDocsError is constructed anywhere in the diff; the new tests only observe messages the production code already builds. One of them, test_a_body_that_is_not_githubs_json_falls_back_to_the_status, asserts the message is exactly GitHub API returned HTTP 502, which pins that an unparseable upstream body is dropped rather than pasted into the exception.
• Result-not-exception: NOT APPLICABLE — js/src/client.ts is untouched.
• Stdlib-only: PASS — the import block at the top of the file is unchanged; no import is added by this diff, and python/pyproject.toml is not in it, so dependencies = [] stands.
• No runtime dependency (js): PASS — neither js/package.json nor js/package-lock.json appears in the diff.
• Export parity: NOT APPLICABLE — no symbol is added to js/src/*.ts.
• Support-matrix parity: NOT APPLICABLE — no supported version is changed. Worth recording that the suite was run locally on Python 3.8, which is below the declared 3.9 floor; that is not a support claim, and the floor is covered by the python (3.9) CI leg.
• Sentinel intact: PASS — TOKEN is not in the diff. Its value and the comment explaining why it is deliberately not credential-shaped are both untouched.

Two items that were scored FAIL first and then fixed

Both were mechanical, so both were fixed in a025eb0 rather than left as comments.

• python/tests/test_client.py:227 — test_an_empty_response_body_is_not_a_json_parse_failure had no assertion at all. It passed by not raising, which is a real property but leaves a reader unable to see what the call was meant to do. The urlopen mock is now captured and the outgoing ref is asserted.
• python/tests/test_client.py:511 — the non-JSON error fixture was built with message="Bad Gateway" only after this fix; before it, the default message="Not Found" was passed alongside a 502 HTML body, so the HTTPError object contradicted its own body. Nothing asserted on it, which is exactly why it would have survived to confuse the next reader.

Judgement calls left standing, with reasons

• test_a_root_itself_is_inside_that_root pins that is_managed("handbook") is true for the root directory itself, not just for files under it. This is characterisation, not endorsement. It is the honest reading of the prefix gate as written, and get_file on a directory fails upstream anyway. Recorded here so that if the gate is ever tightened, the test is understood as a description rather than a requirement.
• test_the_refusal_says_the_repository_when_no_roots_are_configured reaches its branch via a traversal path (handbook/../infrastructure/main.tf) rather than an out-of-roots path. With allowed_roots=None there is no out-of-roots path by definition, so _is_safe_path is the only way into _require_managed's error. The branch under test is the message, and the message is what is asserted.
• The user-agent assertion depends on urllib capitalising header names, so it reads User-agent rather than User-Agent. A precedent and an explanatory comment for this already existed in the credential test; the same comment was repeated at the new site rather than assumed.

One observation from outside the diff

python/src/github_docs/__init__.py exports __version__ in its __all__, but the API list in python/README.md does not mention it. Marginal, out of this PR's scope, and left alone here rather than smuggled into a test-only change. Filed separately so it is not lost.

---

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson