Skip to content

Phase 52 W1b + W2 — provenance off install's critical path (−697 ms cold, p≈0.0002)#10

Merged
tolgaergin merged 2 commits into
mainfrom
phase-52-bun-parity-pipeline
Apr 26, 2026
Merged

Phase 52 W1b + W2 — provenance off install's critical path (−697 ms cold, p≈0.0002)#10
tolgaergin merged 2 commits into
mainfrom
phase-52-bun-parity-pipeline

Conversation

@tolgaergin
Copy link
Copy Markdown
Contributor

Summary

  • W1b (cd64f8a) adds a per-stage decomposition of provenance-fetch cost (cache_hit_ns / http_ns / parse_ns) inside fetch_provenance_snapshot, plus a new perf.prov_ns_split log line. Splits fetch_and_parse into fetch_bundle_bytes (HTTP-only) + parse_bundle_or_log (CPU-only) helpers so the production path can time each half independently.
  • W2 (7c38972) moves provenance fetching from lpm install's build_blocked_set_metadata to lpm approve-scripts. Install no longer pays the cost; approve-scripts does an eager parallel fetch for the effective blocked set right after computing it. Drift detection is unchanged — the install drift gate still re-fetches candidate attestations independently and reads provenance_at_approval (which approve-scripts now stamps from a fresh fetch).

Empirical impact

W1b's perf.prov_ns_split measured 99.98 % HTTP, 0.02 % parse on cold install — the entire pre-W2 provenance cost was network round-trips for attestation bundles the install pipeline never read.

V2 cold-equal-footing bench on bench/fixture-large (266 packages):

n=30 raw n=24 (10 %-trimmed)
BEFORE median 5,156 ms 5,156 ms
AFTER median 4,354 ms 4,354 ms
Median delta (B − A) +802 ms (+15.6 %) +870 ms
Mean delta +500 ms +697 ms
Stdev 2,008 ms 772 ms
Paired t-stat 1.36 4.42
α=0.05 critical 2.045 2.069
Significance not significant p ≈ 0.0002

The trimmed view drops 6 outliers (3 per side — bidirectional network spikes on tarball downloads at iters 11 BEFORE / 18 AFTER) and lands on a tight, symmetric distribution with AFTER stdev (375 ms) tighter than BEFORE (563 ms) — the same variance-collapse pattern Phase 51 W1d showed.

