fix(review): bot pull requests are reviewed, edits cancel no review, conventions read live labels

.github/workflows/claude-review.yml

@@ -103,14 +103,25 @@ jobs:
   review:
     name: Claude review
     runs-on: ubuntu-latest
-    timeout-minutes: 30
+    # An edit may wait for its push's review, which itself can take the 30
+    # minutes a review job has: an edit's job gets longer to outlast it.
+    timeout-minutes: ${{ github.event.action == 'edited' && 60 || 30 }}
     if: >-
       github.event_name == 'pull_request' &&
       github.event.pull_request.head.repo.full_name == github.repository &&
       !github.event.pull_request.draft
+    # An edit of the title or description and a push can land on the same
+    # commit a second apart, and a cancelled run leaves a cancelled check on
+    # the commit that blocks the merge even when the other run passed. So an
+    # edit runs in a group of its own run: it cancels nothing and nothing
+    # cancels it (it spends nothing: it reuses the verdict on a commit the
+    # review read, and waits for the push's review of one it has not, then
+    # reuses that; with no such review it fails). A push
+    # still cancels the review of the push before it, whose check sits on an
+    # older commit and blocks nothing.
     concurrency:
-      group: claude-review-${{ github.repository }}-${{ github.event.pull_request.number }}
-      cancel-in-progress: true
+      group: claude-review-${{ github.repository }}-${{ github.event.pull_request.number }}-${{ github.event.action == 'edited' && github.run_id || 'code' }}
+      cancel-in-progress: ${{ github.event.action != 'edited' }}
     outputs:
       risk: ${{ steps.verdict.outputs.risk }}
       complexity: ${{ steps.verdict.outputs.complexity }}
@@ -188,6 +199,7 @@ jobs:
           PR: ${{ github.event.pull_request.number }}
           HEAD: ${{ github.event.pull_request.head.sha }}
           BASE: ${{ github.event.pull_request.base.sha }}
+          ACTION: ${{ github.event.action }}
           FLOOR: ${{ steps.policy.outputs.floor }}
           MAX_AUTO: ${{ inputs.max-auto-reviews }}
           MODEL: ${{ steps.policy.outputs.model }}
@@ -215,6 +227,30 @@ jobs:
             mode=reuse
           elif [ -n "$last" ] && [ "$count" -ge "$MAX_AUTO" ] && [ "$has_verdict" = yes ]; then
             mode=capped
+          elif [ "$ACTION" = edited ]; then
+            # An edit on a commit its push's run is still reviewing: it pays
+            # for no second review. It waits for that run's record of this
+            # commit and reuses its verdict (the edited description is then
+            # compared with the one the review read), and fails when none
+            # comes: a green check without a verdict would replace the push
+            # run's own under the same name.
+            # Up to 50 minutes: longer than a push's review job (30) can run.
+            for _ in $(seq 1 100); do
+              sleep 30
+              record=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR/comments" --paginate \
+                | jq -r --arg bot "$BOT" '.[] | select(.user.login == $bot and (.body | startswith("<!-- dx-review -->"))) | .body' \
+                | grep -oP '<!-- dx-review-record \K.*(?= -->)' | tail -1 || true)
+              printf '%s' "${record:-null}" | jq . > "$RUNNER_TEMP/previous.json" 2>/dev/null || echo null > "$RUNNER_TEMP/previous.json"
+              if [ "$(jq -r '.sha // empty' "$RUNNER_TEMP/previous.json")" = "$HEAD" ] \
+                 && [ "$(jq -r 'if .risk and .complexity and (.blocking != null) then "yes" else "no" end' "$RUNNER_TEMP/previous.json")" = yes ]; then
+                mode=reuse; break
+              fi
+            done
+            if [ "$mode" != reuse ]; then
+              echo "::error::No review of ${HEAD:0:7} came from its push within 50 minutes; push again, or re-run that review."
+              exit 1
+            fi
+            last=$HEAD; count=$(jq -r '.count // 0' "$RUNNER_TEMP/previous.json")
           elif [ -n "$last" ] && git cat-file -e "$last^{commit}" 2>/dev/null \
                && git merge-base --is-ancestor "$last" "$HEAD" \
                && [ -z "$(git rev-list --merges "$last..$HEAD")" ]; then
@@ -284,7 +320,11 @@ jobs:
         with:
           anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
           github_token: ${{ steps.bot.outputs.token }}
-          allowed_bots: 'dependabot[bot]'
+          # Dependabot's pull requests, and the organisation's own App's (the
+          # weekly review-learnings proposals, the reproduction drafts): the
+          # action refuses any other bot as the actor, and the review of those
+          # ended without a verdict.
+          allowed_bots: dependabot[bot],${{ steps.bot.outputs.app-slug }}
           prompt: |
             You are the code reviewer for ${{ github.repository }}, pull request #${{ github.event.pull_request.number }}.
 
@@ -462,8 +502,10 @@ jobs:
           # The changed files travel with the record: the weekly learning job
           # uses them as evidence for a repository's safe and risky paths.
           files=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json files --jq '[.files[].path] | .[:300]' 2>/dev/null || echo '[]')
