Skip to content

What Argon's read of #540 left: the unpack cap's exact bound, the redirect row, and the Head tests run as a file - #670

Open
don-petry wants to merge 1 commit into
mainfrom
reads-540-follow-on
Open

don-petry wants to merge 1 commit into
mainfrom
reads-540-follow-on

Conversation

@don-petry

@don-petry don-petry commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Follow-on to #540, "The review reads the pull request's head, held to it by --restricted". Argon was out of quota when #540 merged. It read #540 at its merge commit, 9682602, and found one major and three minors. This PR fixes three of them; the fourth was already held at the caller.

No protected file is touched and no record is added.

Argon's finding What this does
(major) The unpack cap's test checks no exact bound and no cumulative size The cap is one check on the whole archive unpacked, so it is cumulative by construction. Its bound is now held: an archive of exactly UNPACKED bytes, two members of 2048 and 2049, is written, and with one byte fewer allowed it is refused. A new mutant, "an archive of exactly the cap is refused" (> to >=), is killed by that test
(minor) adapters/AGENTS.md says an artifact's download "alone" follows its redirect, one row under the row that allows a tarball's too The row now says the download follows its redirect to the storage alone, which is what it does
(minor) Head stands after if __name__ == "__main__" in tests/test_harness.py, so running the file as a script skips it The block is now at the end of the file
(minor) Head never asserts that a refused archive leaves nothing written Already held: unpack stops at the first check that fails and leaves what it wrote for its caller, and harness/read.py removes it, as tests/test_steps.py's head_unreadable cases assert. The docstring said "refused whole"; it now says this

Not done: a row in adapters/AGENTS.md that names the tarball redirect's own tests. The Claude review and the security pass both asked for one; see the table.

Checks

  • make verify on an export of the head: 1716 tests OK, at 424af11.
  • The new mutant is killed. All mutant targets occur once in their files.

Reads

Reviewer Head Unread parts Blocking Major Minor Info
agy 424af11 none 0 0 0 0
muse 424af11 none 0 0 0 0
claude-review 424af11 none 0 0 1 1
claude-security 424af11 none 0 0 0 1
Finding Severity Criterion Where What Disposition
claude-review#1 minor PR-1 adapters/AGENTS.md:12 Only half of Argon's redirect-row finding is fixed: the row allowing a tarball's redirect still names no test that holds it Left: adapters/AGENTS.md is at its 40-line limit and its rows near 260 characters, and a rule names a test, never a class, so no row can name the tarball's two tests without dropping another rule. The rule is held by those tests and their mutants; the contradiction Argon named is gone.
claude-review#2 info none docs/decisions/0138-the-review-reads-the-pull-requests-head-with-the-read-tools-alone-held-to-it-by-restricted.md:41 The Head docstring now denies 'refused whole', but record 0138's boundary row and the test's name still claim it Left: record 0138 is replaced, and its row is never edited but to re-point; both callers remove what a refusal leaves, as tests/test_steps.py holds.
claude-security#1 info none adapters/AGENTS.md:13 The rewritten redirect row names the artifact download's destination limit but no row names the tarball redirect's limit to codeload Left, as claude-review#1.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

…irect row, and the Head tests run as a file (#540)

Argon read #540 after it merged (it was out of quota then):
- The unpack cap is one check on the whole archive unpacked, so it is cumulative by
  construction, but its bound was held by nothing: a 5000-byte member against 4096. Now an
  archive of exactly UNPACKED bytes, two members of 2048 and 2049, is written, and one byte
  fewer allowed refuses it; the mutant "an archive of exactly the cap is refused" is
  killed by it.
- adapters/AGENTS.md said an artifact's download "alone" follows its redirect, one row
  under the row that lets a tarball's follow too. It now says the download follows its
  redirect to the storage alone, as it does.
- tests/test_harness.py's Head class stood after the __main__ block, so running the file
  as a script skipped it; the block is at the end.
- Head's docstring said a refused archive is refused whole. unpack stops at the first
  check that fails and leaves what it wrote for its caller, which harness/read.py removes,
  as tests/test_steps.py's head_unreadable cases hold; the docstring says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@don-petry
don-petry requested a review from a team as a code owner November 9, 2026 05:01
@coderabbitai

coderabbitai Bot commented Nov 9, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-09T05:01:48.395733Z
We couldn’t complete this review. Comment @codex review to try again.
424af11 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant