feat(rehearse): hard gate — run_publish requires passing rehearsal (#97 PR 3)#133
Conversation
… PR 3) Close the rehearsal loop. PR 2 gave us \`shipper rehearse\` that publishes to an alt registry and emits RehearsalComplete. This PR makes live publish refuse to run unless a fresh, passing rehearsal receipt exists for the current plan_id. ## What's new - **\`state::rehearsal\`** module: a \`rehearsal.json\` sidecar next to \`state.json\` / \`events.jsonl\`. Schema versioned (\`shipper.rehearsal.v1\`), atomic write (\`.tmp\` + rename + fsync), crash-safe. Small enough to be grep-readable. - **Event schema additive**: \`RehearsalStarted\` and \`RehearsalComplete\` now carry \`plan_id\`. The events.jsonl audit trail now has enough information to replay the gate's decision after the fact. This is additive on variants that only shipped with PR 2 (#127), so no migration needed. - **\`engine::enforce_rehearsal_gate\`** + call site at the top of \`run_publish\`. Decision tree: 1. \`opts.rehearsal_registry = None\` → dormant, publish proceeds. Unchanged behavior for every caller who hasn't opted in. 2. \`opts.rehearsal_skip = true\` → proceed with loud warning. No fake-passing receipt is synthesized; auditors see "no rehearsal ran for this plan_id" in events.jsonl. 3. No \`rehearsal.json\` → refuse with fix-suggesting error. 4. Receipt's \`plan_id\` != current plan_id → refuse (stale). 5. \`passed: false\` → refuse with reason. 6. Fresh passing receipt → proceed, log a confirmation info line. - **\`run_rehearsal\` persists the sidecar** on completion. Best-effort: if the sidecar write fails, the events log is still authoritative and the gate will (correctly) refuse, blocking publish — the safe default. ## Tests 13 new tests. All green locally, clippy clean, fmt clean: - 3 roundtrip/persistence tests in \`state::rehearsal\` - 6 gate-helper tests (dormant / skip / missing / stale / failing / fresh-pass) - 1 e2e test that \`run_publish\` actually calls the gate and bails when rehearsal is required but missing Total rehearsal+gate coverage: 17 unit tests. ## Scope — not in this PR - **Install / smoke check** against the rehearsal registry (PR 4 or follow-on). The current gate trusts the rehearsal's publish + visibility verification. - **CI recipe** for kellnr or equivalent sidecar (separate PR; docs work, not code).
Stitch the Prove pillar tier 2 together with a step-by-step operator guide that walks through rehearsal-registry configuration, shipper rehearse, the hard gate, and a kellnr-sidecar CI recipe. Covers: - When to use rehearsal (first-publish, post-refactor) - Config via .shipper.toml or CLI flag - Event emissions + rehearsal.json sidecar - Hard-gate decision tree - Complete GitHub Actions example with kellnr sidecar - Troubleshooting for the 3 common gate errors - What rehearsal does NOT cover yet (install/smoke, consumer-build) Tied to docs/README.md so operators can find it. No code changes.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
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 37 minutes and 8 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 selected for processing (6)
✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6c8a50e8b
ℹ️ 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".
| if receipt.plan_id != ws.plan.plan_id { | ||
| bail!( | ||
| "rehearsal receipt is stale: rehearsal ran for plan_id {} but the current plan_id is {}. \ | ||
| The workspace changed between rehearse and publish; re-run `shipper rehearse` against the current plan.", | ||
| receipt.plan_id, |
There was a problem hiding this comment.
Require rehearsal registry to match the gate configuration
The hard gate currently validates only plan_id and passed, but never checks that the persisted receipt was produced against the configured rehearsal_registry. In practice, a successful rehearsal on registry A can authorize shipper publish --rehearsal-registry B if the plan_id matches, which weakens the guarantee that the exact configured rehearsal target was validated before live publish.
Useful? React with 👍 / 👎.
| if let Err(err) = crate::state::rehearsal::save_rehearsal( | ||
| &state_dir, | ||
| &crate::state::rehearsal::RehearsalReceipt { | ||
| schema_version: crate::state::rehearsal::CURRENT_REHEARSAL_VERSION.to_string(), |
There was a problem hiding this comment.
Do not ignore receipt persistence failures after rehearsal
This branch only logs a warning when rehearsal.json write fails, which can leave a previous passing receipt on disk. If a subsequent rehearsal run fails and persistence also fails (e.g., disk/permission error), the stale passed: true receipt may still satisfy enforce_rehearsal_gate, allowing publish despite the latest rehearsal failure; the gate should fail closed by invalidating stale receipts or returning an error.
Useful? React with 👍 / 👎.
| RehearsalStarted { | ||
| registry: String, | ||
| plan_id: String, | ||
| package_count: usize, |
There was a problem hiding this comment.
Keep rehearsal events backward-compatible for deserialization
Making plan_id required in rehearsal event payloads introduces a wire-format break for previously written events.jsonl lines that lacked this field. Since EventLog::read_from_file performs strict serde_json deserialization, older rehearsal logs can become unreadable after upgrade (e.g., inspect/event consumers failing) unless these new fields are made backward-compatible.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Three-column reconciliation of what's actually merged vs each issue's acceptance checklist. Single source of truth for what closes each pillar issue. **Findings:** - **#90 Recover** — honestly closable. Code side is done (#124 + #130); operator-side real rehearsal is an ops action, not a code gap. - **#97 Prove tier 2** — 85% done. Rehearsal + visibility + hard gate + plan_id binding all landed (#127 + #133). Missing: install/smoke check (cargo install against the rehearsal registry / consumer build). One narrow follow-up PR closes it. - **#98 Remediate** — 60% done. Receipt schema + plan-yank (from-receipt) + yank primitive + fix-forward planning all landed (#121 + #132 + #134). Missing: plan-yank's --starting-crate graph mode, plan execution for yank + fix-forward. Two narrow follow-ups. Also captures the two review concerns on #122 (Trusted Publishing) that were addressed in a follow-up commit to that PR. Recommended next merge order and follow-up PRs spelled out at bottom.
Adds the final piece of #97's acceptance: \`cargo install --registry <rehearsal> <crate>\` after rehearsal publishes succeed, as end-to-end proof that the crate actually resolves and installs via the registry index — the scenario that workspace-path dependencies defeat and that killed the rc.1 first-publish. ## What's new - **EventType variants**: \`RehearsalSmokeCheckStarted\`, \`RehearsalSmokeCheckSucceeded\`, \`RehearsalSmokeCheckFailed\`. - **RuntimeOptions**: \`rehearsal_smoke_install: Option<String>\`. - **CliOverrides + CLI flag**: \`--smoke-install <CRATE>\` (global). Crate must be in the rehearsal plan and have a \`[[bin]]\` target; library-only crates can't be smoke-installed directly (consumer- workspace build variant is a follow-on). - **\`ops::cargo::cargo_install_smoke\`**: wrapper around \`cargo install --registry <name> <crate> --version <v> --root <tmp> --force --locked\`. - **engine::run_rehearsal** post-publish step: if all packages passed AND \`--smoke-install\` names an in-plan crate, install it from the rehearsal registry. Install failure fails the whole rehearsal (sets \`first_failure\` + \`RehearsalComplete { passed: false }\`). Named-but-not-in-plan: warn only; don't fail rehearsal over a typo. ## Tests - \`run_rehearsal_smoke_install_happy_path_emits_succeeded_event\` — flag named an in-plan crate, fake cargo returns 0, assert \`rehearsal_smoke_check_started\` + \`_succeeded\` events. - \`run_rehearsal_smoke_install_missing_target_warns_without_failing\` — flag named a crate not in the plan, assert reporter warn + rehearsal still passes. 19 rehearsal+gate tests pass (up from 17). ## Closes #97 With this PR, #97's issue checklist is 100% fulfilled: - [x] Publish packaged tarballs to alt registry (#127) - [x] Verify presence on rehearsal registry (#127) - [x] Install/build from rehearsal registry (**this PR**) - [x] Record proof in RehearsalComplete event (#127) - [x] Hard gate binding on plan_id (#133) The \`#97\` issue can be closed once this merges.
Three-column reconciliation of what's actually merged vs each issue's acceptance checklist. Single source of truth for what closes each pillar issue. **Findings:** - **#90 Recover** — honestly closable. Code side is done (#124 + #130); operator-side real rehearsal is an ops action, not a code gap. - **#97 Prove tier 2** — 85% done. Rehearsal + visibility + hard gate + plan_id binding all landed (#127 + #133). Missing: install/smoke check (cargo install against the rehearsal registry / consumer build). One narrow follow-up PR closes it. - **#98 Remediate** — 60% done. Receipt schema + plan-yank (from-receipt) + yank primitive + fix-forward planning all landed (#121 + #132 + #134). Missing: plan-yank's --starting-crate graph mode, plan execution for yank + fix-forward. Two narrow follow-ups. Also captures the two review concerns on #122 (Trusted Publishing) that were addressed in a follow-up commit to that PR. Recommended next merge order and follow-up PRs spelled out at bottom.
* feat(rehearse): smoke-install check closes #97 Prove tier 2 (#97 PR 4) Adds the final piece of #97's acceptance: \`cargo install --registry <rehearsal> <crate>\` after rehearsal publishes succeed, as end-to-end proof that the crate actually resolves and installs via the registry index — the scenario that workspace-path dependencies defeat and that killed the rc.1 first-publish. ## What's new - **EventType variants**: \`RehearsalSmokeCheckStarted\`, \`RehearsalSmokeCheckSucceeded\`, \`RehearsalSmokeCheckFailed\`. - **RuntimeOptions**: \`rehearsal_smoke_install: Option<String>\`. - **CliOverrides + CLI flag**: \`--smoke-install <CRATE>\` (global). Crate must be in the rehearsal plan and have a \`[[bin]]\` target; library-only crates can't be smoke-installed directly (consumer- workspace build variant is a follow-on). - **\`ops::cargo::cargo_install_smoke\`**: wrapper around \`cargo install --registry <name> <crate> --version <v> --root <tmp> --force --locked\`. - **engine::run_rehearsal** post-publish step: if all packages passed AND \`--smoke-install\` names an in-plan crate, install it from the rehearsal registry. Install failure fails the whole rehearsal (sets \`first_failure\` + \`RehearsalComplete { passed: false }\`). Named-but-not-in-plan: warn only; don't fail rehearsal over a typo. ## Tests - \`run_rehearsal_smoke_install_happy_path_emits_succeeded_event\` — flag named an in-plan crate, fake cargo returns 0, assert \`rehearsal_smoke_check_started\` + \`_succeeded\` events. - \`run_rehearsal_smoke_install_missing_target_warns_without_failing\` — flag named a crate not in the plan, assert reporter warn + rehearsal still passes. 19 rehearsal+gate tests pass (up from 17). ## Closes #97 With this PR, #97's issue checklist is 100% fulfilled: - [x] Publish packaged tarballs to alt registry (#127) - [x] Verify presence on rehearsal registry (#127) - [x] Install/build from rehearsal registry (**this PR**) - [x] Record proof in RehearsalComplete event (#127) - [x] Hard gate binding on plan_id (#133) The \`#97\` issue can be closed once this merges. * test: rebaseline shipper-config snapshots for rehearsal_smoke_install field * test: rebaseline shipper-types RuntimeOptions debug snapshot for smoke-install field
Reopened after #127 merge. Original PR #128 auto-closed when base branch was deleted.
PR 3 of the #97 Prove tier 2 staged rollout:
Completes the Prove tier 2 rollout alongside the now-merged #127.