BEFORE = e35890b (Phase 51 merge). AFTER = 7c38972 (this PR's HEAD).

Investigation: lockfile, frozen-mode, drift detection

The W2 design hinged on three open questions answered empirically by reading the rust-client source (full citations in the close-out doc):

  1. Lockfile (lpm.lock / lpm.lockb) carries no attestation digestsLockedPackage has zero attestation_* / provenance_* / cert_sha256 fields.
  2. No --frozen flag exists — only --offline and --force. Drift gate explicitly fires on fresh resolution only (!used_lockfile); lockfile fast-path is already provenance-passive.
  3. approve-scripts is the only end-consumer of provenance_at_capture — drift check on the next install reads provenance_at_approval (the value approve-scripts stamps), not the install-time capture. Moving the fetch to approval time preserves the drift-detection semantic end-to-end.

Bonus: W1a samply flamegraph invalidates Phase 51 §4.2's pool hypothesis

A debug-symbol samply capture (1 kHz, 234 threads, fully symbolicated via --unstable-presymbolicate + dSYM) revealed:

  • Workers are idle ~58 % of wall__psynch_cvwait (4,505 ms) + kevent (1,355 ms) across the sample. Not contending; starving.
  • Real work is filesystem-bound — ~2,486 ms across __open + clonefileat + write + close + mkdir + __rename + lstat + stat. lpm_store::PackageStore::stream_and_store_package is 2,480 ms inclusive (dominant lpm hot path).
  • Surprising hot leaf: lpm_security::behavioral::supply_chain::detect_minified is 401 ms inclusive / 73 ms leaf — runs in the per-file extract loop.

Phase 51 §4.2 + §5.1 hypothesized pool-redesign as the highest-priority post-provenance workstream. The flamegraph contradicts that — there's no contention to fix. W4 (pool redesign) is dropped from the Phase 52 priority list. New top priorities are W6 (detect_minified audit) + W7 (extractor filesystem-syscall batching).

Full close-out + flamegraph data in tolgaergin/a-package-manager:phase52-w2-closeout.md.

Pre-merge gate (CARGO_TARGET_DIR=/tmp/lpm-rs-phase52-target)

  • cargo build --workspace — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • fancy-regex banned-crate check — pass
  • cargo nextest run --workspace --exclude lpm-integration-tests --no-fail-fast4383 passed, 7 skipped, 0 failed

Test plan

  • All workspace unit tests pass (4383/4383)
  • V2 cold-equal-footing bench n=30 measures expected delta
  • V2 trimmed-mean test passes α=0.05 (p≈0.0002)
  • W1a flamegraph confirms HTTP cost removed; new bottlenecks identified for W6/W7
  • Optional manual E2E: lpm install then lpm approve-scripts esbuild@0.21.5 to confirm provenance_at_approval is populated from the approval-time fetch (verifies drift-detection end-to-end on the next install)

🤖 Generated with Claude Code

tolgaergin and others added 2 commits April 26, 2026 00:44
The Phase 51 close-out reported `prov_sum_ms=12000+` cold on the
266-pkg fixture and proposed it as the lever for Phase 52. Audit
revealed two issues with that framing: (1) `prov_sum_ms` is summed
across ~24 parallel tasks, so the wall-clock contribution is closer
to 470 ms (matches `perf.build_blocked_set_metadata`); and (2) the
single atomic can't tell whether the cost is cache-hit, network, or
parse — three very different fixes.

This commit lands the smallest instrumentation that resolves the
ambiguity: a `ProvenanceTimings` struct with three atomics
(`cache_hit_ns`, `http_ns`, `parse_ns`) threaded into
`fetch_provenance_snapshot`, with `fetch_and_parse` split into
`fetch_bundle_bytes` (HTTP-only) + `parse_bundle_or_log` (CPU-only)
helpers so each half can be timed independently. The pre-Phase-52
`fetch_and_parse` wrapper is kept under `#[cfg(test)]` so existing
tests keep their original one-call shape.

Caller (`build_blocked_set_metadata`) emits a new
`perf.prov_ns_split pkgs={} cache_hit_ms={} http_ms={} parse_ms={}`
line alongside the existing `perf.blocked_set_metadata_split`. The
three values do not sum to `prov_sum_ms` — the no-URL early return,
the cache-miss `read_cache` call itself, and the cache-write step
are deliberately not split (bounded small, would add noise without
changing what design decision the breakdown informs).

Drift-gate call site at install.rs:1896 passes `None` — it's not in
the build_blocked_set_metadata hot loop and not worth instrumenting.

Pre-merge gate (CARGO_TARGET_DIR=/tmp/lpm-rs-phase52-target):
- cargo build --workspace — clean
- cargo clippy --workspace -- -D warnings — clean
- cargo fmt --check — clean
- fancy-regex banned-crate check — pass
- cargo nextest run --workspace --exclude lpm-integration-tests
  --no-fail-fast — 4383 passed, 7 skipped, 0 failed

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
W1b's `perf.prov_ns_split` measured 99.98 % of build_blocked_set_metadata's
~550 ms cold wall as HTTP and 0.02 % as parse — the entire provenance
cost on cold install is network round-trips for attestation bundles
the install pipeline never reads. The unblocker investigation
(documented at the head of approve_scripts::fetch_provenance_for_effective_set)
confirmed that the only end-consumer of `provenance_at_capture` is
`lpm approve-scripts`, which reviews a small subset of the install
(typically 1–10 scripted packages out of hundreds).

This commit moves the capture from install to approval time:

* `build_blocked_set_metadata` (install.rs:3690) drops its per-package
  `fetch_provenance_snapshot` call. The function still concurrently
  fetches metadata for `published_at` + `behavioral_tags`, but
  attestation HTTP is gone. `BlockedSetMetadataEntry.provenance_at_capture`
  is always written as `None` — schema retained for build-state.json
  forward/backward compat. The `perf.prov_ns_split` line and its
  `ProvenanceTimings` plumbing are removed (always 0).

* `approve_scripts::run` adds an eager parallel fetch of provenance
  for the effective blocked set right after the set is computed.
  Skipped for `--list` (read-only, never materializes a binding) and
  for the empty-set case. The fetch overlaps with the user reading
  approval cards on the interactive path; on `--yes` bulk it
  parallelizes via `join_all` against the smaller fan-out.

* `approval_metadata_from_blocked` takes `provenance_at_approval` as
  an explicit parameter; each of the three call sites looks up the
  pre-fetched value from a `HashMap<(name, version), ...>`. No more
  reads of `blocked.provenance_at_capture`.

Drift detection is unaffected: the install drift gate (install.rs:1810)
re-fetches candidate attestations independently and reads
`provenance_at_approval` (the value approve-scripts now stamps from
its own fresh fetch) as its reference.

Pre-merge gate (CARGO_TARGET_DIR=/tmp/lpm-rs-phase52-target):
- cargo build --workspace — clean
- cargo clippy --workspace --all-targets -- -D warnings — clean
- cargo fmt --check — clean
- fancy-regex banned-crate check — pass
- cargo nextest run --workspace --exclude lpm-integration-tests
  --no-fail-fast — 4383 passed, 7 skipped, 0 failed

V2 cold-equal-footing bench (n=10) is the next step; expected
~400-550 ms median cold-install drop on bench/fixture-large.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tolgaergin tolgaergin merged commit fd3548f into main Apr 26, 2026
3 checks passed
@tolgaergin tolgaergin deleted the phase-52-bun-parity-pipeline branch April 26, 2026 10:06
tolgaergin added a commit that referenced this pull request May 14, 2026
…ix (#58)

* test(workflows): pin concurrency + recovery contracts for lpm install

Adds tests/workflows/tests/install_concurrency.rs with 13 falsifiable
tests covering production failure modes that had zero coverage:

Category A — process racing:
  * two concurrent installs on same project (pins finding-#77 floor)
  * install + concurrent store-clean serialize via shared/exclusive
    store_lock (probed via try_with_exclusive_lock on the actual
    lock file, not a directory-existence proxy)
  * two concurrent `lpm install -g` via global_tx_lock — proves
    final manifest + WAL coherence under serialized commits

Category B — interruption recovery:
  * kill mid-tarball-fetch leaves no .lpm/install-hash
  * next `lpm install` converges to a coherent end state

Category C — network faults:
  * tarball 503 → 200 succeeds after retry (counting Respond impl)
  * metadata 404 fails immediately without retry (<2s wall-clock)

Category D — filesystem faults:
  * readonly project dir fails with actionable error (no panic);
    POSIX-only via #[cfg(unix)], RAII guard restores permissions
  * `<project>/.lpm` planted as a regular file fails clearly

Category E — partial state recovery:
  * stale install-hash triggers re-resolve + refetch
  * partial node_modules re-links to full state
  * truncated lpm.lockb either recovers or fails cleanly (no panic)

Category F — WAL recovery hook:
  * torn WAL tail (3 garbage bytes) gets truncated by the dispatcher's
    recovery hook before the command runs; idempotent on re-invocation

Support helper refactor (same commit so the new helper has callers):
  * extracts env-isolation set into `LpmEnvSink` trait +
    `apply_lpm_env(cmd, project)` shared by `lpm()` (assert_cmd) and
    the new `lpm_spawnable()` / `lpm_spawnable_with_registry()`
    (std::process::Command, supports Child::kill())
  * trait impl on both Command variants ensures the two helpers
    cannot drift on the ~30 env knobs that gate test isolation

Surfaced findings during this work:
  * #77 — no project-level install lock: concurrent installs silently
    drop one side's work AND/OR fail with atomic-rename races (3
    observed failure modes documented in findings.md). Fix shape:
    LpmRoot::project_install_lock + with_exclusive_lock_async wrap.
  * #78 — retry-backoff has no test-friendly knob; retry-exhaustion
    tests take 15s+. Fix shape: LPM_RETRY_BACKOFF_MS_OVERRIDE env in
    debug builds.

CI gate locally green:
  clippy --workspace --all-targets -- -D warnings: clean
  cargo fmt --check: clean
  fancy-regex ban: empty
  cargo build --workspace: clean
  cargo nextest run --workspace --exclude lpm-integration-tests:
      6439 passed, 7 skipped, 1 leaky (pre-existing)

Deferred (filed under "next session" in the followup plan):
  B.3 (kill doesn't tear lockfile) — subsumed by B.1/B.2
  B.4 (panic injection) — needs LPM_TEST_PANIC_AT env hook
  C.2 (retry exhaustion) — blocked by finding #78
  C.3 (truncated body) — needs custom Respond with Content-Length mismatch
  D.3 (disk-full simulation) — no portable mechanism
  F.2, F.3 (orphan WAL, torn WAL with real records) — needs
    framed-WAL construction helpers

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(workflows): pin lpm.lock well-formedness + recovery skip-on-contention

Closes B.3 and F.2 of the concurrency tranche — 13 → 15 tests, meeting
the "≥15 of 21" acceptance criterion for Item 2.

B.3 — `install_killed_mid_pipeline_leaves_well_formed_or_absent_lockfile`:
Exercises two SIGKILL windows on the install pipeline — fresh project
and project with a committed lpm.lock from a prior install. After each
kill, asserts the on-disk lpm.lock is either absent OR parses as TOML.
Never half-written. Adds `toml = { workspace = true }` as a workflow-
tests dev-dep for the parse assertion. Helper
`assert_lockfile_well_formed_or_absent` shared between both windows.

F.2 — `lpm_command_skips_recovery_when_another_lpm_holds_global_tx_lock`:
Validates the dispatcher's `try_with_exclusive_lock` idempotent-skip
path at `main.rs:2531`. A background thread acquires `global_tx_lock`
via `lpm_common::with_exclusive_lock` and blocks on a channel. With
the lock held, runs `lpm global list` against a project with a torn-
WAL prefix — asserts the WAL bytes are UNCHANGED (skip arm fired,
recovery did not run). Then releases the lock and re-runs; asserts
the WAL is now truncated (recovery defers correctly to the next
lock-free invocation). Exercises both branches of the `try_with_
exclusive_lock` Ok(None) / Ok(Some) arm.

CI gate locally green:
  cargo clippy --workspace --all-targets -- -D warnings: clean
  cargo fmt --check: clean
  cargo nextest run --workspace --exclude lpm-integration-tests:
      6441/6441 passed, 7 skipped
  5x parallel re-run of install_concurrency: 15/15 stable each run

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(workflows): pin truncated-tarball + orphan-WAL recovery contracts

Two new tests in tests/workflows/tests/install_concurrency.rs:

- C.3 tarball_connection_dropped_mid_body_fails_or_retries: a custom
  wiremock Respond impl serves half a tarball with a Content-Length
  header naming the full length. Pins the install pipeline's
  retry-then-fail behavior on transport-class failures (~14s wall-clock
  for the full 4-attempt retry schedule). Hyper 1.9 server-side panics
  on the Content-Length lie, dropping the connection — a valid surrogate
  for a broken upstream / CDN dropping mid-body. Surfaced 8 tarball GETs
  per install (deterministic, 3-of-3 reproducer), explained by two
  distinct download_tarball_* call sites in install.rs each running the
  4-attempt retry budget.

- F.3 lpm_command_with_orphan_pending_tx_emits_recovery_banner: plants
  both halves of an orphan transaction (WAL Intent record without
  matching Commit/Abort + matching [pending.<pkg>] row in manifest.toml
  pointing at a non-existent install root) and asserts the dispatcher's
  recovery hook fires the RolledBack banner from main.rs:2543. Sets
  RUST_LOG=lpm=info to lift the default lpm=warn filter so the
  tracing::info! line surfaces. Adds lpm-global as a workflow dev-dep
  for WalWriter / IntentPayload / write_for. Pins post-state: orphan
  pending row gone, no spurious active row.

Together these close the C.3 and F.3 gaps in Item 2 of the test
coverage follow-up plan: 17/21 scenarios pinned (was 15/21). The four
remaining items all need source-side hooks (LPM_TEST_PANIC_AT,
LPM_RETRY_BACKOFF_MS_OVERRIDE, container infra) and are out of scope
for this tranche.

Full CI gate green: clippy clean, fmt clean, fancy-regex empty,
6443/6443 nextest pass (was 6441 pre-tranche).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(workflows): pin tarball-extraction security contracts at install tier

New file tests/workflows/tests/tarball_security.rs ships phase 1 of
Item 3 (tarball-extraction security): 5 of 10 planned tests covering
the most distinct security contracts at the install-pipeline tier.
Each test constructs its malicious tarball in-line via tar::Builder
(no checked-in fixtures), serves it through MockRegistry, and runs
lpm install end-to-end so any pipeline-level regression that bypasses
the extractor's hardening is caught.

Tests landed:

- #1 tarball_with_dot_dot_path_entry_is_rejected_by_install — pokes
  package/../escape.txt into the raw tar header bytes; install fails
  with "path traversal detected"; outside sentinel never created.
- #3 tarball_with_absolute_path_entry_is_normalized_to_relative_under_package_dir
  — renamed from "rejected" to reflect actual contract. The extractor's
  strip_first_component consumes the RootDir; an entry like
  /etc/lpm-pwned.txt extracts as node_modules/<pkg>/etc/lpm-pwned.txt.
  Install SUCCEEDS; literal /etc/lpm-pwned.txt is never written.
  Defensible: malformed-but-safe input normalized rather than refused.
- #2 tarball_with_symlink_to_outside_path_is_silently_skipped —
  renamed. The is_file() gate at lib.rs:398 silently drops symlinks;
  install succeeds with byte-identical outside sentinel.
- #5 tarball_with_hard_link_to_outside_file_is_silently_skipped —
  renamed. Same is_file() gate; hardlinks silently skipped; outside
  victim file unmodified.
- #8 tarball_with_setuid_executable_extracts_with_setuid_bit_stripped
  (POSIX-only) — tarball entry mode 0o4755 extracts as 0o755. SUID,
  SGID, and sticky bits all cleared via set_preserve_permissions(false)
  + the explicit `0o644 | exec_bits` mode set after write. Exec bits
  preserved.

Three tests carry a "plan-vs-actual" docstring section explaining why
the rename is defensible — the actual extractor contract differs from
the plan's prescribed phrasing in safe ways, not in regression-grade
ways. No findings filed.

Phase 2 (5 remaining tests: Unicode normalization, device file, FIFO,
zero-byte sanity, OS-max path) is deferred to a follow-up tranche
with rationale + lift estimate documented in the plan. None blocks
phase 1 acceptance.

Pre-merge gate green: clippy clean, fmt clean, fancy-regex empty,
6448/6448 nextest pass (was 6443; +5 for the new tests). 0.18s wall-
clock for the full file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(install): per-project lock prevents concurrent-install data loss

Closes finding #77. Two `lpm install <pkg>` invocations on the same
project no longer race on the manifest snapshot+commit window.

Pre-fix, both processes acquired only a SHARED store_lock and proceeded
in parallel. Each opened its own per-process ManifestTransaction
snapshot of the pre-edit package.json, staged its own dep on top, and
ran the install pipeline. Whoever wrote package.json + lpm.lock last
won; the other process's edits — including its node_modules link —
silently vanished. Both processes still exited 0 with success-path
output. CI scripts that ran two installs in parallel saw no signal of
the data loss.

The fix introduces:

- crates/lpm-common/src/paths.rs::project_install_lock(project_dir):
  free helper returning <project_dir>/.lpm/.install.lock. Re-exported
  from crates/lpm-common/src/lib.rs.

- run_add_packages and run_install_filtered_add in
  crates/lpm-cli/src/commands/install.rs now wrap the snapshot →
  stage → install → finalize → commit window in
  with_exclusive_lock_async against the project lock. The lock is
  per-project (no cross-project contention) and held across all
  ?-early-exits via the async block's return.

For the workspace path, the lock sits at the discovered workspace root
(not per-member) so two concurrent `lpm install --filter <member>`
invocations on the same workspace serialize without per-member
deadlock-ordering complexity.

run_with_options (the inner install pipeline) does NOT acquire this
lock — it's called from inside both run_add_packages's wrap and from
many other commands; double-acquiring the same fd-lock would deadlock
in-process.

Deferred (phase 2, not exercised by A.1): lpm add (add.rs:723-904)
has a similar 180-line transaction with recursive Swift handling.
Wrapping it is invasive and the race surface is theoretical (users
don't typically run `lpm add` and `lpm install` concurrently). Defer
to a separate tranche if a concurrent `lpm add` × `lpm install` race
is ever observed.

Test contract tightening (bug-first per CLAUDE.md):
two_concurrent_installs_on_same_project_leave_well_formed_manifest in
tests/workflows/tests/install_concurrency.rs went from "at-least-one
survives + manifest is well-formed JSON" (the floor) to "BOTH installs
succeed, BOTH packages present in package.json deps, BOTH packages
linked in node_modules/" (the contract). Pre-fix: 1/1 fail (pkg-b
silently dropped). Post-fix: 5/5 pass with no flakes (~1.2s wall-clock
each — install B observes pkg-a's commit and reports "Resolved 2
packages").

Pre-merge gate green: clippy --workspace --all-targets clean, fmt
clean, fancy-regex empty, 6448/6448 nextest pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(registry): test-only retry-backoff override env knob

Closes finding #78 + lands C.2 (`tarball_503_exhausts_retries_fails_with_http_status`).

Pre-fix, retry-exhaustion tests were blocked: the registry client's
backoff schedule (1+2+4+8s, capped at 10s) made every retry-exhaustion
test take ~15s per fetch site (~28s with the install pipeline's 2
distinct download_tarball_* call sites). MAX_RETRIES, RETRY_BASE_DELAY,
and RETRY_MAX_DELAY are private const with no env override. C.2
therefore had to be #[ignore]-gated behind LPM_RUN_SLOW_TESTS=1, and
the retry-exhaustion contract went unproven on `cargo nextest run`.

The fix introduces:

- crates/lpm-registry/src/client.rs::backoff_override(): reads
  LPM_RETRY_BACKOFF_MS_OVERRIDE (a u64 ms value) gated by
  cfg!(debug_assertions) || LPM_TEST_MODE=1. Returns Some(Duration)
  when both conditions hold; None otherwise. Production retry policy
  is immune — release builds without LPM_TEST_MODE=1 silently ignore
  the env.

- backoff_delay(attempt) consults the override before computing the
  exponential schedule.

- The two 429 Retry-After sleep sites also consult the override so a
  future 429-flood retry-exhaustion test wouldn't hang on the
  server-supplied header.

C.2 test landed alongside (bug-first per CLAUDE.md):

- Mock returns 503 on every tarball request — no recovery path.
- Test sets LPM_RETRY_BACKOFF_MS_OVERRIDE=10 on the lpm subprocess.
- Asserts: install fails non-zero, no panic, ≥4 attempts (proves the
  retry loop fired), elapsed < 2s (load-bearing — without the knob
  this fails at ~14s), stderr contains an actionable HTTP-class
  noun (503 / status / http / network / etc).
- Surfaces 8 tarball GETs per install (4 attempts × 2 distinct
  download_tarball_* call sites — matches C.3's observation).

Pre-fix verification: same C.2 against the unfixed client.rs failed
on the elapsed assertion at 14.04s (knob ignored). Post-fix: passes
in 1.6s cold / 0.1s warm. 5/5 passes with no flakes.

Pre-merge gate green: clippy --workspace --all-targets clean, fmt
clean, fancy-regex empty, 6449/6449 nextest pass (was 6448 pre-fix;
+1 for C.2).

Item 2 of the test-coverage-followup-plan now at 18/21 (was 17/21).
Both findings #77 and #78 fixed in production.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(workflows): tarball-security phase 2 — Unicode, device, FIFO, zero-byte, long-path

Adds 5 more tests to tarball_security.rs, completing Item 3 of the
test-coverage follow-up plan. Each test pins the actual extractor
contract under malicious-or-edge-case tarball shapes that reach the
install pipeline through MockRegistry.

Tests landed:

- #4 tarball_with_unicode_lookalike_parent_dir_extracts_safely_as_literal_bytes
  — renamed from "_normalization_traversal_rejected" to reflect the
  actual contract. Tarball entry path uses full-width dots U+FF0E `..`
  (bytewise NOT ASCII `..`). Component::ParentDir is byte-exact, so
  `..` becomes Component::Normal. Install SUCCEEDS; `..` materializes
  as a literal directory under node_modules/<pkg>/; outside sentinel
  byte-identical. Defensible because Path::components() doesn't
  NFKC-normalize on POSIX.

- #6 tarball_with_character_device_entry_is_silently_skipped
  (POSIX-only). EntryType::Char with /dev/null-shaped major/minor.
  Same is_file() gate as symlinks/hardlinks — silently skipped.
  Install SUCCEEDS; no device file at the expected path.

- #7 tarball_with_fifo_entry_is_silently_skipped (POSIX-only).
  EntryType::Fifo. Same posture as #6.

- #9 tarball_with_zero_byte_regular_file_extracts_as_empty_file.
  Sanity check that empty files still extract correctly (legitimate
  npm shape: .gitkeep, license placeholders).

- #10 tarball_with_single_path_component_exceeding_name_max_fails_cleanly.
  300-byte single-component name, well over POSIX NAME_MAX=255. Tar
  wire format succeeds via GNU long-name extension; the FILESYSTEM
  rejects on extraction (ENAMETOOLONG). Extractor wraps as
  LpmError::Io → install fails non-zero with the OS error visible
  and an actionable noun in stderr.

Three of the five tests are renamed to reflect actual extractor
contract vs the plan's prescribed phrasing — same "plan-vs-actual"
docstring pattern as phase 1. No findings filed; all 10 contracts
across phase 1 + 2 are defensible-as-implemented.

Pre-merge gate green: clippy --workspace --all-targets clean, fmt
clean, fancy-regex empty, 6454/6454 nextest pass (was 6449 pre-tranche;
+5 for the new tests). Full file 0.2s wall-clock for all 10 tests.

Item 3 now COMPLETE (10/10).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(workflows): cross-command flows Item 4 — migrate→rebuild + workspace filter isolation

Closes Item 4 of the test-coverage-followup-plan at 6/6 (target was
≥5). Two additions to tests/workflows/tests/cross_command_flows.rs:

- Plan #1 — extended flow_migrate_install_audit_lockfile_round_trips
  with a `lpm rebuild --dry-run --policy=deny` step. Pins the full
  migrate → install → audit → rebuild lifecycle. Asserts the rebuild
  step exits 0 + does not mutate the post-audit state (lpm.lock +
  lpm.lockb still present). Catches regressions where rebuild's
  lockfile or build-state parser breaks against a freshly-migrated
  manifest.

- Plan #5 — added flow_workspace_install_filter_member_a_does_not_mutate_member_b
  (new test, 159 LOC). Pins the workspace-member isolation contract
  using the workspace-monorepo fixture (3 members: app, core,
  utils):
    1. Initial filtered install on @test/core (re-pinning its
       existing semver dep) populates core's per-member quadruple:
       lpm.lock=319 B, lockb=230 B, install_hash=118 B.
    2. Snapshot core's full quadruple.
    3. Run `lpm install chalk@5.3.0 --filter @test/app` to add a
       new dep to app ONLY.
    4. Assert app's package.json gained chalk; core's quadruple
       (package.json + lpm.lock + lpm.lockb + install-hash) is
       BYTE-IDENTICAL post-install; chalk does NOT appear in
       core's node_modules/.

  Catches a regression where a per-member filtered install
  accidentally also mutates a sibling member's package.json /
  lockfile / install-hash — a real bug class because run_install_filtered_add
  shares the workspace-root project lock (added in #77 fix) and
  could over-snapshot if the target-set computation drifts.

  Helper `mount_pkg_full(mock, name, version)` factors out the
  three-step metadata + batch-metadata + tarball mount so the
  test body stays readable.

Other 4 plan flows already covered pre-tranche:
- Plan #2: flow_add_install_graph_added_dep_visible
- Plan #3: flow_install_patch_patch_commit_install_persists_patch
- Plan #4: flow_token_rotate_publish_dry_run_picks_new_token
- Plan #6: flow_install_upgrade_major_audit_picks_new_version

Pre-merge gate green: clippy --workspace --all-targets clean, fmt
clean, fancy-regex empty, 6455/6455 nextest pass (was 6454; +1 for
the new flow). Plan #5 stable across 5/5 reruns at ~0.11s each.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(install): LPM_TEST_PANIC_AT hook + B.4 panic-rollback contract

Adds a deterministic panic-injection hook to the install pipeline +
unblocks the long-deferred B.4 contract test for ManifestTransaction
Drop-based rollback on panic.

The hook (`maybe_test_panic(stage)` in
crates/lpm-cli/src/commands/install.rs) reads LPM_TEST_PANIC_AT and
panics when the env value matches the stage name. Gated to
`cfg!(debug_assertions) || LPM_TEST_MODE=1` — same pattern as the
#78 retry-backoff override. Production builds without LPM_TEST_MODE=1
silently treat the env as no-op.

Wired 4 stages in `run_add_packages`:
- "after-snapshot" — manifest unchanged; Drop is no-op
- "after-stage" — placeholder `*` written to package.json (load-bearing)
- "after-install" — pipeline complete; manifest still has `*`
- "after-finalize" — concrete versions written; pre-commit only

The hook unblocks B.4 (`install_panics_mid_pipeline_rollback_restores_manifest`),
deferred since the original Item 2 tranche because there was no
deterministic way to trigger a panic mid-install from a workflow
test. Recoverable errors fire `?`-rollback (covered by E.1/E.2/E.3);
SIGKILL bypasses Drop entirely (B.1/B.2/B.3 cover that). The panic
path was the missing rollback proof.

B.4 sets LPM_TEST_PANIC_AT=after-stage and asserts:
- process exits non-zero (panic propagates to runtime)
- stderr contains `"panicked at"` AND `"LPM_TEST_PANIC_AT=after-stage"`
- package.json BYTE-IDENTICAL to pre-stage (Drop ran on unwind, snapshot
  bytes restored — load-bearing)
- the new pkg is NOT in dependencies (placeholder rollback worked)
- .lpm/install-hash absent (invalidate-on-rollback)
- lpm.lock absent (matched optional snapshot's None pre-state)

Catches a regression where:
- panic = "abort" added to release profile (no Drop on panic)
- ManifestTransaction Drop logic stops restoring snapshot bytes
- The `lpm install` snapshot+commit window grows without re-wiring Drop

Test runs in 0.07s warm. 5/5 stable across reruns.

Pre-merge gate green: clippy --workspace --all-targets clean, fmt
clean, fancy-regex empty, 6456/6456 nextest pass (was 6455; +1 for B.4).
install_concurrency now at 19/19. Item 2 of test-coverage-followup-plan
moves to 19/21 — only A.2 (no contract) and D.3 (needs container
infra) remain deferred indefinitely.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(workflows): align MockRegistry tarball URL shape with production /-/ gate

Workflow tests mounted tarballs at `/tarballs/{name}-{version}.tgz` —
missing the `/-/` path segment that the registry-client's `evaluate_cached_url`
gate at [crates/lpm-registry/src/client.rs#L4117] requires (`.tgz` suffix
AND `/-/` substring). The gate is a defense-in-depth check that blocks
the H1 auth-token leak: a tampered lockfile URL like `/api/admin/foo.tgz`
(no `/-/`) would otherwise attach the bearer to a non-registry endpoint.

The mismatch produced two test-environment side effects that don't manifest
in production:

1. **WARN noise**: every install test that read a tarball URL from the
   lockfile fast path logged `cached tarball URL for X@Y failed shape check;
   falling back to on-demand lookup`. Polluted stderr across the suite.
2. **`shape_mismatch_count` defeated**: the registry-client documents this
   counter as a "BUG signal — the writer should never emit a gate-rejectable
   URL". Test runs incremented it on every install, making the counter
   useless for catching real bugs.

This commit migrates the mock to the production-shape
`/tarballs/{name}/-/{name}-{version}.tgz` everywhere — both the helper
methods (`MockRegistry::tarball_path` / `tarball_url`) and the ~60
hard-coded `format!` sites across 14 test files + 1 snapshot.

The new `tarball_path` helper is `pub` with a prominent docstring warning
future test authors not to re-introduce the legacy shape. Internal mounts
in `with_package_and_deps` / `with_package_published_at` /
`with_full_package_metadata` all route through it.

Post-fix verification: WARN gone, gate `Accepted` path runs, all 691
lpm-workflows tests pass (0 leaky in the latest full-workspace run, down
from 1-3 leaky pre-fix — fewer fallback paths firing).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(workflows): test-coverage-followup tranche — Items 2/3/4/5

Closes the remaining open rows from `private/test-coverage-followup-plan.md`
across four items. ~2,600 LOC of new test code + fixture + budget infra.

**Item 3 — tarball-security additional candidate surfaces (7 tests
in `tarball_security.rs`):**

- `tarball_with_pax_path_traversal_rejected` — PAX extended `path`
  header smuggling `..` is rejected by the extractor's `Component::ParentDir`
  check after the tar crate resolves the override.
- `tarball_with_gnu_longname_traversal_rejected` — symmetric GNU `L`
  entry; same rejection path.
- `tarball_rejects_or_rolls_back_when_later_entry_is_malicious` — pins
  the `rollback_extraction` contract: valid first entry is cleaned up
  when a later `..`-traversal entry trips rejection mid-stream.
- `tarball_with_duplicate_member_path_rejected_or_deterministic` — pins
  current last-write-wins contract (defensible; flagged scanner-
  disagreement risk in test comment).
- `tarball_with_truncated_gzip_rolls_back_partial_extract` — half-
  truncated gzip stream → libdeflate fails cleanly → no partial extract.
- `tarball_ignores_uid_gid_ownership_metadata` (POSIX) — bogus uid/gid
  in tar header is ignored; extracted files owned by process uid.
- `tarball_with_sparse_huge_file_rejected_by_declared_size` — manually-
  constructed tarball with header declaring `MAX_FILE_SIZE + 1` and
  empty on-wire body; extractor rejects on the pre-check at lib.rs:306
  before draining body.

**Item 4 — cross-command flows additional candidate surfaces (2 tests
in `cross_command_flows.rs`):**

- `flow_install_uninstall_install_graph_round_trip` — pins manifest /
  link / graph hand-off through a full round-trip.
- `flow_cache_clean_then_offline_install_uses_store_or_fails_helpfully` —
  pins the cache/store boundary: `cache clean` must not corrupt offline
  install; store-side bytes byte-identical after a clean.

**Item 2 — concurrency/recovery additional candidate surfaces (3 tests
in `install_concurrency.rs`):**

- `cache_clean_during_slow_tarball_install_does_not_corrupt_install`
  (G.4) — install + cache clean run concurrently (different lock paths,
  no serialization); install succeeds despite metadata cache wipe
  mid-stream. Empirical timing observed: install elapsed 1.57s, cache
  clean fired at t=30-39ms cleanly inside the install window.
- `install_panics_after_install_hash_write_rollback_invalidates_hash`
  (G.5) — reuses existing `LPM_TEST_PANIC_AT=after-install` stage (no
  new source-side hook needed — `write_post_install_v6_hash` runs
  inside `run_with_options` which returns BEFORE that stage fires).
  Pins that Drop-based rollback restores manifest AND deletes the
  freshly-written install-hash.
- `malformed_registry_json_fails_without_manifest_or_lockfile_mutation`
  (G.6) — truncated JSON on all three metadata endpoints; install
  fails cleanly, no panic/backtrace, package.json byte-identical, no
  torn lockfile.

**Verdaccio-npm parity for `which@4.0.0`
(`install_real_registry.rs`):**

- `verdaccio_npm_parity_for_bin_package_pins_metadata_and_shim_presence`
  — extends the existing lodash byte-diff with a bin-shipping target
  package. Asserts metadata equivalence + `.bin/<name>` shim present
  on both sides + bin target file materialized + exec bits non-zero
  (POSIX).

**Item 5 — realworld fidelity (new fixture + new test file):**

- `tests/fixtures/realworld-nextjs/` (package.json + README) — pinned
  Next.js 14.2.13 + React 18.3.1 + TypeScript 5.6.3 + 3 `@types/*`
  packages. Resolves to ~28 transitive deps empirically. README
  documents the calibration methodology including raw measurement data.
- `tests/workflows/tests/install_realworld.rs` — `install_realworld_nextjs_fixture_succeeds_through_verdaccio`
  installs the fixture through Verdaccio→npmjs and asserts end-to-end
  success at production scale. Always logs cold + warm wall-clock + peak
  RSS to stderr for calibration data.
- **`LPM_BUDGET_GATE=1`-gated budget assertions**: cold ≤ 25s, warm ≤
  25ms, cold peak RSS ≤ 1500 MiB. Calibrated from N=6 cold + N=3 warm +
  N=3 RSS runs on M-series macOS, 2026-05-14. Memory measurement via
  `/usr/bin/time -l` (macOS) / `-v` (Linux); Windows skips with a clear
  warning.

This closes Item 5 entirely (all 4 acceptance criteria green) and brings
Items 2/3/4 to the parked-by-design or infrastructure-blocked baseline.

CI gate: clippy `--workspace --all-targets -- -D warnings` clean, fmt
clean, fancy-regex empty, build clean, `cargo nextest run --workspace`
6471/6471 pass. Suite runtime ~2:40 (was ~2:24 pre-tranche; +15s for the
realworld test).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(workflows): collapse Linux-only let-chain in parse_peak_rss

CI lint on Linux failed on `clippy::collapsible_if` in the Linux-cfg'd
branch of `parse_peak_rss`. The macOS branch had an intermediate
`let bytes_str = rest.trim();` between the two `if let`s, which is why
the local clippy run on macOS didn't catch this — only the macOS-cfg
branch compiled there.

Collapse the Linux branch to use `&&` (stable let-chains) so it
satisfies the lint while preserving the same semantics.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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