Skip to content

Add the Iris WP Maintainer and an automated PR reviewer - #5

Open
bbertucc wants to merge 4 commits into
mainfrom
ci-maintainer-and-review
Open

bbertucc wants to merge 4 commits into
mainfrom
ci-maintainer-and-review

Conversation

@bbertucc

@bbertucc bbertucc commented Oct 6, 2026

Copy link
Copy Markdown
Member

Summary

  • maintainer.yml: when a repository admin adds Ready for Build to an issue, Claude (Bedrock) builds it against the mock Iris and opens one PR for the admin to review, or explains on the issue why it should not be built as written. It refuses labels added by non-admins, closed issues, issues with an open maintainer PR, and issues whose body or title changed after the label. A separate verify job, on a fresh runner with no checkout, drafts any PR it opened that touches .github/, LICENSE, a .gitignore, WordPress files, PDFs or dumps, then starts the review.
  • code-review.yml: an adversarial review of every same-repo PR, modelled on equalify-iris's reviewer. It runs php-checks.sh, actionlint (pinned and verified), shellcheck, and the DDEV smoke test when plugin/ or test-site/ changed. It posts one review per commit as github-actions[bot], with a fallback if cut off and a separate job that fails when no review was posted. Forks are refused.
  • test-site/smoke.sh: one PDF from upload to the tagged link a logged-out visitor sees, against the mock.
  • docs/CI.md: how both work and what the repository needs.

Testing

  • actionlint and shellcheck --severity=warning are clean on everything.
  • php-checks.sh: lint and compat pass.
  • smoke.sh passes locally.
  • admin-labelled.sh was checked against real events on equalify-iris, including a title change after the label.
  • The workflows themselves have not run on GitHub yet. They need AWS_BEDROCK_ROLE_ARN, and the role's trust policy must allow this repo.

Before merging

  • Set the AWS_BEDROCK_ROLE_ARN secret and extend the IAM trust policy to this repository.
  • Protect main.

🤖 Generated with Claude Code

An admin adding `Ready for Build` to an issue starts maintainer.yml: Claude builds it against the
mock Iris and opens one PR, and a separate job drafts any PR that touches paths it may not.
code-review.yml runs the PHP checks, actionlint, shellcheck and the smoke test, then posts one
adversarial review per commit. Adds test-site/smoke.sh, the CI scripts, and docs/CI.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

This PR changes code-review.yml, so it is reviewed by its own version of it

A pull_request run uses the workflow from the PR. The review below comes from the
reviewer this PR proposes, and a PR that weakened it could pass its own review.

Read the workflow diff by hand. In particular: does it let a fork or PR-authored code
run with secrets, widen permissions:, put github.event.* text into a run: block,
or weaken the step that fails the job when no review was posted?

Run: https://github.com/EqualifyEverything/equalify-iris-wp/actions/runs/37532777838

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review did not complete

The review step ended with failure without posting a review (on failure, most likely its 22-minute limit).
It recorded no findings, so this PR has not been reviewed.

Run a full review with gh workflow run code-review.yml -f pr_number=5.

Site impact: not assessed.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking

1. The new test-network check fails, so test-site/smoke.sh never ran

Check summary: test network and smoke test: fail. The output is the else branch of
.github/workflows/code-review.yml:228:

(the test network did not build)
...
Warning: Unable to create directory wp-content/uploads/2026/10. Is its parent directory writable by the server?
...
==> Adding content
    Created "A Very Long Manual" (publish) on https://equalify-iris-test.ddev.site.

install-ddev.sh succeeded (its log is there and site.log exists), so test-site-ci.sh →
test-site/setup.sh exited non-zero while adding content. The smoke test runs only on the
success branch, so the end-to-end check this PR is built around did not execute once.

Input that reaches it: any PR touching plugin/ or test-site/ — this one.

Same two scripts are the maintainer's baseline (maintainer.yml:295), so the build model
starts every issue with test site: fail, smoke.sh: skipped, against a prompt that says
"If you cannot get the checks green, open no PR".

Lead on the cause, unverified: test-site/setup.sh:191 import_pdf() hides every error
(2>/dev/null, and the pipeline's status is tail's), so a wp media import that cannot
write to wp-content/uploads returns an empty attachment URL instead of failing — matching
the warning above. If the failure is an artifact of this reviewer's sandbox rather than a
GitHub runner, say so and post a green run; as it stands the check that gates both workflows
is red.

