fix(ci): PR-review follow-ups that missed the #73 merge window#74
Merged
Conversation
…en-and-egg ## Summary Round-4 AI review fixes. On a clean installation the labels-bootstrap job was gated on should_run=true, which itself requires the Ready label to already be applied — so the job that creates the Ready label could never run before the label existed. ## Change Log - labels-bootstrap: drop the should_run condition; the job now runs on every opened/labeled event (idempotent, ~4s) - The second round-4 finding (vendored apply-review/open-pr SKILL.md declaring git+gh via requires.anyBins when both are mandatory) is an upstream defect in the skill pack — deferred to DailybotHQ/ai-diff-reviewer#42 rather than hand-editing the vendored tree and drifting it from the skills-lock.json content hash ## Risks - None — one job condition removed; the bootstrap step was already idempotent (`|| true` on existing labels) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…kip release] policy ## Summary Round-5 AI review fix: AGENTS.md § 15.a and the auto-release.yml header were expanded to two accepted [skip release] use cases, but docs/RELEASE_AND_DISTRIBUTION.md still said "one accepted use case" and listed CI/workflow-only PRs as forbidden — contradicting case (b) and this PR's own merge policy. ## Change Log - RELEASE_AND_DISTRIBUTION.md § "When (and only when) to use it": two enumerated cases matching AGENTS.md § 15.a; forbidden list narrowed to CLI docs and release/test-pipeline CI (agent-tooling wiring is case (b)) - AGENTS.md § 15.a opening line no longer says "one narrow situation" while rule 3 enumerates two ## Risks - None — policy documentation only Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AI review for
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
Looks good — the chicken-and-egg bootstrap fix is correct, and the [skip release] docs now match AGENTS.md Rule 15.a / auto-release.yml. Nothing blocking.
Findings table
| # | Severity | File | Summary |
|---|---|---|---|
| — | — | — | No findings |
Notes
labels-bootstrap: Removingif: needs.scope.outputs.should_run == 'true'is the right fix.should_runrequiresReadyto already be applied, so gating label creation on it made bootstrap unreachable on a clean install. The job remains idempotent (gh label create … || true), stillneeds: scope(scope always exits 0), and the expensivereviewjob stays gated onshould_run. Header comment at.github/workflows/pr-review.yml:29(“auto-created on first run”) is now truthful.- Policy docs:
AGENTS.mdopener anddocs/RELEASE_AND_DISTRIBUTION.mdnow both describe two accepted use cases and distinguish forbidden “CI tweaks to the release/test pipelines” from case (b) agent-tooling workflows — aligned withauto-release.ymllines 26–41 and Rule 15.a §3. - Out of scope (deferred upstream):
requires.anyBinsforgit+ghin vendored apply-review/open-pr — correctly left to DailybotHQ/ai-diff-reviewer#42 rather than drifting the vendored tree. - Release: This PR itself is workflow + policy-doc only (case (b) candidate). Merge for a PATCH unless a maintainer explicitly requests
[skip release]on the squash body.
Recommendation: approve
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #73 was merged while its AI-review iteration was still in flight, so the last two review rounds' fixes landed on the branch after the squash merge and never reached
main. This PR cherry-picks them.Changes
labels-bootstrapchicken-and-egg fix (round-4 finding,pr-review.yml): the job that creates theReadylabel was gated onshould_run == 'true', which itself requiresReadyto already be applied — so on a clean installation the bootstrap could never create the label maintainers are told to apply. The job now runs on everyopened/labeledevent (idempotent, ~4s).docs/RELEASE_AND_DISTRIBUTION.mdpolicy alignment (round-5 finding): AGENTS.md § 15.a and theauto-release.ymlheader were expanded in chore(agents): upgrade DWP to 2.17.0 + AI Diff Reviewer Flow B [skip release] #73 to two accepted[skip release]use cases, but this doc still said "one accepted use case" and forbade workflow-only skips — contradicting case (b) and chore(agents): upgrade DWP to 2.17.0 + AI Diff Reviewer Flow B [skip release] #73's own merge. Now aligned (plus a one-line rephrase of § 15.a's opener so it doesn't say "one narrow situation" while rule 3 lists two).Also from #73's review rounds, deferred upstream rather than patched here: the vendored
apply-review/open-prsub-skills declaregit+ghviarequires.anyBinswhen both are mandatory — filed as DailybotHQ/ai-diff-reviewer#42 (editing the vendored tree would drift it from theskills-lock.jsoncontent hash).Release — MUST use
[skip release]Maintainer request: this PR is workflow + policy-doc only (case (b) — agent-tooling / CI wiring; nothing a CLI user installs). Do not cut a release.
When squash-merging, put this exact token in the squash commit body (not only the title):
[skip release]