Plan zero-write Issue lifecycle restoration - #628
Conversation
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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: 058ab4c5b0
ℹ️ 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".
| unless labels.is_a?(Array) && labels.all? { |label| label.is_a?(String) && !label.empty? } && | ||
| labels.uniq.length == labels.length && (labels - label_names).empty? && | ||
| assignees.is_a?(Array) && assignees.all? { |login| github_login?(login) } && |
There was a problem hiding this comment.
In the plan-restore path, this validation only confirms that labels exist in the repository catalog, so an approved target containing no category/triage label—or multiple such labels—still produces a reverse plan and can pass post-state verification. The repository auditor at .github/scripts/issue-lifecycle-audit.rb:165-169 then rejects that resulting issue; validate exactly one of bug/enhancement and exactly one triage-state label before accepting the target.
AGENTS.md reference: AGENTS.md:L43-L46
Useful? React with 👍 / 👎.
| validate_plan_state!(operation["precondition"], number, issue_numbers, label_names, target: false) | ||
| validate_plan_state!(operation["target"], number, issue_numbers, label_names, target: true) |
There was a problem hiding this comment.
When a forward operation changes parent or blocked_by, the target body is accepted without checking that its ## Parent and ## Blocked by summaries describe those new native relationships. Post-state verification consequently accepts a state that the existing lifecycle auditor rejects at .github/scripts/issue-lifecycle-audit.rb:183-197; validate the target body summaries alongside the target relationship fields.
AGENTS.md reference: AGENTS.md:L47-L49
Useful? React with 👍 / 👎.
| valid_reason = value == "open" ? [nil, "reopened"].include?(reason) : | ||
| value == "closed" && [nil, "completed", "duplicate", "not_planned"].include?(reason) | ||
| valid_reason &&= !target || value != "closed" || !reason.nil? |
There was a problem hiding this comment.
This validates state.reason independently of the precondition, so it accepts an already-open issue whose target merely changes the reason to reopened, and it also generates the fixture's close-to-open reverse target with the original nil reason. GitHub derives reopened from reopening rather than allowing it to be assigned arbitrarily, so such targets cannot match the subsequent live capture and the advertised inverse for a close operation is not executable exactly; require reopened specifically for a closed-to-open transition and preserve the existing reason when the state does not change.
Useful? React with 👍 / 👎.
| operation_receipts << { | ||
| "comment_ids" => comment_ids, | ||
| "event_ids" => event_ids, | ||
| "number" => number, | ||
| "precondition_sha256" => Digest::SHA256.hexdigest(Canonical.dump(operation.fetch("precondition"))), | ||
| "result" => "passed", | ||
| "target_sha256" => Digest::SHA256.hexdigest(Canonical.dump(operation.fetch("target"))) |
There was a problem hiding this comment.
For an operation that transitions an issue carrying the runtime qualifier to closed, this code emits a passing operation receipt even when comment_ids and event_ids are empty and never inspects either the existing or appended comment contents. A runtime issue with no target/controller/candidate/rollback/stop-rule/result/cleanup record can therefore pass post-state and live verification after it has already been closed; validate the required closure evidence in the captured comment history before returning a passing receipt.
AGENTS.md reference: AGENTS.md:L54-L57
Useful? React with 👍 / 👎.
| targets[number] = Canonical.value(operation["target"]) | ||
| { "number" => number, "precondition" => operation["target"], "target" => operation["precondition"] } | ||
| end | ||
| validate_plan_edges!(targets) |
There was a problem hiding this comment.
After applying all target states, this only checks relationship cycles, so a target can close an issue carrying tracking while one of its direct sub-issues remains open, or without any separate parent acceptance audit, and the resulting post-state still verifies. The available targets graph already contains every direct child's planned state; reject premature parent closure and require the separate acceptance evidence instead of treating the forward operation itself as sufficient.
AGENTS.md reference: AGENTS.md:L54-L57
Useful? React with 👍 / 👎.
Change contract
Add a read-only
plan-restoreCLI that derives a complete inverse from an exact verified Issue-lifecycle preflight plus approved forward plan, then independently validates the saved reverse plan, post-state receipt/bundle, and live GitHub state before printing the same canonical reverse bytes.Issue relationship
Closes #609
Out of scope
Applying or authorizing any restoration; creating or mutating a retained evidence PR; running a full live repository capture/readback; correcting the 54 legacy violations; changing #459 execution state; changing #460, required checks, branch protection, deployment/runtime resources, or Agent-led research scope; adding rate-limit telemetry to the #607 v1 evidence schema.
Dependencies and merge order
#607 and #608 are already merged and closed. This PR is based on current
mainat5f55f3f4fe9a1bab5ca9040a458c967d39d1ecb7; no stacked dependency remains.Focused validation
duplicatestate reason, derived REST metadata, and cross-repository relationship collisions.bash .github/scripts/test-issue-lifecycle-preflight.sh.github/scripts/test-issue-lifecycle-audit.sh.github/scripts/test-issue-lifecycle-contract.shruby .github/scripts/test-issue-lifecycle-workflow.rbruby .github/scripts/test-issue-lifecycle-reconcile-workflow.rb.github/scripts/test-issue-lifecycle-status-reconcile.sh.github/scripts/test-select-rust-ci-scope.shshellcheck, scopedactionlint, andgit diff --checkapply.058ab4c5b066c6eeb3c07c9a0dd8b401439c12ee.Rollout and rollback
No deployment, runtime, branch-protection, or GitHub metadata mutation. Merge only adds a stdout-only planner/verifier; rollback is a revert of this PR. #459 remains
enhancement + needs-info + runtimeuntil a separate durable carrier contract exists.Scope exception
None. The final diff is 2 files and 741 changed lines, below both split thresholds.