Non-blocking notes

  • .github/scripts/install-ddev.sh:19 — ddev version | head -3 is the last command under
    set -euo pipefail, so it is the script's exit status. If ddev writes after head exits,
    SIGPIPE makes it 141 and both workflows report (the test network did not build) for a
    successful install. || true on that line.
  • code-review.yml:43 + :559 — cancel-in-progress: true with verify's if: always():
    a second push cancels the first run, whose verify still runs and fails with
    ::error::No review posted for <old sha> for a commit nobody wanted reviewed. Add
    needs.review.result != 'cancelled'.
  • Every check is continue-on-error (code-review.yml:190), so a failed check leaves the
    workflow green and the only signal is the review body. Deliberate per docs/CI.md, but this
    PR is the case in point.
  • maintainer.yml:155 — verify re-derives everything from the API, but only for new pull
    requests
    . With contents: write and Bash(*), a model steered by untrusted issue text can
    also push straight to main or onto another open PR's branch, and verify sees neither. The
    only guard is branch protection on main (docs/CI.md:78), a repository setting outside
    this PR — have it on before AWS_BEDROCK_ROLE_ARN exists.
  • code-review.yml:201 passes "${PHP_FILES[@]:-}", i.e. one empty argument when no PHP file
    changed, so $# -eq 0 is never true and the "every plugin file when none are given" default
    documented in php-checks.sh:14 cannot happen from CI. Harmless, but the comment and the
    code disagree.

Checked and clean: no ${{ github.event.* }} text in any run: block (all of it goes through
env:), forks refused in both the job if: and again from the API, dependabot excluded,
actions pinned to SHAs, DDEV and actionlint downloads checked against SHA-256, step caps under
job caps (12+22 < 45, 20+45 < 75), FORBIDDEN/PRIVATE run before anything they guard on a
job with no checkout, and issue text fenced into a file rather than a prompt or a run:.
.github/ did not exist on main, so nothing was weakened.

Site impact: none for a live site — this PR adds CI only; the one thing it ships that a site would feel is the maintainer workflow's ability to push branches, which stops at a pull request a human must merge.

The maintainer now answers each review of its own pull request on the same branch: it fixes what it
agrees with, says why for what it does not, and asks the admin who approved the issue to review only
when the pull request is ready or it is blocked (maintainer-revise.yml). An admin's review sends it
back. A separate job checks every push, and the reviewer reads the maintainer's replies.

The test site did not build on Linux runners: Docker created the plugin's mount point as root, so
WordPress could not write to uploads. setup.sh now makes those directories first, and stops with a
message when a PDF cannot be added instead of hiding it.

A review run replaced by a newer push no longer posts a "did not complete" review or fails verify.
The maintainer's rules and path checks are now one file each, shared by both workflows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking

1. The test-network check still fails: the CI runner has no https, and smoke.sh dies with no message

Check summary: test network and smoke test: fail — unchanged since d4812a8, but the cause
has moved. The setup.sh fix worked: the network builds and the PDF is tagged.

ok    the PDF on a published page is queued
ok    the PDF on a draft page is refused
ok    tagged after 3 runs

Then the log stops. No FAIL line, no error.

Reproduced on this runner, against the site this run left up:

$ curl -sk -o /dev/null -w '%{http_code}\n' https://equalify-iris-test.ddev.site/   # 000, curl exit 35
$ curl -s  -o /dev/null -w '%{http_code}\n' http://equalify-iris-test.ddev.site/    # 200
$ ddev describe   ->   web  OK  http://equalify-iris-test.ddev.site     (no https URL at all)

/tmp/ddev.log says why: mkcert-caroot= is empty, and DDEV warns mkcert may not be properly installed ... and then mkcert -install``. .github/scripts/install-ddev.sh:16 unpacks the
`mkcert` binary but nothing ever runs `mkcert -install`, so the router serves port 80 only.

test-site/smoke.sh:30 is therefore unreachable in CI:

SITE_URL="https://equalify-iris-test.ddev.site"

and test-site/smoke.sh:145

html="$(curl -sk "${SITE_URL}/${slug}/")"

is a command substitution with no || fail, so set -e ends the script right there, silently.
The two checks this PR is built around — the tagged link a logged-out visitor sees, and the tagged
copy being served as a PDF — have never run once, and the reviewer gets a red check with nothing
in it to act on. smoke.sh:142 (tagged_url="$(wpq eval ...)") has the same shape, and wpq
folds stderr into the variable, so it would also exit mute.

Input that reaches it: any PR touching plugin/ or test-site/ — this one.

The same two scripts are the maintainer's baseline (maintainer-revise.yml:362, maintainer.yml),
so every build and every revise round starts from smoke.sh: fail with an empty log, against a
prompt that says not to push unless the checks pass.

