Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/shipper-cargo-failure/tests/classifier_contract_integration.rs`:
- Around line 1-19: The tests only exercise a single pattern per class; add unit
tests that call classify_publish_failure with additional representative strings
from RETRYABLE_PATTERNS and PERMANENT_PATTERNS (e.g., rate-limit messages, auth
failures, "already uploaded" messages) and assert CargoFailureClass::Retryable
or ::Permanent accordingly, and add a proptest using proptest::prelude::*, e.g.,
a test classify_never_panics that generates arbitrary stderr and stdout strings
and asserts the returned outcome.class is one of CargoFailureClass::Retryable |
CargoFailureClass::Permanent | CargoFailureClass::Ambiguous to ensure no panics
or unexpected behavior from classify_publish_failure.

In `@crates/shipper-cli/src/main.rs`:
- Around line 279-294: The current code sets index_base =
cli.api_base.as_ref().map(|_| api_base.clone()), causing index readiness checks
to call the API host; change index_base handling so it is left None when the
user supplies --api-base (allow Registry::get_index_base to derive the index
host), or explicitly derive an index URL from the provided api_base (e.g.,
convert https://crates.io -> https://index.crates.io) before constructing
ReleaseSpec; update the variable index_base and the ReleaseSpec/Registry
construction (see index_base, api_base, ReleaseSpec and Registry) to use the
corrected value.

In `@crates/shipper-cli/tests/bdd_publish.rs`:
- Around line 77-162: Extract the duplicated workspace builders
(create_single_crate_workspace and create_parallel_workspace) into a shared test
helper module (e.g., tests/support or a common test_utils module) and update
this test and the other consumer (parallel_levels_integration.rs) to call the
shared helpers; specifically, move the implementation of both functions into the
shared module, export them (pub) so tests can use them, replace the local
definitions in crates/shipper-cli/tests/bdd_publish.rs with imports from the
shared helper, and update any use sites to call the new shared function names to
avoid drift and duplication.
- Around line 227-268: The test server in spawn_index_readiness_registry
currently stops after exactly four requests (for _ in 0..4), which can make
tests flaky; change the fixed-count loop to a loop that continues until
server.recv_timeout returns None or Err (i.e., loop { match
server.recv_timeout(Duration::from_secs(5)) { Ok(Some(req)) => { ... handle req
(same logic using api_path/index_path/index_body) }, Ok(None) | Err(_) => break
} } ) so the thread serves requests until a timeout occurs (or make the Duration
configurable) rather than a hard limit of 4.
- Around line 313-352: Replace the hard-coded asserts in the test function
given_parallelizable_workspace_when_grouping_levels_then_independent_crates_share_level
with an insta snapshot assertion: import insta::assert_yaml_snapshot at the top
of the test module, serialize the computed levels (the levels variable produced
by ws.plan.group_by_levels()) into a snapshot-friendly form (e.g., map levels to
their package names using the existing iterator logic) and call
assert_yaml_snapshot! with that value so the expected level grouping is stored
and managed by insta instead of inline assert_eq! calls.
- Around line 565-625: Add an insta JSON snapshot assertion for the receipt
instead of (or in addition to) the manual package assertion: import
insta::assert_json_snapshot at the top of the test module and replace the
current packages/assert! check in
given_index_readiness_mode_when_publish_then_reads_sparse_index_metadata with a
call to assert_json_snapshot!(receipt) (or assert_json_snapshot!(packages) if
you prefer to snapshot only the packages array) so the test records the receipt
JSON output for future diffs.

In `@crates/shipper-duration/src/lib.rs`:
- Around line 34-39: The current serialize_duration function casts
duration.as_millis() (u128) to u64, which truncates values > u64::MAX; change
serialize_duration to call duration.as_millis().try_into() and handle the
Result—on Ok pass the u64 into serializer.serialize_u64, on Err return a serde
serialization error (e.g., via S::Error::custom or serializer error conversion)
so overflow is detected and reported instead of silently truncating. Ensure you
update the function signature uses S: Serializer and keep the
serializer.serialize_u64 call for the success path.
- Around line 10-13: Change parse_duration to return anyhow::Result<Duration>
instead of Result<Duration, humantime::DurationError>: add anyhow to
dependencies and update the signature of parse_duration and its body to
map/convert the humantime error into an anyhow::Error (for example via
.map_err(anyhow::Error::from) or using anyhow::Context). Specifically, update
the function named parse_duration to return anyhow::Result<Duration> and convert
the result of humantime::parse_duration(input) into an anyhow result; also add
anyhow = "..." to the crate's Cargo.toml dependencies.

In `@crates/shipper-duration/tests/duration_config_integration.rs`:
- Around line 19-51: Add insta snapshot assertions to the three tests instead of
literal assert_eq checks: add insta = "1" to dev-dependencies, import insta
macros in the test module, and replace the existing equality checks in
toml_accepts_human_readable_durations, json_accepts_millisecond_integers, and
json_serializes_as_milliseconds with insta::assert_snapshot! calls of the
serialized output (e.g., for toml test serialize cfg with toml::to_string(&cfg)
and assert_snapshot!(toml_out), for json tests use serde_json::to_string(&cfg)
or reuse the existing out variable and assert_snapshot!(out)); keep the rest of
the test logic intact so the parsed RetryLikeConfig values are what get
serialized for snapshots.

In `@crates/shipper-levels/src/lib.rs`:
- Around line 148-223: Add insta as a dev-dependency in the crate by adding
`insta = "1"` to the `[dev-dependencies]` in Cargo.toml, then convert the
equality assertions in the tests in lib.rs (tests like
returns_empty_for_empty_input, assigns_chain_to_strict_levels,
assigns_independent_branches_to_same_level,
ignores_dependencies_outside_the_plan,
falls_back_deterministically_when_cycle_is_present) from
assert_eq!(names(&levels), ...) to insta snapshots, e.g. replace each assert_eq!
with insta::assert_debug_snapshot!(names(&levels)); so the tests use insta
snapshots for the output of names(&levels).

In `@crates/shipper-plan/src/lib.rs`:
- Around line 1-9: Add the crate-level attribute to forbid unsafe code by
inserting #![forbid(unsafe_code)] at the top of the crate root (lib.rs) before
the use/imports so the compiler enforces the workspace unsafe-code prohibition
for this crate; ensure the attribute appears as the very first item in the file
above the existing use statements.
- Around line 24-226: The dependency name lists inserted into the plan are
nondeterministic because they iterate over deps_of (keyed by PackageId); in
build_plan, update the logic that builds the dependencies BTreeMap so that for
each id you collect dependency package names, sort them (e.g., lexicographically
by package name) before inserting into dependencies; locate the construction
around the deps_of.get(id) mapping in build_plan and ensure the Vec<String>
dep_names is sorted deterministically prior to dependencies.insert(pkg_name,
dep_names).
- Around line 451-626: Add serial_test = "3" to dev-dependencies in
crates/shipper-plan/Cargo.toml, then import the crate (use serial_test::serial
or #[serial] attribute) in lib.rs and mark the filesystem-mutating tests with
#[serial]: build_plan_filters_publishability_and_orders_dependencies,
build_plan_rejects_publishable_depending_on_non_publishable,
build_plan_package_selection_ignores_unrelated_invalid_deps,
build_plan_allows_dev_dep_on_non_publishable,
build_plan_selected_packages_include_internal_dependencies,
build_plan_selected_single_package_does_not_include_dependents,
build_plan_errors_for_unknown_selected_package, topo_sort_reports_cycles, and
build_plan_is_deterministic_for_independent_nodes_by_name; leave
build_plan_errors_for_missing_manifest unchanged. Ensure the import and
attribute usage reference the serial_test crate so tests run serially when they
touch the filesystem.
- Around line 312-329: The tests module uses verbose inline assertions; add
insta to dev-dependencies and convert those assertions to snapshot tests: update
Cargo.toml [dev-dependencies] to include insta, then in the tests mod (functions
using proptest and the test helpers like write_file, MetadataCommand usages and
the tests that currently call assert_eq! on package name vectors and
assert!(format!("{err:#}").contains(...))) replace those equality and
error-contains checks with insta snapshot assertions (e.g., capture the vector
or formatted error as a string and call insta::assert_snapshot! or
assert_display_snapshot!) so the package name lists and error messages are
stored as snapshots for easier maintenance. Ensure to import insta macros in the
tests module and keep existing test setup (tempdir, write_file, etc.) intact.

In `@crates/shipper-policy/src/lib.rs`:
- Around line 83-123: Add insta as a dev-dependency in the crate's Cargo.toml
(insta = "1.46.3"), import the macro in the test module with use
insta::assert_debug_snapshot;, and replace the three assert_eq! checks in the
tests safe_policy_respects_verify_ownership_and_readiness_flags,
balanced_policy_disables_ownership_enforcement, and
fast_policy_disables_safety_checks with assert_debug_snapshot!(effects); so each
test snapshot-asserts the result of evaluate(PolicyKind::..., ...), referencing
the evaluate function, PolicyKind enum, and PolicyEffects struct.

In `@crates/shipper-policy/tests/runtime_options_contract.rs`:
- Around line 36-81: Add the insta snapshot testing dependency and replace the
manual boolean assertions with snapshot assertions: add insta = "1.46" to
[dev-dependencies] in the crate's Cargo.toml (match workspace version), add use
insta::assert_debug_snapshot at the top of
crates/shipper-policy/tests/runtime_options_contract.rs, and in each test
(apply_policy_reads_runtime_options_for_safe_mode,
apply_policy_balanced_ignores_strict_ownership,
apply_policy_fast_disables_all_safety_checks) replace the multiple assert! calls
with a single assert_debug_snapshot!(effects) to snapshot the apply_policy
output created from sample_runtime_options and apply_policy.

In `@crates/shipper-schema/src/lib.rs`:
- Around line 6-15: The validation in parse_schema_version is too permissive
because parts[0].starts_with("shipper") accepts values like "shipperx"; change
the check to require an exact match (parts[0] == "shipper") and keep the
existing v-prefixed version check on parts[2]; then continue parsing
version_part as before (ensure you still slice off the leading 'v' from parts[2]
and parse into u32). This update should be made inside the parse_schema_version
function where parts, parts[0], and parts[2] are inspected.

In `@crates/shipper-sparse-index/src/lib.rs`:
- Around line 18-26: The function sparse_index_path panics on non-ASCII crate
names because it uses byte-range slicing like &lower[..2] and &lower[2..4];
change it to operate on chars (e.g., iterate lower.chars()) and build prefix
segments from characters rather than byte indices so multibyte UTF-8 chars don't
split; compute segments by collecting the first 0/1/2 chars and the next 2 chars
as needed (falling back to available chars for short names), then format the
same "0/", "1/{...}", "2/{...}", "3/{}/{...}", and "{}/{}/{}" patterns using
those char-derived strings to preserve behavior without panics in
sparse_index_path and any callers using lower.

In `@crates/shipper-types/tests/release_levels_integration.rs`:
- Around line 15-59: Add insta snapshot testing for the test
release_plan_groups_parallel_levels_from_shared_microcrate_logic: add insta to
dev-dependencies in crates/shipper-types/Cargo.toml, then replace the three
explicit assert_eq checks against levels with a single snapshot assertion that
serializes `levels` (the value returned from ReleasePlan::group_by_levels())
into a stable string (e.g., using Debug/serde JSON or Display) and call
insta::assert_snapshot! with that serialized output; keep the test name and use
the `levels` variable to generate the snapshot so future changes to
ReleasePlan::group_by_levels() are validated via insta snapshots.

In `@crates/shipper/src/lib.rs`:
- Around line 160-164: The feature-gated module swap causes two distinct
PlannedWorkspace types; fix by moving the PlannedWorkspace definition into a
shared types crate and making both implementations import and re-export that
single type so crate::plan::PlannedWorkspace is identical whether feature
"micro-plan" is enabled or not: create a new crate (e.g., shipper-types) that
defines PlannedWorkspace, update crates/shipper/src/plan.rs to use and re-export
shipper-types::PlannedWorkspace, update crates/shipper-plan to depend on and use
the same shipper-types::PlannedWorkspace (and have plan_micro.rs re-export it),
then keep engine.rs and engine_parallel.rs importing
crate::plan::PlannedWorkspace unchanged.

In `@crates/shipper/tests/parallel_levels_integration.rs`:
- Around line 93-117: The test currently uses multiple assert_eq! checks on the
levels variable to validate ordering; replace these with an insta snapshot
assertion by converting levels into a stable, human-readable representation (for
example map each level's packages to their names via levels.iter().map(|l|
l.packages.iter().map(|p|
p.name.as_str()).collect::<Vec<_>>()).collect::<Vec<_>>() or otherwise stringify
the levels) and call insta::assert_snapshot! on that serialized value so the
expected ordering is stored in a snapshot; update the test to import insta and
snapshot the derived Vec of package-name lists from levels (referencing the
levels variable and packages/name fields).
- Around line 1-7: The filesystem-mutating test that uses tempdir() and
write_file() must be marked with the serial test attribute to avoid concurrent
runs; add use serial_test::serial to the imports and annotate the test function
that performs the tempdir()/write_file() operations with #[serial] (place the
attribute immediately above the test fn declaration, e.g., the test that calls
tempdir()/write_file()), ensuring the test remains annotated alongside any
existing #[test] attribute.

In `@features/preflight_checks.feature`:
- Around line 45-49: The test Scenario "Balanced policy ignores strict ownership
requirement" currently uses the "--no-verify" flag which can short-circuit
ownership checks; either remove "--no-verify" from the When step that runs
"shipper preflight" so the preflight performs real ownership verification, or
keep the flag but add an explicit assertion that ownership verification was
executed and that it was bypassed due to the balanced policy (e.g. assert the
verification log/message or a specific exit-path indicator); update the Scenario
step referencing the command and the policy flags ("--policy balanced" and
"--strict-ownership") and add the extra assertion step that checks for the
ownership-verification outcome to ensure the test actually validates policy
behavior.

In `@fuzz/fuzz_targets/schema_version.rs`:
- Around line 10-11: Tests only fuzz the first parameter of
validate_schema_version because validate_schema_version calls
parse_schema_version on both the fuzz-controlled input and the hardcoded
minimum_supported value; the hardcoded side is never exercised. Modify the fuzz
target in schema_version.rs to split the incoming fuzz bytes into two slices
(e.g., via simple length prefix or half-splitting) and pass one slice to
shipper_schema::parse_schema_version and the other slice as the
minimum_supported argument to shipper_schema::validate_schema_version so both
parse_schema_version and the minimum_supported parsing are fuzz-exercised.

In `@fuzz/fuzz_targets/sparse_index.rs`:
- Around line 3-29: The fuzz harness builds JSON by embedding raw fuzzer strings
with format! which fails when inputs contain quotes or backslashes; update the
code that creates JSON lines (the mapping in content_lines and the guaranteed
string) to build JSON objects via serde_json (e.g., use the json! macro or
serde_json::to_string on a map) so values are properly escaped, add the
appropriate use/import for serde_json::json at the top, and replace the two
format! calls that produce "{\"vers\":\"...\"}" (used when building
content_lines and guaranteed_content) with serde_json-produced strings; keep all
existing variable names and calls to contains_version and sparse_index_path
unchanged.

---

Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 101-117: Replace the debug check command in the MSRV job: the step
currently named "Check with MSRV" runs "cargo check --workspace" — change it to
run "cargo build --workspace" (and optionally rename the step to "Build with
MSRV") so the MSRV job uses a debug build per the build-command guideline.