-          record=$(jq -c --arg risk "$risk" --arg floor "$FLOOR" --arg sha "$SHA" --arg model "$MODEL" --arg mode "$MODE" --argjson count "$count" --arg cost "${cost:-}" --argjson files "$files" --arg type "$type" --arg text "$text" \
-            '{risk:$risk, model_risk:.risk, floor:$floor, complexity, blocking, type:$type, description_matches:.description_matches, text:$text, lessons, findings, sha:$sha, model:$model, mode:$mode, count:$count, cost:$cost, files:$files}' <<<"$OUT")
+          # `max` lets the conventions tell a capped review (no new reading
+          # will come) from one still to run on a new commit.
+          record=$(jq -c --arg risk "$risk" --arg floor "$FLOOR" --arg sha "$SHA" --arg model "$MODEL" --arg mode "$MODE" --argjson count "$count" --argjson max "${MAX_AUTO:-5}" --arg cost "${cost:-}" --argjson files "$files" --arg type "$type" --arg text "$text" \
+            '{risk:$risk, model_risk:.risk, floor:$floor, complexity, blocking, type:$type, description_matches:.description_matches, text:$text, lessons, findings, sha:$sha, model:$model, mode:$mode, count:$count, max:$max, cost:$cost, files:$files}' <<<"$OUT")
           body=$(cat <<EOF
           <!-- dx-review -->
           ### Claude review · risk **$risk** · complexity **$complexity** · type **$type**$blk

.github/workflows/conventions.yml

@@ -38,6 +38,10 @@ on:
         description: Maximum length of a PR title or commit header.
         type: number
         default: 100
+      review-bot:
+        description: The login of the App that posts the Claude review; only its comments carry a record the conventions trust.
+        type: string
+        default: 'dilux-bot[bot]'
       central-ref:
         description: Ref of DiluxOne/.github to read the conventions script from.
         type: string
@@ -99,6 +103,26 @@ jobs:
           path: .dx-central
           persist-credentials: false
 
+      - name: The labels and the review's comments, as they are now
+        id: live
+        env:
+          GH_TOKEN: ${{ github.token }}
+          PR: ${{ github.event.pull_request.number }}
+          EVENT_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }}
+        # The event carries the labels of the moment it fired, and a re-run
+        # replays that event: a type:* label the review changed since would
+        # read as the old one. Ask for them now, and for the comments, where
+        # the review's last record says which commit it read.
+        run: |
+          if labels=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR/labels" --jq '[.[].name] | join(",")') \
+             && gh api "repos/$GITHUB_REPOSITORY/issues/$PR/comments" --paginate > "$RUNNER_TEMP/comments.json"; then
+            echo "labels=$labels" >> "$GITHUB_OUTPUT"
+          else
+            echo "::warning::Could not read the pull request's labels and comments (the caller's job needs pull-requests: read); the event's labels are used, and a re-run may see a type:* label the review has changed since."
+            echo "labels=$EVENT_LABELS" >> "$GITHUB_OUTPUT"
+            echo '[]' > "$RUNNER_TEMP/comments.json"
+          fi
+
       - name: Check branch, title and commits
         env:
           # Through env, never interpolated into the script: a title is
@@ -111,7 +135,9 @@ jobs:
           BODY: ${{ github.event.pull_request.body }}
           AUTHOR_TYPE: ${{ github.event.pull_request.user.type }}
           SECTIONS: ${{ inputs.required-sections }}
-          LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }}
+          LABELS: ${{ steps.live.outputs.labels }}
+          COMMENTS_FILE: ${{ runner.temp }}/comments.json
+          REVIEW_BOT: ${{ inputs.review-bot }}
         run: bash .dx-central/scripts/conventions.sh
 
   docs:

README.md

@@ -26,7 +26,8 @@ request template, issue forms) unless it has its own. The rules for contributors
    base branch, so a change cannot pick its own reviewer), skips the
    repository's code rules for text-only changes, and after `max-auto-reviews`
    (5) keeps the last verdict until the `review:full` label asks for more.
-   Drafts and forks are not reviewed.
+   Drafts and forks are not reviewed; pull requests Dependabot or the
+   organisation's App open are (`allowed_bots`).
 3. **Merge.** A human, with [`scripts/squash-merge.sh`](scripts/squash-merge.sh)
    `<owner/repo> <number>` (description verbatim, co-authors kept), or
    GitHub's auto-merge when the floor is low, the verdict is low risk and low
@@ -37,16 +38,27 @@ Mention `@dilux-bot` in a review thread or in the conversation and Claude
 answers there; it resolves its own thread when the point is settled.
 
 Nothing runs twice for one change. An edit of the title or the description
