Skip to content

feat(packaging): three-crate facade — cargo install shipper (#95)#123

Closed
EffortlessSteven wants to merge 2 commits into
mainfrom
feat/95-three-crate-facade
Closed

feat(packaging): three-crate facade — cargo install shipper (#95)#123
EffortlessSteven wants to merge 2 commits into
mainfrom
feat/95-three-crate-facade

Conversation

@EffortlessSteven

Copy link
Copy Markdown
Member

Summary

Close #95 by landing the three-crate façade layout for Shipper:

  • shipper-core — library (renamed from crates/shipper). All
    real publishing logic lives here. Published as shipper-core on
    crates.io.
  • shipper-cli — CLI implementation (unchanged conceptually).
    Its binary is renamed from [[bin]] name = "shipper" to
    shipper-cli so the façade can own the shipper name. Exposes
    pub fn run() as its single public entry point.
  • shipper — NEW UX façade. [[bin]] name = "shipper" with a
    three-line main.rs calling shipper_cli::run(); src/lib.rs is
    pub use shipper_core::*;. Published as shipper — operators
    can finally run cargo install shipper --locked.

User-facing goal achieved:

cargo install shipper --locked   # was: cargo install shipper-cli
shipper plan

Library API is unchanged (shipper = "0.3" → same re-exports).

Commits

  1. refactor(shipper-cli): split main.rs into lib.rs + thin main.rs
    prep commit; zero behavior change.
  2. refactor: rename crates/shipper -> crates/shipper-core + introduce facade
    the actual rename + facade creation + CI/doc/snapshot updates.

Dep alias pattern

shipper-cli/Cargo.toml aliases the core dep as shipper:

shipper = { package = "shipper-core", path = "../shipper-core", version = "0.3.0-rc.1" }

This keeps use shipper::… inside shipper-cli/src/lib.rs and its
integration tests working verbatim. Same trick in fuzz/Cargo.toml.

shipper-core's own integration tests live in the crate itself and
saw shipper::shipper_core:: rewrites (they're a separate
compilation unit and don't see Cargo's dep alias).

CI adjustments

  • .github/workflows/architecture-guard.yml: paths crates/shipper/src/**
    crates/shipper-core/src/** (10 sites).
  • .github/workflows/release.yml: builds -p shipper (façade) instead
    of -p shipper-cli, so the installed binary is the façade's
    shipper bin that calls shipper_cli::run(). CRATES publish list
    adds shipper-core and orders topologically: core → cli → facade.

Snapshot rebaseline

~330 files in crates/shipper-core/src/**/snapshots/ renamed from
shipper__…shipper_core__… (insta uses module_path!() which
follows the crate name). Orphans deleted; no test semantics changed.

Test plan

  • cargo build --workspace — clean.
  • cargo test -p shipper-core --lib — 1848 pass.
  • cargo test -p shipper-core --tests — 39+ pass.
  • cargo test -p shipper-cli — full suite green.
  • cargo test -p shipper — OK (no tests yet in façade — it's a
    thin re-export).
  • cargo clippy --workspace --all-targets --all-features — clean.
  • cargo fmt --all --check — clean.
  • target/debug/shipper --versionshipper 0.3.0-rc.1.
  • CI multi-OS green.

Prep commit for #95 three-crate facade. Zero behavior change:

- `src/main.rs` → `src/lib.rs`; `fn main()` → `pub fn run()`.
- new `src/main.rs` is a 3-line shim: `shipper_cli::run()`.
- lib-level doc comment notes the façade crate's role as the install
  entry point.

The seam exists so the upcoming `crates/shipper` façade crate can own
the `[[bin]] name = "shipper"` slot and call `shipper_cli::run()`
without duplicating CLI logic.
…cade (#95)

Part 2 of the three-crate facade layout for Shipper. This commit
physically moves the monolith library into `crates/shipper-core`
and introduces a new UX-focused `crates/shipper` crate whose only
job is to be the 'install me' entry point.

## New crate layout

- `crates/shipper-core` — library (renamed from `crates/shipper`,
  `package.name = "shipper-core"`). All real code lives here.
  Published as `shipper-core` on crates.io.
- `crates/shipper-cli` — unchanged conceptually. Its binary target
  is renamed from `[[bin]] name = "shipper"` to `shipper-cli`
  so the facade can own the `shipper` binary name. `pub fn run()`
  remains the sole public entry point (prep from the prior commit).
- `crates/shipper` — new facade. `[[bin]] name = "shipper"` with
  a three-line `main.rs` that calls `shipper_cli::run()`.
  `src/lib.rs` is `pub use shipper_core::*;` so library users
  doing `shipper = \"0.3\"` get exactly the same public API.
  Published as `shipper` — `cargo install shipper` works.

## Mechanical changes

- `git mv crates/shipper crates/shipper-core` preserves blame.
- Workspace `Cargo.toml` adds `crates/shipper-core` to `members`
  and `workspace.dependencies`.
- `shipper-cli/Cargo.toml` aliases the core dep as `shipper`
  (`shipper = { package = "shipper-core", path = "../shipper-core" }`)
  so existing `use shipper::...` imports inside the CLI and its
  tests continue to work verbatim. `workspace = true` can't combine
  with `package = "..."`, so path + version are explicit.
- `fuzz/Cargo.toml` uses the same alias trick so fuzz targets stay
  unchanged.
- `shipper-core/tests/**/*.rs` rewrote `shipper::` → `shipper_core::`
  (integration tests are a separate compilation unit and don't see
  Cargo's dep alias).
- `shipper-cli/tests/**/*.rs` rewrote `cargo_bin!("shipper")` →
  `cargo_bin!("shipper-cli")` since the CLI crate's bin renamed.
- Insta snapshot filenames rebaselined throughout
  `shipper-core/src/**/snapshots/` (module_path went from
  `shipper::...` to `shipper_core::...`). Orphans deleted.
- `.github/workflows/architecture-guard.yml` paths updated to
  `crates/shipper-core/src/**`.
- `.github/workflows/release.yml` switches every
  `cargo build -p shipper-cli` to `-p shipper` so the facade's
  `shipper` binary is what gets installed to `/usr/local/bin`.
  CRATES publish list adds `shipper-core` and reorders for topo
  publish order (core → cli → facade).

## User-facing impact

- `cargo install shipper --locked` now installs the `shipper`
  binary directly (operator UX goal of #95).
- Library consumers (`shipper = \"0.3\"`) get the same public
  API unchanged, re-exported verbatim from `shipper-core`.
- Operators who prefer pinning to the CLI crate directly can still
  `cargo install shipper-cli --locked` and get a `shipper-cli`
  binary on PATH.

Closes #95.
@coderabbitai

coderabbitai Bot commented Apr 17, 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 21 minutes and 33 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 21 minutes and 33 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: d74319aa-9443-405a-8175-af22f6b78614

📥 Commits

Reviewing files that changed from the base of the PR and between d8e7657 and 90cec90.

⛔ Files ignored due to path filters (190)
  • Cargo.lock is excluded by !**/*.lock
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__display_empty_name_and_version.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__display_multi_package_sequence.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__display_prerelease_version.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__display_single_package.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__percentage_large_workspace.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__percentage_milestones.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__percentage_single_package.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__percentage_three_packages.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__percentage_zero_total.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__state_after_first_package.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__state_fresh_reporter.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__state_full_lifecycle.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__state_overwrite_same_index.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__snapshot_tests__state_zero_packages.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_display_format_edge_cases.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_failed_midway_state.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_progress_at_0_percent.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_progress_at_100_percent.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_progress_at_25_percent.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_progress_at_50_percent.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_progress_at_75_percent.snap is excluded by !**/*.snap
  • crates/shipper-cli/src/output/progress/snapshots/shipper_cli__output__progress__tests__snapshot_single_package_lifecycle.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__ci_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__clean_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__completion_missing_shell.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__config_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__doctor_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__help_text.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__no_subcommand_error.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__plan_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__preflight_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__publish_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__resume_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__status_help.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/cli_snapshots__unknown_subcommand_error.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__error_missing_ci_subcommand.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__error_missing_completion_shell.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__error_missing_config_subcommand.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__error_unknown_subcommand.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_ci.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_ci_github_actions.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_ci_gitlab.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_clean.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_completion.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_config.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_config_init.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_config_validate.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_doctor.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_inspect_events.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_inspect_receipt.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_plan.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_preflight.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_publish.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_resume.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_root.snap is excluded by !**/*.snap
  • crates/shipper-cli/tests/snapshots/e2e_expanded__help_status.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_chunk_by_max_concurrent_basic.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_chunk_by_max_concurrent_empty.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_chunk_by_max_concurrent_larger_than_items.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_chunk_by_max_concurrent_one.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_execution_plan_diamond.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_execution_plan_independent_forest.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_execution_plan_linear_chain.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_execution_plan_wide_fan_out.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_parallel_config_default.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_policy_effects_balanced.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_policy_effects_fast.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/parallel/snapshots/shipper_core__engine__parallel__tests__snapshot_tests__snapshot_policy_effects_safe.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/snapshots/shipper_core__engine__tests__error_classification_matrix.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/snapshots/shipper_core__engine__tests__init_state_multi_package.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/snapshots/shipper_core__engine__tests__init_state_single_package.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/snapshots/shipper_core__engine__tests__sm_receipt_multi_package.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/snapshots/shipper_core__engine__tests__sm_state_partial_failure.snap is excluded by !**/*.snap
  • crates/shipper-core/src/engine/snapshots/shipper_core__engine__tests__state_after_mixed_transitions.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__error_message_snapshots__error_msg_credentials_not_found.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__error_message_snapshots__error_msg_empty_credentials.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__error_message_snapshots__error_msg_malformed_toml.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__error_message_snapshots__error_msg_token_wrong_registry.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_credentials_crates_io_registry_section.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_credentials_custom_registry_section.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_credentials_file_with_all_formats.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_credentials_legacy_toplevel_format.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_error_malformed_credentials_file.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_error_missing_credentials_file.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_error_token_not_found_for_registry.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_list_configured_registries_empty.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_list_configured_registries_mixed.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__credentials__tests__snapshots__snapshot_list_registries_many.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_auth_info_not_detected.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_auth_info_with_credentials_file.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_auth_info_with_env_default.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_auth_info_with_env_registry.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_mask_token_long_value.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_mask_token_short_values.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_resolve_unicode_token.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_resolve_whitespace_token.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_token_source_credentials_file.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_token_source_env_default.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_token_source_env_registry.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__edge_snapshots__snapshot_token_source_none.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__error_message_snapshots__error_msg_missing_token.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__error_message_snapshots__error_msg_token_source_none.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__snapshots__snapshot_auth_info_default.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__snapshots__snapshot_resolve_token_custom_registry_from_credentials.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__snapshots__snapshot_resolve_token_from_credentials_file.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__snapshots__snapshot_resolve_token_from_env_default.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__snapshots__snapshot_resolve_token_from_env_registry.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/auth/snapshots/shipper_core__ops__auth__resolver__tests__snapshots__snapshot_resolve_token_none_found.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/cargo/snapshots/shipper_core__ops__cargo__tests__snapshot_tests_absorbed__snapshot_invalid_package_names.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/cargo/snapshots/shipper_core__ops__cargo__tests__snapshot_tests_absorbed__snapshot_package_info_prerelease_version.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/cargo/snapshots/shipper_core__ops__cargo__tests__snapshot_tests_absorbed__snapshot_package_info_simple.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/cargo/snapshots/shipper_core__ops__cargo__tests__snapshot_tests_absorbed__snapshot_package_info_with_registries.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/cargo/snapshots/shipper_core__ops__cargo__tests__snapshot_tests_absorbed__snapshot_valid_package_names.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__clean_result_err_not_git.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__clean_result_err_uncommitted.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__clean_result_ok.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_all_fields.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_detached_head.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_dirty_detached.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_dirty_with_tag.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_feature_branch_clean.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_no_commit_no_branch.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__context_tagged_detached.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__is_dirty_all_variants.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__porcelain_empty_output.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__porcelain_parsed_files.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__porcelain_renamed_and_copied.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__porcelain_spaces_in_names.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__short_commit_empty_string.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__edge_case_snapshots__short_commit_exactly_eight.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__clean_working_tree.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__dirty_default_behavior.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__dirty_working_tree.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__ensure_git_clean_error.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__git_context_commit_only.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__git_context_dirty_no_tag.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__git_context_empty.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__git_context_full.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__git_rev_parse_failed_error.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__git_status_failed_error.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__short_commit_full_hash.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__short_commit_none.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__short_commit_seven_chars.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__short_commit_short_hash.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__tag_absent.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__tag_dirty_tree.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__tag_prerelease.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__tag_semver.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/git/snapshots/shipper_core__ops__git__context__snapshot_tests__unknown_dirty_state.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_info_all_fields.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_info_no_plan_id.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_state_corrupt.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_state_locked_no_plan.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_state_locked_with_plan.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_state_stale.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__edge_case_tests__lock_state_unlocked.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__hardened_tests__lock_file_after_set_plan_id.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__hardened_tests__lock_info_empty_plan_id.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__hardened_tests__lock_info_json_key_order.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__error_corrupt_lock_file_prefix.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__error_fresh_lock_with_timeout_prefix.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__error_lock_already_held.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__error_lock_already_held_with_plan_id.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__error_read_nonexistent_lock_prefix.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__error_set_plan_id_after_release_prefix.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_file_content_with_plan_id.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_file_content_without_plan_id.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_file_on_disk.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_info_debug.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_info_yaml.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_path_with_root_filename.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_path_without_root.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_status_locked.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/lock/snapshots/shipper_core__ops__lock__snapshot_tests__lock_status_unlocked.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_output_failure.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_output_json_format.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_output_success.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_output_timed_out.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_result_failure_no_exit_code.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_result_failure_with_exit_code.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_result_json_format.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_result_success.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__command_result_with_multiline_output.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__error_message_empty_stderr.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__error_message_with_exit_code.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__error_message_without_exit_code.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__snapshot_publish_args_basic.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__snapshot_publish_args_full_flags.snap is excluded by !**/*.snap
  • crates/shipper-core/src/ops/process/snapshots/shipper_core__ops__process__snapshot_tests__snapshot_publish_args_with_registry_and_no_verify.snap is excluded by !**/*.snap
📒 Files selected for processing (110)
  • .claude/worktrees/agent-a1ddd724
  • .claude/worktrees/agent-a2849897
  • .claude/worktrees/agent-a3b5001e
  • .claude/worktrees/agent-a4154c57
  • .claude/worktrees/agent-a48a6f62
  • .claude/worktrees/agent-a63fbc18
  • .claude/worktrees/agent-a6890464
  • .claude/worktrees/agent-a6aa80ce
  • .claude/worktrees/agent-a6c6afa7
  • .claude/worktrees/agent-a851615a
  • .claude/worktrees/agent-a89cb3c1
  • .claude/worktrees/agent-a89f5fc8
  • .claude/worktrees/agent-a8cd5c3e
  • .claude/worktrees/agent-a91cca46
  • .claude/worktrees/agent-a99dad25
  • .claude/worktrees/agent-aae70baf
  • .claude/worktrees/agent-aaf19b4c
  • .claude/worktrees/agent-aafaf1c5
  • .claude/worktrees/agent-ab07f0d7
  • .claude/worktrees/agent-ab6e6c0a
  • .claude/worktrees/agent-abd40c33
  • .claude/worktrees/agent-ac29c1aa
  • .claude/worktrees/agent-ac419106
  • .claude/worktrees/agent-ac671cf3
  • .claude/worktrees/agent-ac74df4f
  • .claude/worktrees/agent-ad022620
  • .claude/worktrees/agent-ad1ccb3c
  • .claude/worktrees/agent-ae489ac1
  • .claude/worktrees/agent-af8315cb
  • .claude/worktrees/agent-afa1f4c4
  • .claude/worktrees/agent-exec-core
  • .github/workflows/architecture-guard.yml
  • .github/workflows/release.yml
  • CURRENT_STATE.md
  • Cargo.toml
  • crates/shipper-cli/Cargo.toml
  • crates/shipper-cli/src/lib.rs
  • crates/shipper-cli/src/main.rs
  • crates/shipper-cli/tests/bdd_config.rs
  • crates/shipper-cli/tests/bdd_doctor.rs
  • crates/shipper-cli/tests/bdd_error_handling.rs
  • crates/shipper-cli/tests/bdd_error_recovery.rs
  • crates/shipper-cli/tests/bdd_micro_backends.rs
  • crates/shipper-cli/tests/bdd_parallel.rs
  • crates/shipper-cli/tests/bdd_preflight.rs
  • crates/shipper-cli/tests/bdd_publish.rs
  • crates/shipper-cli/tests/bdd_publish_advanced.rs
  • crates/shipper-cli/tests/bdd_publish_flow.rs
  • crates/shipper-cli/tests/bdd_resume.rs
  • crates/shipper-cli/tests/bdd_status.rs
  • crates/shipper-cli/tests/bdd_workflow.rs
  • crates/shipper-cli/tests/cli_e2e.rs
  • crates/shipper-cli/tests/cli_snapshots.rs
  • crates/shipper-cli/tests/e2e_config.rs
  • crates/shipper-cli/tests/e2e_doctor.rs
  • crates/shipper-cli/tests/e2e_expanded.rs
  • crates/shipper-cli/tests/e2e_plan.rs
  • crates/shipper-cli/tests/e2e_preflight.rs
  • crates/shipper-cli/tests/e2e_publish.rs
  • crates/shipper-cli/tests/e2e_resume.rs
  • crates/shipper-cli/tests/e2e_status.rs
  • crates/shipper-cli/tests/integration_config.rs
  • crates/shipper-core/CLAUDE.md
  • crates/shipper-core/Cargo.toml
  • crates/shipper-core/README.md
  • crates/shipper-core/proptest-regressions/property_tests.txt
  • crates/shipper-core/proptest-regressions/types.txt
  • crates/shipper-core/src/config.rs
  • crates/shipper-core/src/encryption.rs
  • crates/shipper-core/src/engine/CLAUDE.md
  • crates/shipper-core/src/engine/mod.rs
  • crates/shipper-core/src/engine/parallel/CLAUDE.md
  • crates/shipper-core/src/engine/parallel/mod.rs
  • crates/shipper-core/src/engine/parallel/policy.rs
  • crates/shipper-core/src/engine/parallel/publish.rs
  • crates/shipper-core/src/engine/parallel/readiness.rs
  • crates/shipper-core/src/engine/parallel/reconcile.rs
  • crates/shipper-core/src/engine/parallel/tests.rs
  • crates/shipper-core/src/engine/parallel/webhook.rs
  • crates/shipper-core/src/git.rs
  • crates/shipper-core/src/lib.rs
  • crates/shipper-core/src/ops/CLAUDE.md
  • crates/shipper-core/src/ops/auth/CLAUDE.md
  • crates/shipper-core/src/ops/auth/credentials.rs
  • crates/shipper-core/src/ops/auth/mod.rs
  • crates/shipper-core/src/ops/auth/oidc.rs
  • crates/shipper-core/src/ops/auth/resolver.rs
  • crates/shipper-core/src/ops/cargo/CLAUDE.md
  • crates/shipper-core/src/ops/cargo/mod.rs
  • crates/shipper-core/src/ops/git/CLAUDE.md
  • crates/shipper-core/src/ops/git/bin_override.rs
  • crates/shipper-core/src/ops/git/cleanliness.rs
  • crates/shipper-core/src/ops/git/context.rs
  • crates/shipper-core/src/ops/git/mod.rs
  • crates/shipper-core/src/ops/lock/CLAUDE.md
  • crates/shipper-core/src/ops/lock/mod.rs
  • crates/shipper-core/src/ops/mod.rs
  • crates/shipper-core/src/ops/process/CLAUDE.md
  • crates/shipper-core/src/ops/process/cargo.rs
  • crates/shipper-core/src/ops/process/cross_platform_edge_case_tests.rs
  • crates/shipper-core/src/ops/process/mod.rs
  • crates/shipper-core/src/ops/process/run.rs
  • crates/shipper-core/src/ops/process/snapshot_tests.rs
  • crates/shipper-core/src/ops/process/tests.rs
  • crates/shipper-core/src/ops/process/timeout.rs
  • crates/shipper-core/src/ops/process/types.rs
  • crates/shipper-core/src/ops/process/which.rs
  • crates/shipper-core/src/ops/storage/CLAUDE.md
  • crates/shipper-core/src/ops/storage/mod.rs
  • crates/shipper-core/src/plan/CLAUDE.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/95-three-crate-facade

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the workspace by renaming the core logic crate to shipper-core and moving the CLI implementation into a library module within shipper-cli. The shipper facade is updated to point to these new components, and documentation and test snapshots are refreshed to reflect the new structure. Feedback identifies a compilation error in match arms involving temporary strings, recommends replacing hardcoded ANSI escape sequences with the console crate for better terminal compatibility, and suggests updating documentation paths and examples to align with the crate renames. Additionally, a suggestion was made to avoid hardcoded versions in dependency aliases to simplify future maintenance.

Comment on lines +1159 to +1170
let state_str = match &p.state {
shipper::types::PackageState::Published => "\x1b[32mPublished\x1b[0m",
shipper::types::PackageState::Pending => "Pending",
shipper::types::PackageState::Uploaded => "\x1b[33mUploaded\x1b[0m",
shipper::types::PackageState::Skipped { reason } => &format!("Skipped: {}", reason),
shipper::types::PackageState::Failed { class, message } => {
&format!("\x1b[31mFailed ({:?}): {}\x1b[0m", class, message)
}
shipper::types::PackageState::Ambiguous { message } => {
&format!("\x1b[33mAmbiguous: {}\x1b[0m", message)
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This match statement has inconsistent return types in its arms and attempts to return a reference to a temporary value, which will not compile. Some arms return a &'static str, while others attempt to return a &String where the String is a temporary created by format!.

To fix this, all arms should return an owned String. This also provides an opportunity to replace the hardcoded ANSI color codes with the console crate.

Suggested change
let state_str = match &p.state {
shipper::types::PackageState::Published => "\x1b[32mPublished\x1b[0m",
shipper::types::PackageState::Pending => "Pending",
shipper::types::PackageState::Uploaded => "\x1b[33mUploaded\x1b[0m",
shipper::types::PackageState::Skipped { reason } => &format!("Skipped: {}", reason),
shipper::types::PackageState::Failed { class, message } => {
&format!("\x1b[31mFailed ({:?}): {}\x1b[0m", class, message)
}
shipper::types::PackageState::Ambiguous { message } => {
&format!("\x1b[33mAmbiguous: {}\x1b[0m", message)
}
};
let state_str = match &p.state {
shipper::types::PackageState::Published => console::style("Published").green().to_string(),
shipper::types::PackageState::Pending => "Pending".to_string(),
shipper::types::PackageState::Uploaded => console::style("Uploaded").yellow().to_string(),
shipper::types::PackageState::Skipped { reason } => format!("Skipped: {}", reason),
shipper::types::PackageState::Failed { class, message } => {
console::style(format!("Failed ({:?}): {}", class, message)).red().to_string()
}
shipper::types::PackageState::Ambiguous { message } => {
console::style(format!("Ambiguous: {}", message)).yellow().to_string()
}
};

Comment thread CURRENT_STATE.md
Comment on lines +69 to +72
- Root cause of past failures: grep matched forbidden-import examples inside `crates/shipper/src/ops/CLAUDE.md` (the CLAUDE.md documents the rule with `❌ use crate::engine::...` lines, which the guard scanned as real violations).
- **Fix merged in #85 (`ea75b53`):** added `--include='*.rs'` to every guard grep. Current `.github/workflows/architecture-guard.yml` on main is correct.
- **Why it still shows red in the runs list:** the guard's `paths:` filter restricts triggering to `crates/shipper/src/**`. No commit on main since #85 has touched that path — #86 only touched `crates/shipper-cli/tests/`. So the fixed guard has not had an opportunity to re-run and post a green status.
- **Risk:** low-confidence green. The fix is in place but unverified until the next commit that touches `crates/shipper/src/**` triggers it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The file paths mentioned in this document seem to be outdated after the shipper to shipper-core rename. For example, crates/shipper/src/ops/CLAUDE.md should likely be crates/shipper-core/src/ops/CLAUDE.md, and paths like crates/shipper/src/** should be updated to crates/shipper-core/src/** in this section and on line 138.

# after the upstream rename to `shipper-core` (#95). `workspace = true`
# can't combine with `package = "..."`, so pin path + version
# explicitly. Library API is re-exported unchanged.
shipper = { package = "shipper-core", path = "../shipper-core", version = "0.3.0-rc.1" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The hardcoded version 0.3.0-rc.1 for shipper-core could become a maintenance issue. When the workspace version is bumped, this will need to be updated manually here and in other places using this aliasing trick (like fuzz/Cargo.toml).

While aliasing with workspace = true isn't possible, a future refactoring could remove the alias and use shipper-core directly throughout this crate. This would allow you to use shipper-core.workspace = true and inherit the version from the workspace definition, removing the hardcoded version string.

Comment on lines +742 to +752
#[allow(clippy::collapsible_if)]
if let Some(deps) = ws.plan.dependencies.get(&p.name) {
if deps.len() > 3 {
issues.push(format!(
" - {}@{} has {} dependencies (may require longer publish time)",
p.name,
p.version,
deps.len()
));
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The collapsible_if lint is suppressed here, but the if statements can be combined. This improves readability by reducing nesting.

        if let Some(deps) = ws.plan.dependencies.get(&p.name) && deps.len() > 3 {
            issues.push(format!(
                "  - {}@{} has {} dependencies (may require longer publish time)",
                p.name,
                p.version,
                deps.len()
            ));
        }

Comment on lines +852 to +860
let (finishability_color, finishability_text) = match rep.finishability {
Finishability::Proven => ("\x1b[32m", "PROVEN"),
Finishability::NotProven => ("\x1b[33m", "NOT PROVEN"),
Finishability::Failed => ("\x1b[31m", "FAILED"),
};
println!(
"Finishability: {}{}",
finishability_color, finishability_text
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoded ANSI escape codes are used for coloring output. This can lead to issues on terminals that don't support colors or when output is redirected to a file. It also doesn't respect the NO_COLOR environment variable.

Since the console crate is already a dependency, it's better to use it for styling text. This will handle color support automatically.

This also applies to other parts of this function that use hardcoded colors (e.g., lines 939-952).

Suggested change
let (finishability_color, finishability_text) = match rep.finishability {
Finishability::Proven => ("\x1b[32m", "PROVEN"),
Finishability::NotProven => ("\x1b[33m", "NOT PROVEN"),
Finishability::Failed => ("\x1b[31m", "FAILED"),
};
println!(
"Finishability: {}{}",
finishability_color, finishability_text
);
let finishability_styled = match rep.finishability {
Finishability::Proven => console::style("PROVEN").green(),
Finishability::NotProven => console::style("NOT PROVEN").yellow(),
Finishability::Failed => console::style("FAILED").red(),
};
println!("Finishability: {}", finishability_styled);

Comment on lines +30 to +33
use shipper::config::{CliOverrides, ShipperConfig};
use shipper::engine::{self, Reporter};
use shipper::plan;
use shipper::types::{Registry, ReleaseSpec};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since this README is for the shipper-core crate, the example should probably use shipper_core in the use statements to reflect how it would be used when depended upon directly. A user might depend on shipper-core to avoid the dependencies of the shipper facade.

Suggested change
use shipper::config::{CliOverrides, ShipperConfig};
use shipper::engine::{self, Reporter};
use shipper::plan;
use shipper::types::{Registry, ReleaseSpec};
use shipper_core::config::{CliOverrides, ShipperConfig};
use shipper_core::engine::{self, Reporter};
use shipper_core::plan;
use shipper_core::types::{Registry, ReleaseSpec};

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90cec907c8

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/shipper/Cargo.toml
sha2 = "0.11"
[dependencies]
shipper-core.workspace = true
shipper-cli.workspace = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make shipper-cli optional for library consumers

The new façade crate declares shipper-cli as an unconditional dependency even though crates/shipper/src/lib.rs only re-exports shipper_core::* and never uses CLI code. This means every downstream project that depends on shipper as a library now has to compile the full CLI stack (clap, progress UI, etc.), which is a measurable build-time/perf regression and increases dependency-surface risk for non-CLI use cases. Please gate shipper-cli behind a bin-only feature (or equivalent) so library consumers only pull shipper-core.

Useful? React with 👍 / 👎.

@EffortlessSteven EffortlessSteven marked this pull request as draft April 17, 2026 13:20
@EffortlessSteven

Copy link
Copy Markdown
Member Author

Parking this PR as draft per sequencing feedback: three-crate facade was the wrong shape for #95, and #95 itself must wait until #97 (rehearsal-registry proof) and #98 (remediation) land first.

The correct #95 direction is narrower: keep shipper as the main library/product, add a [[bin]] target to the shipper package itself (with CLI code as a thin adapter), leave shipper-cli as a temporary compatibility shim. Package-level unification, architectural separation preserved — no shipper-core facade invention.

Will revisit after #97 + #98 land and #95 issue text is rewritten to the narrower shape.

@EffortlessSteven

Copy link
Copy Markdown
Member Author

#95 reframe — narrower shape (captured)

After session review, the three-crate facade shape in this PR was the wrong direction. Parking it here as a reference implementation; the real #95 should be rewritten from the narrower shape below. Keeping this comment on the PR so future work starts from the corrected premise instead of re-deriving it.

The narrower shape

  • Keep `shipper` as the main library/product facade (no separate `shipper-core`).
  • Add a `shipper` binary target to the `shipper` package. So `shipper` has `[lib]` + `[[bin]]` side by side.
  • Keep CLI code isolated as a thin adapter inside `shipper` (e.g. `shipper::cli` module).
  • Keep `shipper-cli` as a temporary compatibility shim during migration — trivial `[[bin]]` that delegates to `shipper::cli::run()`.

Package-level unification, architectural separation preserved.

Why this is better than the three-crate facade

  • One public product story. Library users do `shipper = "0.3"`, operators do `cargo install shipper`. Single crate to learn, document, search, cite.
  • No cycles. The three-crate facade (`shipper-core` lib + `shipper-cli` CLI + `shipper` facade) kept tripping over cyclic dep risk because the facade depended on shipper-cli which depended on the library. A single crate with a `cli` submodule sidesteps this entirely.
  • No secretly-hidden core. `shipper-core` as a publicly meaningful crate was the wrong long-term story — it hands the audience a decision (`shipper` or `shipper-core`?) they shouldn't have to make.
  • Smaller churn. One physical rename (`shipper-cli/src/*` → `shipper/src/cli/`) instead of moving 570+ files.
  • Clean compatibility path. `cargo install shipper-cli` keeps working during migration; the shim forwards. Can be deprecated + removed at a future minor.

Sequencing

Still after #97 and #98 land. The issue has always been packaging polish, not a prerequisite — and the remediation pillar (#98) + prove tier 2 (#97) are the remaining release-closure gaps that need to close first.

Implementation sketch when the time comes

  1. `shipper/src/cli/` — move CLI code here from `shipper-cli/src/main.rs` + `output/`. Expose `pub fn run() -> Result<()>`.
  2. `shipper/src/main.rs` — three-line shim: `fn main() { shipper::cli::run() }`.
  3. `shipper/Cargo.toml` — add `[[bin]] name = "shipper"` alongside the existing `[lib]`.
  4. `shipper-cli/src/main.rs` — reduces to `fn main() { shipper::cli::run() }`.
  5. `shipper-cli/Cargo.toml` — rename `[[bin]]` to `shipper-cli` to avoid workspace-build collision with shipper's new `shipper` bin. Drop transitive deps that move into `shipper`.
  6. Tests: `shipper-cli/tests/` → `shipper/tests/cli_` (they exercise the binary, which now lives in shipper).
  7. Docs: README + install instructions switch to `cargo install shipper`.
  8. Snapshot rebaseline (lots of help-text snapshots will shift slightly).

What this PR stays useful for

  • The `shipper-cli/src/main.rs` → `lib.rs` extraction with `pub fn run()` is the same first step. Cherry-pickable as PR A when implementation begins.
  • The README / Cargo.toml / architecture-guard changes are NOT reusable — those targeted `shipper-core`.

Leaving this as draft indefinitely. Close when the reframed #95 lands.

EffortlessSteven added a commit that referenced this pull request Apr 18, 2026
Reframed packaging UX. Supersedes the parked three-crate facade (#123).

## What changes

- **\`shipper\`** becomes a single public crate with both \`[lib]\` and
  \`[[bin]]\`. The CLI lives at \`shipper::cli\` (module, pub) and the
  binary target at \`src/bin/shipper.rs\` is a three-line shim over
  \`shipper::cli::run()\`.
- **\`shipper-cli\`** stays published as a compatibility shim:
  \`[[bin]] name = \"shipper-cli\"\` that forwards into
  \`shipper::cli::run()\`. Operators on the old name keep working;
  new setups should \`cargo install shipper --locked\`.
- README, docs, release workflow updated to prefer
  \`cargo install shipper\` everywhere.

## Why this shape

- **One public product story.** Library users \`shipper = \"0.3\"\`,
  operators \`cargo install shipper\`. One crate to learn, document,
  cite.
- **No dep cycles.** The earlier three-crate \`shipper-core\` + \`shipper\`
  + \`shipper-cli\` facade kept tripping cycle risk because the facade
  depended on the CLI which depended on the library. A single crate
  with a \`cli\` submodule sidesteps this.
- **No hidden core.** \`shipper-core\` as a publicly meaningful crate
  would have forced consumers to choose between \`shipper\` and
  \`shipper-core\` for no gain.
- **Smaller churn than the three-crate attempt.** One directory move
  (\`shipper-cli/src/\` → \`shipper/src/cli/\`) + bin rename, vs the
  ~570-file shuffle the earlier attempt produced.

## Mechanical changes

- \`shipper-cli/src/main.rs\` + \`shipper-cli/src/output/\` moved under
  \`shipper/src/cli/\` (git mv preserves history). \`fn main\` became
  \`pub fn run\`. All \`use shipper::...\` rewrote to \`use crate::...\`.
- \`shipper/Cargo.toml\` gains \`[[bin]] name = \"shipper\"\` + the CLI's
  deps (clap, clap_complete, indicatif).
- \`shipper-cli/Cargo.toml\` reduces to a shim: \`[[bin]] name = \"shipper-cli\"\`,
  \`src/main.rs\` is three lines.
- \`shipper-cli/tests/\` preserved (25 files, extensive coverage). Every
  \`cargo_bin!(\"shipper\")\` → \`cargo_bin!(\"shipper-cli\")\`. Tests now
  exercise the shim, which internally calls the same \`shipper::cli::run\`
  — so functional coverage is identical.
- Help-text snapshots regenerated: \`Usage: shipper-cli\` when invoked
  via the shim (correct behavior — it IS shipper-cli when you run it
  that way). \`cli::output::progress\` snapshots moved to their new
  module path under \`shipper/src/cli/\`.
- Workspace \`default-members\` reordered (\`shipper\` first).
- Release workflow builds \`-p shipper\` instead of \`-p shipper-cli\`,
  installs \`target/release/shipper\` to PATH (facade bin name).
- Docs (README, release-runbook, run-in-github-actions, tutorials,
  failure-modes, preflight, tech) all switch to
  \`cargo install shipper --locked\`.

## Acceptance

- [x] \`cargo install --path crates/shipper --locked\` installs a working
      \`shipper\` binary.
- [x] \`cargo install --path crates/shipper-cli --locked\` still works
      (produces a \`shipper-cli\` binary via the shim).
- [x] \`shipper --version\` and \`shipper-cli --version\` both print the
      version.
- [x] Library imports (\`use shipper::engine;\` etc.) unchanged.
- [x] \`cargo clippy --workspace --all-targets --all-features -- -D warnings\` clean.
- [x] \`cargo doc -p shipper --no-deps\` clean.
- [x] \`cargo fmt --all --check\` clean.
@EffortlessSteven

Copy link
Copy Markdown
Member Author

Superseded by #135 (the reframed #95). Closing this one.

EffortlessSteven added a commit that referenced this pull request Apr 18, 2026
* feat(packaging): unify shipper CLI into the shipper crate (#95)

Reframed packaging UX. Supersedes the parked three-crate facade (#123).

## What changes

- **\`shipper\`** becomes a single public crate with both \`[lib]\` and
  \`[[bin]]\`. The CLI lives at \`shipper::cli\` (module, pub) and the
  binary target at \`src/bin/shipper.rs\` is a three-line shim over
  \`shipper::cli::run()\`.
- **\`shipper-cli\`** stays published as a compatibility shim:
  \`[[bin]] name = \"shipper-cli\"\` that forwards into
  \`shipper::cli::run()\`. Operators on the old name keep working;
  new setups should \`cargo install shipper --locked\`.
- README, docs, release workflow updated to prefer
  \`cargo install shipper\` everywhere.

## Why this shape

- **One public product story.** Library users \`shipper = \"0.3\"\`,
  operators \`cargo install shipper\`. One crate to learn, document,
  cite.
- **No dep cycles.** The earlier three-crate \`shipper-core\` + \`shipper\`
  + \`shipper-cli\` facade kept tripping cycle risk because the facade
  depended on the CLI which depended on the library. A single crate
  with a \`cli\` submodule sidesteps this.
- **No hidden core.** \`shipper-core\` as a publicly meaningful crate
  would have forced consumers to choose between \`shipper\` and
  \`shipper-core\` for no gain.
- **Smaller churn than the three-crate attempt.** One directory move
  (\`shipper-cli/src/\` → \`shipper/src/cli/\`) + bin rename, vs the
  ~570-file shuffle the earlier attempt produced.

## Mechanical changes

- \`shipper-cli/src/main.rs\` + \`shipper-cli/src/output/\` moved under
  \`shipper/src/cli/\` (git mv preserves history). \`fn main\` became
  \`pub fn run\`. All \`use shipper::...\` rewrote to \`use crate::...\`.
- \`shipper/Cargo.toml\` gains \`[[bin]] name = \"shipper\"\` + the CLI's
  deps (clap, clap_complete, indicatif).
- \`shipper-cli/Cargo.toml\` reduces to a shim: \`[[bin]] name = \"shipper-cli\"\`,
  \`src/main.rs\` is three lines.
- \`shipper-cli/tests/\` preserved (25 files, extensive coverage). Every
  \`cargo_bin!(\"shipper\")\` → \`cargo_bin!(\"shipper-cli\")\`. Tests now
  exercise the shim, which internally calls the same \`shipper::cli::run\`
  — so functional coverage is identical.
- Help-text snapshots regenerated: \`Usage: shipper-cli\` when invoked
  via the shim (correct behavior — it IS shipper-cli when you run it
  that way). \`cli::output::progress\` snapshots moved to their new
  module path under \`shipper/src/cli/\`.
- Workspace \`default-members\` reordered (\`shipper\` first).
- Release workflow builds \`-p shipper\` instead of \`-p shipper-cli\`,
  installs \`target/release/shipper\` to PATH (facade bin name).
- Docs (README, release-runbook, run-in-github-actions, tutorials,
  failure-modes, preflight, tech) all switch to
  \`cargo install shipper --locked\`.

## Acceptance

- [x] \`cargo install --path crates/shipper --locked\` installs a working
      \`shipper\` binary.
- [x] \`cargo install --path crates/shipper-cli --locked\` still works
      (produces a \`shipper-cli\` binary via the shim).
- [x] \`shipper --version\` and \`shipper-cli --version\` both print the
      version.
- [x] Library imports (\`use shipper::engine;\` etc.) unchanged.
- [x] \`cargo clippy --workspace --all-targets --all-features -- -D warnings\` clean.
- [x] \`cargo doc -p shipper --no-deps\` clean.
- [x] \`cargo fmt --all --check\` clean.

* test: strip .exe from shipper-cli binary name in help snapshots

Fixes CI failures on Ubuntu + macOS where the bin has no .exe suffix.
The normalize_output / normalize_stderr helpers were stripping
`shipper.exe` but not `shipper-cli.exe`, so my Windows-regenerated
snapshots leaked the platform-specific suffix.

Order matters in the rewrite: strip `shipper-cli.exe` → `shipper-cli`
first, else the subsequent `shipper.exe` → `shipper` rule would eat
the `-cli` suffix after trimming `.exe`.
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.

Packaging UX: make 'cargo install shipper' work by collapsing binary into shipper package

1 participant