Repository navigation
fix: pin get_binary_url safe-path behavior (OVH-05) - #22
Merged
Merged
Conversation
The safe runtime (default, legacy=False) used to return a fabricated
cache-relative path that did not correspond to any real download —
acquisition writes to cache_dir/<executable_name> while get_binary_url
returned cache_dir/<relative_path>, and the method also produced a path
rather than a URL despite its name. Latent: nothing internal called it
on the safe path.
Raise NotImplementedError on the safe path with a message directing
callers to ensure_binary() (or a pre-verified binary_path= constructor
argument). Acquisition stays explicit — no network at construction, no
implicit URL fabrication.
Legacy branch byte-identical: supported platforms still return the URL
string and unsupported platforms still raise the typed error pinned by
tests/test_characterization_platform.py.
Adds tests/test_default_api.py::test_get_binary_url_raises_on_safe_runtime
pinning the new behavior. Drops the now-unused ArtifactCatalog import
from py7zip.py.
OVH-05 per docs/handoffs/2026-10-11-py7zip-overhaul.md; addresses F2 in
docs/handoffs/PLATFORM_ARCHITECTURE_PY7ZIP_2026-10-10.md.
Validated:
python3 -m pytest tests/test_default_api.py -q # 4 passed
python3 -m pytest tests/test_characterization_platform.py \
tests/test_characterization_urls.py -q # 34 passed (legacy byte-identical)
python3 -m pytest tests -p no:cacheprovider -q # 129 passed (was 128)
Co-Authored-By: Claude Code <noreply@anthropic.com>
|
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR #22 review follow-up: the safe-runtime NotImplementedError message and get_binary_url docstring directed callers to ensure_binary(), which is internal to SafePy7zip (reachable only via the private _safe attribute). The public Py7zip entry point for acquisition is download_binary(), which delegates to _safe.ensure_binary(); point the message and docstring there instead. test_get_binary_url_raises_on_safe_runtime now asserts the message names download_binary() and never mentions the internal ensure_binary name. Adds the AGENTS.md-required Unreleased entry to .github/CHANGELOG.md for the OVH-05 user-visible behavior change. Validated: python3 -m pytest tests/test_default_api.py \ tests/test_characterization_platform.py \ tests/test_characterization_urls.py -p no:cacheprovider -q # 38 passed ruff check py7zip/py7zip.py tests/test_default_api.py # passed ruff format --check py7zip/py7zip.py tests/test_default_api.py # clean python3 -I -m py_compile py7zip/py7zip.py tests/test_default_api.py Co-Authored-By: Claude Code <noreply@anthropic.com>
aliasfoxkde
marked this pull request as ready for review
October 11, 2026 05:02
aliasfoxkde
added a commit
that referenced
this pull request
Oct 11, 2026
* docs: rebase overhaul handoff on main (rev 3) Rebaseline docs/handoffs/2026-10-11-py7zip-overhaul.md onto origin/main = d606b75 (PR #22) and record OVH-05 as landed throughout (S1, S5.1, ledger, S6 spec); py7zip.py citations re-verified at the new HEAD (PR #22's import removal shifted the file by one line; publish.yml tag gate tightened to :28-35). Incorporate the five independently reviewed gaps: 1. OVH-14 becomes a deprecate-then-remove sequence for full/incremental/differential/snapshot and cd, grounded in new V20 evidence: the PyPI 0.6.2 sdist ships all five methods, so direct removal would turn 0.6.2 callers' silent no-op success into an AttributeError on upgrade. 2. B0 Windows behavior is explicit (repo-hosted checksum-pinned binaries as a governed fallback, typed refusal on windows/arm, no support claim), and D-OVH-2/OVH-07 gains a bounded decision deadline with escalation instead of an indefinite fallback. 3. Acceptance rules require runner-owned mktemp scratch honoring TMPDIR; fixed /var/tmp paths in evidence rows are classified as historical probe artifacts, never acceptance requirements. 4. Env-guarded integration probes are non-gating for OVH-11/OVH-21: the probe must exist (registered marker, skipped by default); its run output is receipt evidence only. 5. New 6.2 enumerates the supported platform/architecture rows (exactly ArtifactCatalog's surface plus typed refusals) and binds them into OVH-03/04/09/11/23 acceptance. Docs-only; no runtime code edited. Revision 3 entry added per the document's in-place revision convention. Co-Authored-By: Claude <noreply@anthropic.com> * docs: revise overhaul handoff per review (rev 4) Folds an independent review of PR #23 at a9a0a06 (Z.AI GLM Turbo, glm-5-turbo; PASS with corrections) into the overhaul handoff: - M1: D-OVH-2 catalog-freeze deadline is enforced by an explicit OVH-09 acceptance rule; drop the false claim that OVH-13's Depends on enforces it (OVH-09 deps are OVH-02/08/29). Ledger re-validated: acyclic. - L1: new gate OVH-30 (D-OVH-7) records the snapshot-family/cd removal version and window; OVH-14 commit 1 quotes the record instead of inventing a version. - L2: citation fixes - characterization pins are tests/test_characterization_wrapper.py:300-356, publish.yml tag gate is :28-34, requests claim adds legacy setup() at py7zip/py7zip.py:123. - L3: OVH-11 fake-transport acceptance requires a size-aware or chunking fake (FakeUrllibRequest._Context.read() takes no size; production reads read(1024*1024) chunks). - L4: OVH-25 consumes only PLATFORM_MATRIX.md tier definitions; its host rows are historical and contradicted by platforms.py. - Info: workflow_dispatch cannot exercise the trusted-publishing step (release-only gates); OVH-02 must reconcile the stale bin/INFO.md inventory. Docs-only; no runtime code edited. Co-Authored-By: Claude <noreply@anthropic.com> * docs: correct OVH-02/05 evidence in handoff Follow-up to rev-4 review (3f14593): - OVH-05: the landed NotImplementedError message directs callers to download_binary() or a pre-verified binary_path= argument only (py7zip/py7zip.py:162-167); it never mentions ensure_binary(), pinned by tests/test_default_api.py:50. Cited source and test. - OVH-02: the two INFO.md-listed paths (bin/win/arm/x86/7za.exe, bin/mac/arm/x32/7za) are absent from disk, while the win/arm and mac/arm slots exist only as 0-byte placeholders at different names (x32/x64 without .exe; x64/x86) - no usable artifact either way. Narrowed the convention claim: only INFO.md:6 drops architecture; INFO.md:7 and the examples keep it; the INFO.md:10 lin/arm/x86 example disagrees with on-disk bin/lin/arm/x32/7za. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Michael Kinney <aliasfoxkde@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
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.
Summary
docs/handoffs/2026-10-11-py7zip-overhaul.md): the saferuntime's
Py7zip.get_binary_url()previously returned a fabricatedcache-relative path that did not correspond to any real download
(acquisition writes
cache_dir/<executable_name>; the method returnedcache_dir/<relative_path>). It also returned a path despite its"URL" name. Latent: nothing internal called it on the safe path.
NotImplementedErrordirecting callers toensure_binary()(or a pre-verifiedbinary_path=constructorargument). Acquisition stays explicit — no network at construction,
no implicit URL fabrication.
legacy=True) byte-identical: supported platformsstill return the URL string; unsupported host platforms still raise
the typed error pinned by
tests/test_characterization_platform.py.Evidence
docs/handoffs/PLATFORM_ARCHITECTURE_PY7ZIP_2026-10-10.md(cache-relative path mismatch; non-URL).
docs/handoffs/2026-10-11-py7zip-overhaul.md§6: "Preferdeprecation/delegate to
ensure_binary(); keep legacy branchbyte-identical; new pinning test in
tests/test_default_api.py."Targeted validation
python3 -m pytest tests/test_default_api.py -qpython3 -m pytest tests/test_characterization_platform.py tests/test_characterization_urls.py -qpython3 -m pytest tests -p no:cacheprovider -qFiles changed
py7zip/py7zip.py— safe-path raise + docstring; legacy branchunchanged; remove now-unused
ArtifactCatalogimport.tests/test_default_api.py— newtest_get_binary_url_raises_on_safe_runtimepinning the safe-pathbehavior.
Scope
previously-buggy
get_binary_url()safe-path return value.Next step
OVH-01 (classifier + status bump), OVH-02 (provenance ledger), and
the OVH-26 audit-D4 decision are the remaining P0 release-unblock tasks
on the B0 critical path; OVH-05 sits between OVH-03 (docs truth-pass)
and the publication gate (audit P0.5/D3, taken inside OVH-21).
🤖 Generated with Claude Code