-re-runs only the conventions, the review (free on a commit it already read)
-and the auto-merge decision, never the suites. A push to `main` runs the slow
-suites only when it must: the job verifies that the commit is the squash
-merge of a pull request of this repository, that its tree is the one that
-pull request's checks ran on and that every check there passed; any doubt
-runs everything. A pull request whose base branch changed after its last
-push fails the conventions on every edit until a push runs the suites
-against the new base. Once a week everything runs against today's
-WordPress and tools, and a failure opens one `ci:weekly` issue; *Run
-workflow* runs everything by hand, and its failure is in the run alone.
+re-runs only the conventions, the review (free on a commit it already
+read) and the auto-merge decision, never the suites. A push to `main` runs
+the slow suites only when it must: the job verifies that the commit is the
+squash merge of a pull request of this repository, that its tree is the
+one that pull request's checks ran on and that every check there passed;
+any doubt runs everything. A pull request whose base branch changed after
+its last push fails the conventions on every edit until a push runs the
+suites against the new base. The conventions read the labels as they are
+when they run, not as the event carried them, so a re-run sees the
+`type:*` label the review set since; they compare the title's type with it
+once the review has read the commit (the review App's last record names
+it) or has reached its cap, so a push that changes the title waits for the
+review's new reading instead of failing on the old label. An edit's review
+runs in a concurrency group of its own: it cancels nothing and nothing
+cancels it, since a cancelled run leaves a cancelled check that blocks the
+merge. It spends nothing: on a commit the review read it reuses the
+verdict, and on one its push's run is still reviewing it waits for that
+verdict and reuses it, or fails when none comes. Once a week everything
+runs against today's WordPress and tools, and a failure opens one
+`ci:weekly` issue; *Run workflow* runs everything by hand, and its failure
+is in the run alone.
 
 ## Adopt it in a new repository
 
@@ -151,11 +163,11 @@ Call them pinned to `@v2`; a breaking change ships as `v2`. A stack suffix
 
 | Workflow | Does | Inputs |
 | --- | --- | --- |
-| [`conventions.yml`](.github/workflows/conventions.yml) | Branch, title, commits, description sections, no "Generated with" footer (all in `scripts/conventions.sh`), relative doc links, retired names. | `retired-names`, `required-sections`, `max-header`, `central-ref` |
+| [`conventions.yml`](.github/workflows/conventions.yml) | Branch, title, commits, description sections, no "Generated with" footer (all in `scripts/conventions.sh`), relative doc links, retired names. Reads the labels and the review App's last record live, so a re-run sees today's `type:*` label. | `retired-names`, `required-sections`, `max-header`, `central-ref`, `review-bot` |
 | [`claude-review.yml`](.github/workflows/claude-review.yml) | The review described above. Outputs `risk`, `complexity`, `floor`, `trusted`, `blocking`. | `profile`, `central-ref`, `max-auto-reviews` |
 | [`review-reply.yml`](.github/workflows/review-reply.yml) | Answers `@dilux-bot` mentions from members and collaborators, with the strong model. | `profile`, `central-ref` |
 | [`auto-merge.yml`](.github/workflows/auto-merge.yml) | Turns GitHub's auto-merge on or off from the review's outputs and commits the description verbatim. `pull-request-edited.yml` runs it on `edited` too. | the five review outputs |
-| [`scripts/conventions.sh`](scripts/conventions.sh) | Not a workflow: the conventions a pull request is held to (branch, title, every commit header, no session trailer, description sections, no "Generated with" footer). `conventions.yml` runs it on a pull request, `local-review.sh` before one; `--test` for its own tests. | env: `BRANCH`, `TITLE`, `BODY`, `BASE`, `HEAD_REF`, `MAX_HEADER`, `SECTIONS`, `LABELS`, `AUTHOR_TYPE` |
+| [`scripts/conventions.sh`](scripts/conventions.sh) | Not a workflow: the conventions a pull request is held to (branch, title, every commit header, no session trailer, description sections, no "Generated with" footer). `conventions.yml` runs it on a pull request, `local-review.sh` before one; `--test` for its own tests. | env: `BRANCH`, `TITLE`, `BODY`, `BASE`, `HEAD_REF`, `COMMENTS_FILE`, `REVIEW_BOT`, `MAX_HEADER`, `SECTIONS`, `LABELS`, `AUTHOR_TYPE` |
 | [`scripts/review-brief.sh`](scripts/review-brief.sh) | Not a workflow: the review brief, the one file the Claude review reads (profiles, `AGENTS.md`, `docs/architecture.md`, policy floor, description, diff). `claude-review.yml` and `local-review.sh` build it with the same script; `--test` for its own tests. | env: see the script's header |
 | [`scripts/local-review.sh`](scripts/local-review.sh) | Not a workflow: the pull request's checks before it exists, on a contributor's machine: conventions, policy floor, brief, and the review through the local Claude Code CLI; the findings go to `.git/dx-review/findings.md` and the next run is incremental. See CONTRIBUTING.md, "Review before the pull request"; `--test` for its own tests (never runs the review). | `--base`, `--title`, `--body-file`, `--profile`, `--no-claude`, `--full`, `--model` |
 | [`scripts/release-ready.sh`](scripts/release-ready.sh) | Not a workflow: whether a readme's newest changelog entry is ready (exit 0) or held by a first line `Unreleased.` (exit 1); `--test` for its own tests. The release job's hold. | the readme |