Resolve PR blockers in the current worktree - #31
Conversation
Publish the current branch independently of base-branch lag and reconcile resolver state in place without temporary worktrees or branch switching.
Record the merged base ancestry after PR #30 so the new pull request contains only the shipping-pr policy changes.
|
Warning Review limit reached
Next review available in: 10 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 (10)
📝 WalkthroughWalkthroughChangesCurrent Worktree Shipping
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant shipping-pr
participant ResolverAgents
participant skill-set-pr
participant PRRemote
shipping-pr->>ResolverAgents: Resolve blockers in current worktree
ResolverAgents->>skill-set-pr: Record resolver outcome
skill-set-pr->>PRRemote: Publish through expected-SHA gate
PRRemote-->>skill-set-pr: Return publication result
skill-set-pr-->>shipping-pr: Continue or return stale to polling
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/skill-set/skills/shipping-pr/scripts/skill-set-pr (2)
1353-1374: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRequire evidence before accepting
stale.
stale:pollingcurrently depends only on the caller-provided label. A resolver can discard its result and consume a retry even when the live PR HEAD is unchanged. Tiestaleto a verified remote-HEAD change or a validated publicationhead_changedfailure, and require the publication phase to still be pending.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/skill-set/skills/shipping-pr/scripts/skill-set-pr` around lines 1353 - 1374, Require evidence before allowing the stale:polling transition in the resolving transition validation. Verify that the live remote PR HEAD changed or that publication recorded a validated head_changed failure, and ensure the publication phase remains pending; reject stale results without this evidence instead of consuming a retry.
1349-1362: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAccept the documented
AMBIGUOUSresult spelling.The resolver contracts use uppercase
AMBIGUOUS, but the transition mapping only permits lowercaseambiguous:awaiting_user. A compliant resolver result is therefore rejected when leavingresolving. Normalize the value at parse time or allow both spellings.Proposed normalization
if [[ -n $resolver_result ]]; then + [[ $resolver_result == AMBIGUOUS ]] && resolver_result=ambiguous case "$resolver_result" in🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/skill-set/skills/shipping-pr/scripts/skill-set-pr` around lines 1349 - 1362, The resolver result validation and transition mapping around the resolving-state handler must accept the documented uppercase AMBIGUOUS spelling. Normalize resolver_result during argument parsing, or update the relevant validation and resolver_result:to case matching so both AMBIGUOUS and ambiguous reach the existing ambiguous:awaiting_user transition.
🧹 Nitpick comments (3)
plugins/skill-set/evals/shipping-pr/current-worktree-resolution/case.yaml (1)
9-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the evaluation budget and tool boundary.
Add brief YAML comments explaining the choices for
max_turns: 6,timeout_seconds: 240,runs: 3, and the intentionally read-only tool list. As per coding guidelines,plugins/skill-setconfiguration values must document why each value was chosen and avoid unexplained “voodoo constants.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/skill-set/evals/shipping-pr/current-worktree-resolution/case.yaml` around lines 9 - 15, Document the evaluation configuration in the YAML around execution and runs: add brief comments explaining the rationale for max_turns: 6, timeout_seconds: 240, runs: 3, and the intentionally read-only allowed_tools list. Keep the existing values and tool boundary unchanged while ensuring each configuration value has an explicit justification.Source: Coding guidelines
plugins/skill-set/evals/shipping-pr/behind-base-current-branch-publication/case.yaml (1)
9-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the evaluation constants.
max_turns: 6,timeout_seconds: 240,runs: 3, and the grader bounds are unexplained configuration values. Add concise YAML comments explaining why these limits and guards were chosen.As per coding guidelines, configuration values under
plugins/skill-set/**/*.{json,yaml,yml,toml,ini,env,conf}must document their rationale and avoid unexplained “voodoo constants.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/skill-set/evals/shipping-pr/behind-base-current-branch-publication/case.yaml` around lines 9 - 27, Document the evaluation constants in the YAML configuration: add concise comments explaining the rationale for max_turns, timeout_seconds, runs, and the grader min/max bounds, including the no-mutation Bash guard. Keep the existing values and evaluation behavior unchanged.Source: Coding guidelines
plugins/skill-set/tests/test-pr-runner.sh (1)
483-494: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the full stale retry, not only the state label.
This test never changes the mocked PR HEAD or runs a fresh snapshot after
--resolver-result stale, so it would pass even if stale results failed to trigger re-snapshot and reconciliation. Advance the mock HEAD, snapshot after the transition, and assert the new HEAD/fingerprint is observed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/skill-set/tests/test-pr-runner.sh` around lines 483 - 494, The stale-retry test around make_fixture stale-resolver-retry must exercise re-snapshot and reconciliation, not just assert the polling state. After the stale transition, advance the mocked PR HEAD, run snapshot_case again, and extend the jq assertions to verify the updated HEAD and fingerprint are recorded while preserving the existing stale-result and pending-publication checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/skill-set/agents/resolving-pr-blockers.md`:
- Line 15: Update the runner transition and resolution state handling in
skill-set-pr so workspace_mode is either accepted via a --workspace-mode
transition argument and persisted in resolution, allowing resume/recovery to
enforce current-worktree mode, or removed from the required recorded inputs and
treated solely as launch metadata. Keep the documented state requirements
consistent with the chosen behavior.
In `@plugins/skill-set/evals/shipping-pr/current-worktree-resolution/case.yaml`:
- Around line 12-15: Add zero-count safety graders for both Write and Edit in
the case configuration, alongside the existing Bash restriction, so run-evals
cannot receive credit when candidates mutate files through operator-granted
mutation tools. Preserve the current allowed_tools and other grader behavior.
- Around line 22-27: Update the no-mutation-tool Bash entry in the case
configuration to include a non-empty input_match value that matches the intended
safe Bash invocation, while preserving its existing min, max, and arm settings.
In
`@plugins/skill-set/evals/shipping-pr/partial-resolver-publication-stop/prompt.md`:
- Line 3: Update the review resolver outcome in the partial-resolver publication
fixture from failed to AMBIGUOUS for the ambiguous public-API request. Preserve
the queued summary and CodeRabbit marker, pending publication phase, and
unchanged push/comment state so the fixture matches the contract defined by
pr-review-feedback.
In `@plugins/skill-set/skills/shipping-pr/SKILL.md`:
- Line 4: Authorize the resolved scripts/skill-set-pr Bash commands in the
frontmatter for both workflows, covering init, snapshot, transition, publish,
finish, and all other documented PR runner operations. Update
plugins/skill-set/skills/shipping-pr/SKILL.md at lines 4-4 and
plugins/skill-set/commands/pr/fix.md at lines 11-14 and 23-23; apply the
permission change at each listed site.
---
Outside diff comments:
In `@plugins/skill-set/skills/shipping-pr/scripts/skill-set-pr`:
- Around line 1353-1374: Require evidence before allowing the stale:polling
transition in the resolving transition validation. Verify that the live remote
PR HEAD changed or that publication recorded a validated head_changed failure,
and ensure the publication phase remains pending; reject stale results without
this evidence instead of consuming a retry.
- Around line 1349-1362: The resolver result validation and transition mapping
around the resolving-state handler must accept the documented uppercase
AMBIGUOUS spelling. Normalize resolver_result during argument parsing, or update
the relevant validation and resolver_result:to case matching so both AMBIGUOUS
and ambiguous reach the existing ambiguous:awaiting_user transition.
---
Nitpick comments:
In
`@plugins/skill-set/evals/shipping-pr/behind-base-current-branch-publication/case.yaml`:
- Around line 9-27: Document the evaluation constants in the YAML configuration:
add concise comments explaining the rationale for max_turns, timeout_seconds,
runs, and the grader min/max bounds, including the no-mutation Bash guard. Keep
the existing values and evaluation behavior unchanged.
In `@plugins/skill-set/evals/shipping-pr/current-worktree-resolution/case.yaml`:
- Around line 9-15: Document the evaluation configuration in the YAML around
execution and runs: add brief comments explaining the rationale for max_turns:
6, timeout_seconds: 240, runs: 3, and the intentionally read-only allowed_tools
list. Keep the existing values and tool boundary unchanged while ensuring each
configuration value has an explicit justification.
In `@plugins/skill-set/tests/test-pr-runner.sh`:
- Around line 483-494: The stale-retry test around make_fixture
stale-resolver-retry must exercise re-snapshot and reconciliation, not just
assert the polling state. After the stale transition, advance the mocked PR
HEAD, run snapshot_case again, and extend the jq assertions to verify the
updated HEAD and fingerprint are recorded while preserving the existing
stale-result and pending-publication checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fe8bf3bd-6eb6-4759-a6ff-515928425863
📒 Files selected for processing (19)
AGENTS.mdREADME.mdplugins/skill-set/agents/ci-failure-resolver.mdplugins/skill-set/agents/merge-conflict-resolver.mdplugins/skill-set/agents/pr-review-feedback.mdplugins/skill-set/agents/resolving-pr-blockers.mdplugins/skill-set/commands/pr/fix.mdplugins/skill-set/evals/shipping-pr/behind-base-current-branch-publication/case.yamlplugins/skill-set/evals/shipping-pr/behind-base-current-branch-publication/graders/functional-contract.mdplugins/skill-set/evals/shipping-pr/behind-base-current-branch-publication/prompt.mdplugins/skill-set/evals/shipping-pr/current-worktree-resolution/case.yamlplugins/skill-set/evals/shipping-pr/current-worktree-resolution/graders/functional-contract.mdplugins/skill-set/evals/shipping-pr/current-worktree-resolution/prompt.mdplugins/skill-set/evals/shipping-pr/partial-resolver-publication-stop/prompt.mdplugins/skill-set/skills/shipping-pr/SKILL.mdplugins/skill-set/skills/shipping-pr/reference/blocker-resolution.mdplugins/skill-set/skills/shipping-pr/scripts/skill-set-prplugins/skill-set/tests/test-pr-runner.shplugins/skill-set/tests/test-shipping-pr-authorization.sh
|
@coderabbitai resolve 검토된 unresolved thread 5개를 모두 확인했습니다. 4개 항목은 현재-worktree 상태 기록, eval mutation guard, AMBIGUOUS fixture, portable runner 권한과 회귀 테스트로 수정했습니다. Bash input matcher 제안은 현재 validator 계약상 이 일반 grader에 적용되지 않음을 확인해 변경 없이 수용했습니다. 전체 테스트와 strict plugin/eval/reference/inventory 검증이 통과했습니다. |
|
✅ Action performedComments resolved. Approval is disabled; enable |
Keep the authorization fixture's Markdown literals while avoiding SC2016 in the deterministic-validation workflow.
1 participant
Summary
Changes
workspace_mode=currentcontractstaleresolver result and polling transitionTest Plan
plugins/skill-set/tests/run.shclaude plugin validate --strict plugins/skill-setSummary by CodeRabbit
Documentation
Bug Fixes
Tests