Skip to content

Hash agent-mode jar members through the shared streaming zip comparator instead of buffering each member #914

Description

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: register comment.

Kind: refactor (performance; one duplicated routine). Source: new finding, register row C51. It is the agent-mode twin of C01 (#569, fixed by #587).

Problem (main @ 9c43dfc)

There are two routines that hash named zip members with Git SHA-256 and compare them to a patch record, and they have drifted apart.

Vendored (streams): vendor/common.rs::zip_bytes_match_after_hashes. Since #587, following the maintainer's direction on #569, it streams each member through compute_git_sha256_from_std_reader:

if !compute_git_sha256_from_std_reader(entry.size(), entry)
    .is_ok_and(|hash| hash == info.after_hash)

Agent mode (buffers): patch/jvm_jar.rs::verify_member_bytes inflates every patched member into a Vec and then hashes it:

let mut entry = a.by_name(member).ok()?;
let mut buf = Vec::new();
entry.read_to_end(&mut buf).ok()?;
Some(buf)
// …
content.map(|c| compute_git_sha256_from_bytes(&c))

verify_member_bytes runs on every agent-mode Maven/Gradle jar record: in apply verification through verify_members (apply.rs L397–L401), in vex verification (vex/verify.rs L344–L349), in apply_jar_swap (L464) and in the rollback restore (L634). That covers every copy in ~/.m2 and in each Gradle cache hash directory.

Measured (temporary core lib test, debug build, peak RSS from /proc/self/status VmHWM, run twice with the same result): a jar holding one deflated 1 GiB member (1.0 MB on disk) plus one small member, with a record for the big member.

Routine Peak RSS before Peak RSS after
jvm_jar::verify_member_bytes 13 MiB 1,067 MiB
common::zip_bytes_match_after_hashes 13 MiB 26 MiB

Symptoms

None filed. Impact: memory grows with the inflated member size on each agent-mode jar verification, in apply, rollback and vex. This is the same defect #569 fixed for vendored mode, so it is per the maintainer a performance problem, not a trust one; no cap is proposed. Size: ~20 lines.

Proposed change

  • Add one helper next to the stream hasher, for example hash::git_sha256::zip_member_git_sha256(archive, name) -> Option<io::Result<String>>. It looks up a member and streams it through compute_git_sha256_from_std_reader.
  • zip_bytes_match_after_hashes and verify_member_bytes both call it.
  • Deleted: the buffered read_to_end closure in verify_member_bytes, and the duplicate lookup-and-hash loop in zip_bytes_match_after_hashes.
  • Keep each caller's lookup rules as they are: vendored refuses keys that fail is_safe_relative_subpath, and agent mode reports a missing member as NotFound/Ready.

Size and scope

Acceptance criteria

  • verify_member_bytes doesn't buffer a member: rg "read_to_end" crates/socket-patch-core/src/patch/jvm_jar.rs prints nothing in the verify path.
  • A regression test in jvm_jar builds (streaming) a jar with a large zero-filled deflated member and checks the VerifyResult for it. The peak-RSS check can stay out of CI, as Stream ZIP member hashes during vendored verification #587's did.
  • The existing jvm_jar tests, patch::apply and vex::verify jar tests, the Maven/Gradle agent e2e tests and vendor::common tests stay green.
  • cargo clippy --workspace --all-features -- -D warnings is clean.

Dependencies

None. It touches jvm_jar.rs lines that #878 and #690 don't edit (those change the private sha256_hex/sha1_hex).

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 6, 2026
  2. added a commit that references this issue on Oct 6, 2026
  3. mikolalysenko commented on Oct 6, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triaged as priority:p3 (Maven/Gradle agent-mode refactor). Not a duplicate: #569 (fixed by #587) covered the vendored routine only. No open PR covers it yet.


    Generated by Claude Code

  4. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Claiming this issue for the architecture refactor routine (highest leverage among free files: removes the buffered jar-member read on every agent-mode Maven/Gradle verify, S 3). Slice: the shared streaming helper plus jvm_jar.rs; vendor/common.rs follows when #1227 frees it. Branch: arch-refactor/914-jar-member-stream. Claim-ID: 2026-10-09T08:58:00Z-02b9a7


    Generated by Claude Code

  5. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Draft PR: #1253 (shared zip_member_git_sha256 helper + the jvm_jar.rs verify path). The vendor/common.rs caller remains for a follow-up once #1227 no longer changes that file.


    Generated by Claude Code

  6. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] The agent-mode slice merged in #1253: jvm_jar::verify_member_bytes now streams each member through hash::git_sha256::zip_member_git_sha256 (302 → 30 MiB peak RSS on a 256 MiB member). Remaining: move vendor/common.rs::zip_bytes_match_after_hashes onto the same helper (the file is free now that #1227 merged) and commit a test for a member whose declared size disagrees with its stream. Releasing the claim until a PR takes that slice.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)pm:mavenMavenpriority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions