feat(rehearsal): config + CLI flag plumbing for rehearsal registry (#97 PR 1)#120
Conversation
… PR 1) Lays the config and CLI surface for the phase-2 rehearsal-registry proof. Plumbing only — no behavior change. The phase-2 execution (packaging + alt-registry publish + install/smoke checks + live-dispatch gate) lands in follow-on PRs under #97. This PR's goal is to pin the config/CLI interface so downstream PRs can thread values without schema churn. ## What lands **shipper-config:** - New `RehearsalConfig` struct: `enabled: bool`, `registry: Option<String>`. - Parses `[rehearsal]` section of `.shipper.toml`. Default disabled (opt-in until execution PR lands). - Added as `rehearsal: RehearsalConfig` field on `ShipperConfig` with `#[serde(default)]` for backward-compat with existing configs. - `CliOverrides` gains `rehearsal_registry: Option<String>` and `skip_rehearsal: bool` fields. Stored on `CliOverrides` but not yet threaded into `RuntimeOptions` — done in a follow-on PR with the execution logic so the engine sees the values when it's ready to act on them. **shipper-cli:** - New clap flags on the global CLI: `--rehearsal-registry <name>` and `--skip-rehearsal`. Plumb into `CliOverrides`. - Rustdoc on each flag cross-references #97 and notes the phase-2 status. ## Why no RuntimeOptions change yet `RuntimeOptions` has no `Default` impl and has 60+ construction sites across the workspace (mostly tests). Threading a new field now would create churn for a feature with no consumer. The execution PR that adds the actual rehearsal dispatch logic will add the `RuntimeOptions` field in the same PR — the change stays coherent. ## Verification - `cargo clippy --workspace --all-targets -- -D warnings` clean - `cargo fmt --check` clean - 4 new unit tests in shipper-config::tests: - `rehearsal_defaults_are_disabled_and_empty` - `rehearsal_section_parses_enabled_with_registry_name` - `rehearsal_section_partial_parses_with_field_defaults` - `rehearsal_cli_overrides_default_to_empty` - shipper-config lib: 235 tests pass (no regressions) - shipper engine tests: 125 pass (no regressions) - Snapshot updates accepted for configs that now include the default rehearsal section (snapshot_default_config, snapshot_config_all_fields, snapshot_default_shipper_config_debug, snapshot_empty_toml_parsed, snapshot_toml_roundtrip_parsed). ## Related - Parent issue: #97 Rehearsal registry (Prove tier 2) - Scout plan on #97: #97 (comment) - Master roadmap: #109
|
Warning Rate limit exceeded
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 24 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (35)
📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces the configuration and CLI plumbing for the 'rehearsal' feature, which allows publishing to a temporary registry for verification before live dispatch. It adds the RehearsalConfig struct to shipper-config, corresponding CLI flags to shipper-cli, and updates relevant tests and snapshots. Feedback was provided to derive PartialEq and Eq for the new configuration struct to improve testability and simplify assertions in unit tests.
| /// enabled = true | ||
| /// registry = "kellnr-local" # name must match an entry in [[registries]] | ||
| /// ``` | ||
| #[derive(Debug, Clone, Serialize, Deserialize, Default)] |
There was a problem hiding this comment.
For better testability and easier comparisons, consider deriving PartialEq and Eq for RehearsalConfig. This would allow you to use assert_eq! on the whole struct in tests, making them more concise and readable.
For example, the rehearsal_defaults_are_disabled_and_empty test could be simplified to:
assert_eq!(config.rehearsal, RehearsalConfig::default());| #[derive(Debug, Clone, Serialize, Deserialize, Default)] | |
| #[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)] |
…gistry / --skip-rehearsal flags
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Phase-2 preflight execution. PR 1 (#120) added config + CLI flag plumbing; this PR makes the flags actually DO something — publish every crate in the plan to an alternate registry, verify visibility, and emit a `RehearsalComplete` event so the outcome is auditable from `events.jsonl` alone. ## What's new - **`EventType` variants** (shipper-types): - `RehearsalStarted { registry, package_count }` - `RehearsalPackagePublished { name, version, duration_ms }` - `RehearsalPackageFailed { name, version, class, message }` - `RehearsalComplete { passed, registry, summary }` - **`RuntimeOptions` fields** (shipper-types): - `rehearsal_registry: Option<String>` — resolved from CLI `--rehearsal-registry` override OR `[rehearsal] enabled=true + registry=...` in `.shipper.toml`. None = rehearsal disabled. - `rehearsal_skip: bool` — operator override (loud-warning path). - **`ShipperConfig::build_runtime_options`** populates the new fields with correct precedence (CLI > config > None). - **`engine::run_rehearsal`** (shipper crate): - Fails clean if no rehearsal registry is configured. - Fails clean if the rehearsal registry isn't in `opts.registries`. - Refuses to rehearse against the same registry as the live target (the whole point is a sandbox). - Honors `--skip-rehearsal` with a warning; returns non-passing outcome so the eventual hard gate (PR 3) can detect it. - Sequential (parallel rehearsal not planned — correctness > speed). - Stops at first failure, emits `RehearsalPackageFailed` + `RehearsalComplete { passed: false, ... }`. - Uses the SAME `version_exists` readiness mechanism as live publish for the post-publish visibility check. - Does NOT touch `state.json` — rehearsal is a pre-publish proof, not an execution. Event log only. - **`shipper rehearse` CLI subcommand** that drives `run_rehearsal` and exits non-zero on failure so CI lanes can gate on it. ## Scope — explicitly NOT in this PR - **Hard gate** wiring `run_publish` → refuse without passing rehearsal (#97 PR 3). The event log carries `RehearsalComplete { passed }` ready for it, but the live path doesn't check yet. - **Install / smoke check** against the rehearsal registry (likely PR 3 or 4; needs a consumer-workspace fixture). - **Parallel rehearsal** (not planned). - **plan_id binding** of rehearsal outcome (needed by PR 3's hard gate; simpler to add alongside the gate itself). ## Tests 6 unit tests in `engine::tests`: - Errors cleanly with no rehearsal registry. - Errors cleanly when rehearsal == live target. - Errors cleanly when the named registry isn't configured. - `--skip-rehearsal` returns immediately, no events written. - Happy path emits Started + PackagePublished + Complete(passed=true). - Cargo-failure path emits PackageFailed + Complete(passed=false). CLI snapshots for `help_text`, `help_root`, `no_subcommand_error` regenerated to reflect the new `rehearse` subcommand.
* feat(yank): shipper yank <crate>@<version> (#98 PR 1) First primitive of the Remediate pillar (#98). Wraps `cargo yank` as containment, not undo — existing lockfile pins stay pinned; the yank only prevents NEW resolves against that version. - ops/cargo: cargo_yank(workspace, name, version, registry, lines, timeout) mirrors the cargo_publish invariants (non-default registry passthrough, timeout polling, output redaction + tailing via output-sanitizer). - CLI: `shipper yank --crate <name> --version <ver> --reason <why>` emits a PackageYanked event to <state_dir>/events.jsonl capturing the reason and the cargo exit code; surfaces a loud operator warning about containment semantics and the `cargo update -p <name>` remediation step for downstream consumers. - types: PackageYanked event variant (crate_name, version, reason, exit_code). - tests: three fake-cargo unit tests in ops/cargo (args passthrough, crates-io registry omission, non-zero exit propagation) + a yank --help snapshot in e2e_expanded. Scope-limited on purpose: follow-on PRs under #98 compose this into `shipper plan-yank` (reverse-topological containment plan) and `shipper fix-forward` (patch-and-republish). * test(yank): regenerate CLI help snapshots after PR #120 landed rehearsal flags
Phase-2 preflight execution. PR 1 (#120) added config + CLI flag plumbing for rehearsal registry; this PR makes the flags actually DO something — publish every crate in the plan to an alternate registry, verify visibility, and emit a RehearsalComplete event so the outcome is auditable from events.jsonl alone. - EventType variants: RehearsalStarted / RehearsalPackagePublished / RehearsalPackageFailed / RehearsalComplete - RuntimeOptions fields: rehearsal_registry, rehearsal_skip - ShipperConfig::build_runtime_options populates with CLI > config > None precedence - engine::run_rehearsal: sequential, stops at first failure, emits events, does not touch state.json (pre-publish proof, not execution), verifies post-publish visibility via the same mechanism live publish uses, refuses to rehearse against the live target registry - shipper rehearse CLI subcommand Also regenerates the matching CLI snapshots (help_text, help_root, no_subcommand_error).
…2) (#127) Phase-2 preflight execution. PR 1 (#120) added config + CLI flag plumbing for rehearsal registry; this PR makes the flags actually DO something — publish every crate in the plan to an alternate registry, verify visibility, and emit a RehearsalComplete event so the outcome is auditable from events.jsonl alone. - EventType variants: RehearsalStarted / RehearsalPackagePublished / RehearsalPackageFailed / RehearsalComplete - RuntimeOptions fields: rehearsal_registry, rehearsal_skip - ShipperConfig::build_runtime_options populates with CLI > config > None precedence - engine::run_rehearsal: sequential, stops at first failure, emits events, does not touch state.json (pre-publish proof, not execution), verifies post-publish visibility via the same mechanism live publish uses, refuses to rehearse against the live target registry - shipper rehearse CLI subcommand Also regenerates the matching CLI snapshots (help_text, help_root, no_subcommand_error).
First of four staged PRs for #97 Rehearsal registry per the scout plan. Plumbing only — no behavior change. Pins the config/CLI interface so follow-on PRs can thread values without schema churn.
What this adds
`shipper-config`:
`shipper-cli`:
Why no `RuntimeOptions` change yet
`RuntimeOptions` has no `Default` impl and has 60+ construction sites across the workspace. Threading a new field now would churn every test fixture for a feature with no consumer. The follow-on PR that adds the actual rehearsal dispatch logic will add the `RuntimeOptions` field in the same PR — the change stays coherent.
Verification
Out of scope (follow-on PRs)
Related