feat(remediate): plan-yank + compromised-release receipt fields (#98 PR 2)#132
Conversation
…ds (#98 PR 2) Next step on the Remediate pillar. PR 1 (#121) landed the single-crate \`shipper yank\` primitive; this PR composes it into a reverse-topological containment plan built from a prior publish receipt. ## What's new - **Receipt schema** (additive): PackageReceipt gains three Option fields, all \`#[serde(default, skip_serializing_if = \"Option::is_none\")]\` so existing receipts read back unchanged: - \`compromised_at: Option<DateTime<Utc>>\` — when this version was marked compromised - \`compromised_by: Option<String>\` — operator-supplied reason (CVE, incident, free-form tag) - \`superseded_by: Option<String>\` — populated by \`shipper fix-forward\` (#98 PR 3) - **\`shipper::engine::plan_yank\`** module: - \`PlanYankFilter::{AllPublished, CompromisedOnly}\` selector - \`YankPlan\`, \`YankEntry\` value types - \`build_plan(receipt, filter)\` — filter receipt.packages, reverse to get dependents-first order, emit entries - \`render_text(plan)\` — produces copy-pasteable \`shipper yank ...\` command lines, one per entry, with \`--reason <REASON>\` placeholders for the operator to fill in - \`load_receipt_from_path\` — receipt loader for arbitrary paths (default is \`<state_dir>/receipt.json\`) - **\`shipper plan-yank\` CLI subcommand**: \`\`\` shipper plan-yank [--from-receipt PATH] [--compromised-only] \`\`\` Honors the global \`--format\` flag: \`text\` (default) prints the copy-pasteable yank-command list; \`json\` prints structured YankPlan for scripting. ## Scope — explicitly NOT in this PR - **Plan execution** (wrapping \`shipper yank\` to run each entry) → #98 PR 3 along with fix-forward. - **\`--mark-compromised\`** (populating the new receipt fields programmatically) → #98 PR 3. For now, operators can mark packages compromised by hand-editing receipt.json or via downstream tooling. - **release_compromised state** (higher-level "this release is a write-off") → #98 PR 3. ## Why reverse topological, specifically For workspace A → B → C (A is a leaf, B depends on A, C depends on B): - Publish order: A, B, C - **Yank order: C, B, A** — dependents first. New resolves for C will still pull B (if B is not yet yanked), but at least C itself is unresolvable. Yanking A first would be fine semantically but can mislead operators who read \`cargo search\` output. Plus: reverse-topo matches the containment mental model users bring from incident response — "pull the thing furthest from your reach first, work back toward the root." ## Tests 5 unit tests in \`engine::plan_yank::tests\` (all green): - reverses publish order for \`AllPublished\` - excludes Failed/Skipped packages (only Published gets a yank entry) - \`CompromisedOnly\` filter drops healthy packages, keeps only flagged - empty receipt → empty plan with a helpful comment - text-render ordering invariant: dependents appear before dependencies Plus a CLI \`plan-yank --help\` snapshot in \`e2e_expanded\`. ## Companion churn Mechanical: every PackageReceipt construction site (engine + state tests + integration tests) now sets the three new Option fields to None. The schema is additive; no runtime semantics changed for existing code paths.
|
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 43 minutes and 17 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 (4)
📒 Files selected for processing (16)
✨ 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: 3d6ace06de
ℹ️ 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".
| pub fn build_plan(receipt: &Receipt, filter: PlanYankFilter) -> YankPlan { | ||
| let mut entries: Vec<YankEntry> = receipt | ||
| .packages | ||
| .iter() | ||
| .filter(|p| include(p, filter)) |
There was a problem hiding this comment.
Detect partial receipts before generating yank order
build_plan assumes receipt.packages is the complete publish sequence and simply reverses it. That breaks on resume-generated receipts in sequential mode, where already-terminal packages are intentionally omitted from the new receipt; in that case this can produce an empty/incomplete yank plan even though versions were published and may need containment. Add a guard for truncated resume receipts (or reconstruct from state) before deriving rollback order so operators don’t miss affected crates.
Useful? React with 👍 / 👎.
| Commands::PlanYank { | ||
| from_receipt, | ||
| compromised_only, | ||
| } => { |
There was a problem hiding this comment.
Handle plan-yank before workspace planning
This branch runs only after main() has already built a workspace release plan and loaded workspace config, so shipper plan-yank --from-receipt <path> still requires a valid Cargo workspace even though the command logic only reads a receipt file. That blocks incident-response usage from archived receipts or non-workspace directories. Treat PlanYank as an early command (like Config/Completion) so receipt-only planning works as documented.
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.
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 second \`plan-yank\` selector mode: **given a broken crate name, walk the workspace's dependency graph to find every crate that transitively depends on it, and emit a containment plan scoped to that affected chain.** Complements the existing \`--compromised-only\` (marker-based) selector. ## What's new - **\`plan_yank::build_plan_from_starting_crate\`**: takes a receipt, a dep-graph (\`BTreeMap<String, Vec<String>>\`), a starting crate, and an optional \`reason\`. Returns a \`YankPlan\` in reverse-topo order covering the starting crate + every transitive dependent. Errors if the starting crate isn't in the receipt. - **CLI**: \`--starting-crate <CRATE>\` and \`--reason <TEXT>\` on \`plan-yank\`. clap \`conflicts_with = \"starting_crate\"\` on \`--compromised-only\` rejects combinations at parse time. - CLI handler reads \`planned.plan.dependencies\` from the current workspace metadata for the graph walk. ## Algorithm Reverse BFS through the dep map: for each currently-reachable crate, scan the map for crates whose dep list contains it; add those to the frontier. Final set = starting crate + all transitive dependents. Then filter the receipt's topo-ordered packages to that set, keep only \`Published\` entries (Failed/never-shipped yank is a no-op), and reverse to get dependents-first order. ## Tests 5 new unit tests (10 total in \`plan_yank::tests\`): - walks all transitive dependents in reverse topo - ignores unrelated crates (no edge → not in plan) - skips non-Published entries (dependent that Failed doesn't yank) - \`--reason\` applies to every entry (falls back to receipt \`compromised_by\` otherwise) - errors when starting crate isn't in receipt Regenerated \`plan-yank --help\` snapshot. ## Closes half of #98's remaining gap \`plan-yank --starting-crate <name> --reason <text>\` now matches #98's issue spec verbatim. Combined with the earlier \`--compromised-only\` flow (#132), plan-yank fully satisfies the graph vs receipt-filter ask. Plan execution wrappers for yank/fix-forward are the remaining piece (#98 PR 5).
Closes the remaining plan-execution gap in #98. \`shipper plan-yank --format json\` emits a YankPlan; \`shipper yank --plan <PATH>\` walks that plan and invokes \`cargo yank\` per entry. ## What's new - \`YankEntry\` / \`YankPlan\` now derive \`Deserialize\` so plans round-trip via \`--format json\` → file → \`--plan\`. The \`filter\` field moves from \`&'static str\` to \`Cow<'static, str>\` so owned strings coming off disk don't require allocation in the construction path. - \`plan_yank::load_plan_from_path\` — thin helper mirroring \`load_receipt_from_path\`. - CLI: \`shipper yank --plan <PATH>\`. Mutually exclusive with \`--crate\` / \`--version\` / \`--reason\` (clap \`conflicts_with\`); those become optional in plan mode. - Handler: iterates entries in order, emits \`PackageYanked\` event per invocation, halts on first non-zero cargo exit (reverse-topo plan means every downstream entry would be failing to yank a dependent of a crate we just failed on — continuing is damage). ## Scope — what this PR does and doesn't **Does:** - Execute yank plans end-to-end. - Correct halt-on-failure semantics. - Events-as-truth: every invocation writes a PackageYanked event (with real exit code) to events.jsonl, including failures. **Does not:** - Resume interrupted plan runs. First yank is destructive on the registry; re-running the plan with the same reason is idempotent (cargo yank of an already-yanked version is a no-op), so the simplest fix is to just re-run the plan, and operator discipline picks up the rest. - Add a \`fix-forward\` executor. That requires Cargo.toml version bumps which are workspace-edit territory — deserves its own PR with --dry-run / --apply / git-cleanliness guards. ## Tests 3 new unit tests (8 total in \`plan_yank::tests\`): - YankPlan JSON round-trips: serialize, write, load → entries + order preserved, per-entry reason preserved. - \`load_plan_from_path\` errors cleanly on missing file. - \`load_plan_from_path\` errors cleanly on malformed JSON. Regenerated \`yank --help\` snapshot. ## Closes the rest of #98's open gap With PR #138 (starting-crate) and this one, #98's acceptance checklist is fulfilled: - [x] Receipt fields (compromised_at/compromised_by/superseded_by) — PR #132 - [x] plan-yank --from-receipt + --starting-crate + --reason — PR #132 + #138 - [x] shipper yank executes a plan — **this PR** - [ ] shipper fix-forward --plan executes a plan — follow-on (needs Cargo.toml edit tooling) - [x] Docs distinguish yank semantics — PR #134 The fix-forward executor is the last remaining #98 piece. It's cleanly deferred because it adds workspace-edit scope; this PR keeps the execution story for yank self-contained.
Adds the second \`plan-yank\` selector mode: **given a broken crate name, walk the workspace's dependency graph to find every crate that transitively depends on it, and emit a containment plan scoped to that affected chain.** Complements the existing \`--compromised-only\` (marker-based) selector. ## What's new - **\`plan_yank::build_plan_from_starting_crate\`**: takes a receipt, a dep-graph (\`BTreeMap<String, Vec<String>>\`), a starting crate, and an optional \`reason\`. Returns a \`YankPlan\` in reverse-topo order covering the starting crate + every transitive dependent. Errors if the starting crate isn't in the receipt. - **CLI**: \`--starting-crate <CRATE>\` and \`--reason <TEXT>\` on \`plan-yank\`. clap \`conflicts_with = \"starting_crate\"\` on \`--compromised-only\` rejects combinations at parse time. - CLI handler reads \`planned.plan.dependencies\` from the current workspace metadata for the graph walk. ## Algorithm Reverse BFS through the dep map: for each currently-reachable crate, scan the map for crates whose dep list contains it; add those to the frontier. Final set = starting crate + all transitive dependents. Then filter the receipt's topo-ordered packages to that set, keep only \`Published\` entries (Failed/never-shipped yank is a no-op), and reverse to get dependents-first order. ## Tests 5 new unit tests (10 total in \`plan_yank::tests\`): - walks all transitive dependents in reverse topo - ignores unrelated crates (no edge → not in plan) - skips non-Published entries (dependent that Failed doesn't yank) - \`--reason\` applies to every entry (falls back to receipt \`compromised_by\` otherwise) - errors when starting crate isn't in receipt Regenerated \`plan-yank --help\` snapshot. ## Closes half of #98's remaining gap \`plan-yank --starting-crate <name> --reason <text>\` now matches #98's issue spec verbatim. Combined with the earlier \`--compromised-only\` flow (#132), plan-yank fully satisfies the graph vs receipt-filter ask. Plan execution wrappers for yank/fix-forward are the remaining piece (#98 PR 5).
Closes the remaining plan-execution gap in #98. \`shipper plan-yank --format json\` emits a YankPlan; \`shipper yank --plan <PATH>\` walks that plan and invokes \`cargo yank\` per entry. - \`YankEntry\` / \`YankPlan\` now derive \`Deserialize\` so plans round-trip via \`--format json\` → file → \`--plan\`. The \`filter\` field moves from \`&'static str\` to \`Cow<'static, str>\` so owned strings coming off disk don't require allocation in the construction path. - \`plan_yank::load_plan_from_path\` — thin helper mirroring \`load_receipt_from_path\`. - CLI: \`shipper yank --plan <PATH>\`. Mutually exclusive with \`--crate\` / \`--version\` / \`--reason\` (clap \`conflicts_with\`); those become optional in plan mode. - Handler: iterates entries in order, emits \`PackageYanked\` event per invocation, halts on first non-zero cargo exit (reverse-topo plan means every downstream entry would be failing to yank a dependent of a crate we just failed on — continuing is damage). **Does:** - Execute yank plans end-to-end. - Correct halt-on-failure semantics. - Events-as-truth: every invocation writes a PackageYanked event (with real exit code) to events.jsonl, including failures. **Does not:** - Resume interrupted plan runs. First yank is destructive on the registry; re-running the plan with the same reason is idempotent (cargo yank of an already-yanked version is a no-op), so the simplest fix is to just re-run the plan, and operator discipline picks up the rest. - Add a \`fix-forward\` executor. That requires Cargo.toml version bumps which are workspace-edit territory — deserves its own PR with --dry-run / --apply / git-cleanliness guards. 3 new unit tests (8 total in \`plan_yank::tests\`): - YankPlan JSON round-trips: serialize, write, load → entries + order preserved, per-entry reason preserved. - \`load_plan_from_path\` errors cleanly on missing file. - \`load_plan_from_path\` errors cleanly on malformed JSON. Regenerated \`yank --help\` snapshot. With PR #138 (starting-crate) and this one, #98's acceptance checklist is fulfilled: - [x] Receipt fields (compromised_at/compromised_by/superseded_by) — PR #132 - [x] plan-yank --from-receipt + --starting-crate + --reason — PR #132 + #138 - [x] shipper yank executes a plan — **this PR** - [ ] shipper fix-forward --plan executes a plan — follow-on (needs Cargo.toml edit tooling) - [x] Docs distinguish yank semantics — PR #134 The fix-forward executor is the last remaining #98 piece. It's cleanly deferred because it adds workspace-edit scope; this PR keeps the execution story for yank self-contained.
…#139) Closes the remaining plan-execution gap in #98. \`shipper plan-yank --format json\` emits a YankPlan; \`shipper yank --plan <PATH>\` walks that plan and invokes \`cargo yank\` per entry. - \`YankEntry\` / \`YankPlan\` now derive \`Deserialize\` so plans round-trip via \`--format json\` → file → \`--plan\`. The \`filter\` field moves from \`&'static str\` to \`Cow<'static, str>\` so owned strings coming off disk don't require allocation in the construction path. - \`plan_yank::load_plan_from_path\` — thin helper mirroring \`load_receipt_from_path\`. - CLI: \`shipper yank --plan <PATH>\`. Mutually exclusive with \`--crate\` / \`--version\` / \`--reason\` (clap \`conflicts_with\`); those become optional in plan mode. - Handler: iterates entries in order, emits \`PackageYanked\` event per invocation, halts on first non-zero cargo exit (reverse-topo plan means every downstream entry would be failing to yank a dependent of a crate we just failed on — continuing is damage). **Does:** - Execute yank plans end-to-end. - Correct halt-on-failure semantics. - Events-as-truth: every invocation writes a PackageYanked event (with real exit code) to events.jsonl, including failures. **Does not:** - Resume interrupted plan runs. First yank is destructive on the registry; re-running the plan with the same reason is idempotent (cargo yank of an already-yanked version is a no-op), so the simplest fix is to just re-run the plan, and operator discipline picks up the rest. - Add a \`fix-forward\` executor. That requires Cargo.toml version bumps which are workspace-edit territory — deserves its own PR with --dry-run / --apply / git-cleanliness guards. 3 new unit tests (8 total in \`plan_yank::tests\`): - YankPlan JSON round-trips: serialize, write, load → entries + order preserved, per-entry reason preserved. - \`load_plan_from_path\` errors cleanly on missing file. - \`load_plan_from_path\` errors cleanly on malformed JSON. Regenerated \`yank --help\` snapshot. With PR #138 (starting-crate) and this one, #98's acceptance checklist is fulfilled: - [x] Receipt fields (compromised_at/compromised_by/superseded_by) — PR #132 - [x] plan-yank --from-receipt + --starting-crate + --reason — PR #132 + #138 - [x] shipper yank executes a plan — **this PR** - [ ] shipper fix-forward --plan executes a plan — follow-on (needs Cargo.toml edit tooling) - [x] Docs distinguish yank semantics — PR #134 The fix-forward executor is the last remaining #98 piece. It's cleanly deferred because it adds workspace-edit scope; this PR keeps the execution story for yank self-contained.
Re-opened after #121 merge auto-closed #129.
Summary
Next step on the Remediate pillar. #121 landed the single-crate `shipper yank` primitive; this PR composes it into a reverse-topological containment plan derived from a prior publish receipt.
What's new
Closes
Continuation of the #98 Remediate staged rollout. PR 1 (#121) → PR 2 (this) → PR 3 (#131, stacked).
See #129 for full original description (auto-closed when the base branch was deleted during #121 merge).