Both halves want fixing: install mkcert in install-ddev.sh, or take the URL from
$DDEV_PRIMARY_URL / ddev describe -j instead of hard-coding the scheme; and put || fail "..."
on smoke.sh:142 and :145, so a step that dies says which one it was.

Non-blocking notes

  • Fixed since d4812a8: install-ddev.sh now ends head -3 || true, and verify no longer fails
    a run a newer push cancelled (code-review.yml:591).
  • test-site/smoke.sh:82 prints bash: line 1: wp/wp-content/debug.log: No such file or directory
    into the check output on every CI run: the < redirect fails before 2>/dev/null can apply. It
    is noise in the one artefact the reviewer reads.
  • maintainer-revise.yml:56 — the comment says relay "runs the PR's copy of this file".
    pull_request_review runs the base branch's copy. The code is safe either way; the comment
    explains a guard by a rule that does not hold.
  • maintainer-revise.yml:268 — build checks out with credentials persisted (the default) and
    holds contents: write, so a model steered by text in its context can push outside the PR's
    branch, and verify compares only that branch. Same shape as last round's note on
    maintainer.yml: have branch protection on main before AWS_BEDROCK_ROLE_ARN exists.
  • Withdrawn from last round: I called install-ddev.sh's head -3 and the cancelled-run verify
    failure separate guesses about the red check; the real cause was neither, and both are now fixed
    anyway.

Checked again on the new files: no ${{ github.event.* }} text inside any run: block (relay
passes REVIEWER and PR through env:); relay holds actions: write and no checkout;
gate refuses a dispatch by anyone but an admin or github-actions[bot], re-derives the PR, the
branch pattern and the approver from the API, and runs pr-path-problem.sh from main before
anything on the branch; only github-actions[bot] reviews and admin comments reach the prompt,
each fenced; the model's reply is posted by verify with <!-- defused; step caps sit under job
caps (20 + 45 < 75); MAX_ROUNDS bounds the review/revise loop; actions pinned to SHAs.

Site impact: none — this PR adds CI only, and as it stands the end-to-end check it ships has not completed a single run.

DDEV served the test site over http only on the runner, because nothing installed mkcert's local
CA, so smoke.sh's first https request failed. It also failed silently: a command substitution
ended the script under set -e with no message. It now says which line stopped it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both blocking findings are fixed, and every check is green for the first time on this PR.

  • Fixed since 0fd623c: .github/scripts/install-ddev.sh:20 now runs mkcert -install, so DDEV serves https://equalify-iris-test.ddev.site and smoke.sh's hard-coded scheme resolves. The check summary now reads test network and smoke test: pass, including the two checks that had never run:

    ok    logged out, the page links to the tagged copy
    ok    the tagged copy is a PDF
    
  • Fixed since 0fd623c: test-site/smoke.sh:41 trap '... $LINENO: $BASH_COMMAND' ERR plus || fail on :146 and :150, so a step that dies now says which one. :86 { wc -l < ...; } 2>/dev/null also removed the stray No such file or directory line.

Non-blocking notes

  • Raised twice, unchanged: maintainer.yml:155 and maintainer-revise.yml:268 check out with credentials persisted and hold contents: write, and verify only re-derives the PR's own branch, so a model steered by issue or review text can push to main or another open branch unseen. The only guard is branch protection, a repository setting (docs/CI.md:78). The PR body lists it under "Before merging"; I'll leave it there for a human rather than raise it a third time.
  • code-review.yml:190 — every check is continue-on-error, so a red check still leaves the workflow green and the review body is the only signal. Deliberate per docs/CI.md; worth knowing that this PR's first two rounds were green with a failing end-to-end check.
  • code-review.yml:235 — ddev exec tail -n 30 wp/wp-content/debug.log still prints Failed to execute command ... exit status 1 in red at the end of every passing run, because a clean run has no debug.log. Same noise I noted in smoke.sh, different line; || true keeps it harmless but it is the last thing the reviewer reads.
  • code-review.yml:201 passes "${PHP_FILES[@]:-}", one empty argument when no PHP changed, so $# -eq 0 never holds and the "every plugin file when none are given" default in php-checks.sh:14 cannot happen from CI. The comment and the code disagree.
  • test-site/smoke.sh:41 — without set -E, the ERR trap is not inherited by shell functions, so a failure inside wpq/cleanup still dies quiet. Every current call site is at top level, so nothing reaches it today.

Site impact: none for a live site — this PR adds CI only, and the end-to-end check it ships now completes a full run against the mock Iris.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant