Skip to content

decrating: absorb shipper-policy into shipper::runtime::policy#54

Merged
EffortlessSteven merged 1 commit into
mainfrom
feature/decrating-absorb-policy
Apr 15, 2026
Merged

decrating: absorb shipper-policy into shipper::runtime::policy#54
EffortlessSteven merged 1 commit into
mainfrom
feature/decrating-absorb-policy

Conversation

@EffortlessSteven

Copy link
Copy Markdown
Member

Summary

Phase 2 of the decrating effort (per docs/decrating-plan.md §6): absorb the standalone shipper-policy microcrate into crates/shipper/src/runtime/policy/ as crate-private modules.

  • Move 1040 LOC of canonical policy logic from crates/shipper-policy/src/lib.rs into crates/shipper/src/runtime/policy/mod.rs (plus CLAUDE.md, renamed snapshot files).
  • Delete the stale 168-LOC duplicate at crates/shipper/src/policy.rs and the 7-LOC shim at crates/shipper/src/policy_micro.rs.
  • Inline integration tests (policy_bdd.rs, runtime_options_contract.rs, etc.) into runtime::policy's #[cfg(test)] modules.
  • Remove shipper-policy from workspace members, from shipper's deps, and drop the micro-policy feature flag on both shipper and shipper-cli.

Visibility: policy stays pub(crate) — it was internal-only before (as called out in the audit) and remains so.

Collateral

  • shipper-engine-parallel previously depended on shipper-policy. Since that crate is gone, inline a local PolicyEffects struct + policy_effects() function (~25 LOC) into shipper-engine-parallel/src/lib.rs. No behavior change.
  • fuzz/fuzz_targets/policy_effects.rs targeted shipper_policy::* directly. Deleted it — policy is no longer a public API surface, so it cannot be fuzzed across a crate boundary. Remove the corresponding [[bin]] and dep from fuzz/Cargo.toml.
  • One leftover #[cfg(feature = "micro-policy")] gate in crates/shipper-cli/tests/bdd_publish.rs dropped; the test now runs unconditionally.
  • Docs updated: docs/architecture.md, docs/testing.md, RELEASE_CHECKLIST_v0.3.0.md, and .github/workflows/mutation.yml no longer reference shipper-policy.
  • crates/shipper's dev-dependency on insta switched to { workspace = true } so YAML snapshot features (needed by the absorbed serialization_snapshot_tests) are available.

Test plan

  • cargo check --workspace — clean.
  • cargo test -p shipper runtime::policy — 75 tests pass.
  • cargo test -p shipper — all 837+ unit + integration tests pass.
  • cargo test -p shipper-cli — all pass.
  • cargo test -p shipper-engine-parallel — 65 pass (inlined PolicyEffects keeps snapshot parity).
  • cargo build -p shipper-cli — succeeds.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean.
  • cargo fmt --all -- --check — clean.

Conflict notes

  • crates/shipper/src/runtime/mod.rs is only touched here (runtime layer); other Phase 2 PRs target ops/ — no overlap.
  • Edits to crates/shipper/Cargo.toml, crates/shipper-cli/Cargo.toml, and root Cargo.toml are disjoint line edits vs. other Phase 2 PRs; git auto-merge should handle.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Apr 15, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@EffortlessSteven has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 28 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 28 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2ff725f6-9560-4d1a-9822-f8d4024f70d6

📥 Commits

Reviewing files that changed from the base of the PR and between 43cc3c3 and 54dbadf.

⛔ Files ignored due to path filters (34)
  • Cargo.lock is excluded by !**/*.lock
  • crates/shipper-policy/tests/snapshots/snapshot_tests__balanced.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__balanced_policy_default_flags.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__balanced_policy_no_verify.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__balanced_policy_readiness_disabled.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__fast.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__fast_policy_default_flags.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__fast_policy_ignores_all_overrides.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__policy_effects_display_formatting.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__safe.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__safe_policy_all_disabled.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__safe_policy_default_flags.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__safe_policy_no_verify_skips_dry_run.snap is excluded by !**/*.snap
  • crates/shipper-policy/tests/snapshots/snapshot_tests__safe_policy_skip_ownership_check.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__serialization_snapshot_tests__all_policy_kinds.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__serialization_snapshot_tests__balanced_policy_effects_default.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__serialization_snapshot_tests__fast_policy_effects_default.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__serialization_snapshot_tests__safe_policy_effects_default.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_balanced_all_flags_true.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_balanced_defaults.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_balanced_no_verify.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_balanced_readiness_disabled.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_fast_all_flags_false.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_fast_all_flags_true.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_fast_defaults.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_policy_kind_balanced.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_policy_kind_fast.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_policy_kind_safe.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_safe_all_disabled.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_safe_all_enabled.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_safe_all_flags_false.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_safe_all_flags_true.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_safe_no_verify.snap is excluded by !**/*.snap
  • crates/shipper/src/runtime/policy/snapshots/shipper__runtime__policy__snapshot_tests__snapshot_safe_skip_ownership.snap is excluded by !**/*.snap
📒 Files selected for processing (26)
  • .github/workflows/mutation.yml
  • Cargo.toml
  • RELEASE_CHECKLIST_v0.3.0.md
  • crates/shipper-cli/Cargo.toml
  • crates/shipper-cli/tests/bdd_publish.rs
  • crates/shipper-engine-parallel/Cargo.toml
  • crates/shipper-engine-parallel/src/lib.rs
  • crates/shipper-policy/CLAUDE.md
  • crates/shipper-policy/Cargo.toml
  • crates/shipper-policy/README.md
  • crates/shipper-policy/tests/policy_bdd.rs
  • crates/shipper-policy/tests/property_tests.rs
  • crates/shipper-policy/tests/runtime_options_contract.rs
  • crates/shipper-policy/tests/snapshot_tests.rs
  • crates/shipper/Cargo.toml
  • crates/shipper/src/engine.rs
  • crates/shipper/src/lib.rs
  • crates/shipper/src/policy.rs
  • crates/shipper/src/policy_micro.rs
  • crates/shipper/src/runtime/mod.rs
  • crates/shipper/src/runtime/policy/CLAUDE.md
  • crates/shipper/src/runtime/policy/mod.rs
  • docs/architecture.md
  • docs/testing.md
  • fuzz/Cargo.toml
  • fuzz/fuzz_targets/policy_effects.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/decrating-absorb-policy

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

Comment @coderabbitai help to get the list of available commands and usage tips.

EffortlessSteven added a commit that referenced this pull request Apr 15, 2026
Sweep-cleanup of CI workflows, templates, and docs for references to
microcrates and micro-* feature flags that have been deleted or are
queued for deletion via absorption PRs #52 #54 #55 #56 #57 #58.

Changes:
- .github/workflows/ci.yml - removed deleted features (micro-lock,
  micro-plan, micro-policy, micro-process, micro-store) from BDD matrix
- .github/workflows/mutation.yml - removed shipper-plan, shipper-policy,
  shipper-levels from the mutation-testing target list
- .github/workflows/release.yml - added note that publish order is
  finalized in Phase 8 of the decrating plan
- templates/circleci-config.yml - matching feature-matrix cleanup
- docs/architecture.md - per-crate absorbed notes in microcrates table
  plus top-of-section notes on Dependency Graph and Module
  Responsibilities pointing out those sections reflect pre-decrating state
- docs/testing.md - removed deleted crates from example test invocations
  and the mutation-testing example
- RELEASE_CHECKLIST_v0.3.0.md - header note listing absorbed crates and
  pruned publish steps
- RELEASE_NOTES_v0.3.0.md - reworded Modular Architecture bullet to
  reflect the consolidated public-crate layout

References to still-in-flight absorptions (auth, environment, git, storage,
engine-parallel, registry, progress) are left alone - they will be cleaned
up in their respective absorption PRs.

Per docs/decrating-plan.md.
Move shipper-policy crate (1040 LOC) into crates/shipper/src/runtime/policy/
as crate-private modules. Delete the stale 168-LOC duplicate at
crates/shipper/src/policy.rs and the 7-LOC shim at policy_micro.rs.
Remove shipper-policy from workspace members and from shipper's deps.
Drop the micro-policy feature flag.

policy stays pub(crate) - it was internal-only before and remains so.

Inline an equivalent PolicyEffects + policy_effects in shipper-engine-parallel
so it no longer depends on shipper-policy. Drop the fuzz/policy_effects
fuzz target since the crate it targeted no longer exists. Update mutation
testing workflow, architecture doc, and release checklist to match.

Per docs/decrating-plan.md section 6 Phase 2.
@EffortlessSteven EffortlessSteven force-pushed the feature/decrating-absorb-policy branch from 3be898e to 54dbadf Compare April 15, 2026 05:54
@EffortlessSteven EffortlessSteven merged commit f99aef6 into main Apr 15, 2026
12 of 15 checks passed
EffortlessSteven added a commit that referenced this pull request Apr 15, 2026
Stacks on PR #53 (env+storage+git dual-impl cleanup). Move the 2202-LOC
shipper-environment crate into crates/shipper/src/runtime/environment/
as crate-private modules (mod.rs + fingerprint.rs + ci.rs, ~2190 LOC
total). Preserves the deduped in-tree wrapper logic (normalize_version,
EnvironmentInfo::collect delegation with graceful fallback) from PR #53.

Split seams:
- mod.rs         — CiEnvironment, detect_environment, is_ci,
                   collect_environment_fingerprint (the absorbed shim),
                   CiEnvironment snapshot tests.
- fingerprint.rs — EnvironmentInfo, get_rust_version, get_cargo_version,
                   get_environment_fingerprint, normalize_tool_version,
                   collect_env_vars, tests + proptests.
- ci.rs          — get_ci_branch, get_ci_commit_sha, is_pull_request,
                   provider-specific tests.

All 13 snapshots preserved (renamed to `shipper__runtime__environment__
snapshot_tests__*.snap`). All 130 absorbed tests pass in-tree.

Surgical unblock (precedent: PR #54 policy→engine-parallel inline):
shipper-state depended on shipper-environment via one call site in
migrate_v1_to_v2. Because shipper-state is an (optional) dep of
shipper, it cannot depend on shipper itself — a dep cycle. Inlined an
~20-LOC collect_environment_fingerprint into shipper-state (marked
`#[doc(hidden)]`, with a comment pointing back at
crate::runtime::environment for the post-shipper-state-absorption
replacement), and dropped shipper-environment from shipper-state's
Cargo.toml. Will be replaced with a direct call to
crate::runtime::environment::collect_environment_fingerprint after
shipper-state is itself absorbed into shipper::state.

Module is `pub(crate) mod runtime; mod environment;` — the former
public `shipper::environment` API had no external consumers in-tree
and is now crate-private per the runtime-layer discipline.
`#[allow(dead_code)]` is applied at the runtime::environment level
because several absorbed items (CI branch/SHA/PR helpers, the short
pipe-fingerprint form, EnvironmentInfo::fingerprint) currently have
no in-crate callers but retain full test coverage for future wiring.

Also updates shipper's `insta` dev-dep to enable the `yaml` feature
(required by the moved snapshot_tests module).

Per docs/decrating-plan.md section 6 Phase 2.
EffortlessSteven added a commit that referenced this pull request Apr 15, 2026
Sweep-cleanup of CI workflows, templates, and docs for references to
microcrates and micro-* feature flags that have been deleted or are
queued for deletion via absorption PRs #52 #54 #55 #56 #57 #58.

Changes:
- .github/workflows/ci.yml - removed deleted features (micro-lock,
  micro-plan, micro-policy, micro-process, micro-store) from BDD matrix
- .github/workflows/mutation.yml - removed shipper-plan, shipper-policy,
  shipper-levels from the mutation-testing target list
- .github/workflows/release.yml - added note that publish order is
  finalized in Phase 8 of the decrating plan
- templates/circleci-config.yml - matching feature-matrix cleanup
- docs/architecture.md - per-crate absorbed notes in microcrates table
  plus top-of-section notes on Dependency Graph and Module
  Responsibilities pointing out those sections reflect pre-decrating state
- docs/testing.md - removed deleted crates from example test invocations
  and the mutation-testing example
- RELEASE_CHECKLIST_v0.3.0.md - header note listing absorbed crates and
  pruned publish steps
- RELEASE_NOTES_v0.3.0.md - reworded Modular Architecture bullet to
  reflect the consolidated public-crate layout

References to still-in-flight absorptions (auth, environment, git, storage,
engine-parallel, registry, progress) are left alone - they will be cleaned
up in their respective absorption PRs.

Per docs/decrating-plan.md.
EffortlessSteven added a commit that referenced this pull request Apr 15, 2026
Stacks on PR #53 (env+storage+git dual-impl cleanup). Move the 2202-LOC
shipper-environment crate into crates/shipper/src/runtime/environment/
as crate-private modules (mod.rs + fingerprint.rs + ci.rs, ~2190 LOC
total). Preserves the deduped in-tree wrapper logic (normalize_version,
EnvironmentInfo::collect delegation with graceful fallback) from PR #53.

Split seams:
- mod.rs         — CiEnvironment, detect_environment, is_ci,
                   collect_environment_fingerprint (the absorbed shim),
                   CiEnvironment snapshot tests.
- fingerprint.rs — EnvironmentInfo, get_rust_version, get_cargo_version,
                   get_environment_fingerprint, normalize_tool_version,
                   collect_env_vars, tests + proptests.
- ci.rs          — get_ci_branch, get_ci_commit_sha, is_pull_request,
                   provider-specific tests.

All 13 snapshots preserved (renamed to `shipper__runtime__environment__
snapshot_tests__*.snap`). All 130 absorbed tests pass in-tree.

Surgical unblock (precedent: PR #54 policy→engine-parallel inline):
shipper-state depended on shipper-environment via one call site in
migrate_v1_to_v2. Because shipper-state is an (optional) dep of
shipper, it cannot depend on shipper itself — a dep cycle. Inlined an
~20-LOC collect_environment_fingerprint into shipper-state (marked
`#[doc(hidden)]`, with a comment pointing back at
crate::runtime::environment for the post-shipper-state-absorption
replacement), and dropped shipper-environment from shipper-state's
Cargo.toml. Will be replaced with a direct call to
crate::runtime::environment::collect_environment_fingerprint after
shipper-state is itself absorbed into shipper::state.

Module is `pub(crate) mod runtime; mod environment;` — the former
public `shipper::environment` API had no external consumers in-tree
and is now crate-private per the runtime-layer discipline.
`#[allow(dead_code)]` is applied at the runtime::environment level
because several absorbed items (CI branch/SHA/PR helpers, the short
pipe-fingerprint form, EnvironmentInfo::fingerprint) currently have
no in-crate callers but retain full test coverage for future wiring.

Also updates shipper's `insta` dev-dep to enable the `yaml` feature
(required by the moved snapshot_tests module).

Per docs/decrating-plan.md section 6 Phase 2.
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