Repository navigation
persona-mention: the job-level if: conditions are barely tested — an accidental narrowing of the comment job's recursion guard would pass CI #1288
Description
Problem
.github/workflows/persona-mention-reusable.yml has two job-level if: expressions that decide whether a run ever reaches a step holding the org token: jobs.route.if (comments) and jobs.route-pull-request.if (pull requests). Each duplicates, in workflow-expression syntax, a shell pre-filter that is unit-tested (pm_should_route, pm_pr_should_route). The expressions themselves are almost untested, so the two copies can drift without a red check.
PR #1287 (#1286) added one structural test for the PR job. Measured on its head 3f9088ea by editing the workflow and re-running tests/persona_mention.bats:
| Edit to the workflow | Tests failing |
|---|---|
route-pull-request.if: startsWith(pull_request.body…) → contains(…) |
1 (caught) |
route-pull-request.if: marker line removed |
1 (caught) |
route.if: !contains(comment.body, '<!-- persona:') → !startsWith(…) |
0 (not caught) |
The third is the one that matters most. The comment job's marker check is the recursion guard added after the #860 runaway (pm_is_agent_comment deliberately matches the marker anywhere in a comment). Narrowing it by accident would let a persona's own comment re-trigger routing, and nothing in CI would notice. The existing test also greps the whole file, so a matching string in a YAML comment could satisfy it.
Acceptance criteria
- The structural tests parse the workflow (
python3+ PyYAML, already required bypm_manifest_query) and assert onjobs["route"]["if"]andjobs["route-pull-request"]["if"]specifically, not on the file's text. route.ifis pinned: it contains!contains(github.event.comment.body, '<!-- persona:'), the@petry-projects/handle check, both bot-login exclusions, and the trust-floor list.route-pull-request.ifis pinned:!startsWith(github.event.pull_request.body, '<!-- persona:'), nocontains(github.event.pull_request.body, the same-repo head check, the bot-login exclusions on bothpull_request.user.loginandgithub.actor, and the trust-floor list.- The bot logins hardcoded in both
if:expressions equal the setpm_is_bot_actorrejects (one test that fails if either side gains or loses a login). - Each assertion is shown to bite: the PR description lists, per assertion, the one-line workflow edit that makes it fail.
Budget guard
Tests only — tests/persona_mention.bats and, if needed, a small helper under tests/. No change to the workflow or to scripts/lib/persona-mention.sh. If AC 4 needs a library change to expose the bot list, stop and file that separately.
Activity
don-petry commentedon Oct 10, 2026
Dev-Lead Implementation Plan
Issue: #1288 — persona-mention: the job-level if: conditions are barely tested
Scope
Tests only: parse
persona-mention-reusable.ymlwith python3 + PyYAML and pinjobs.route.ifandjobs.route-pull-request.ifclause by clause, plus a bot-login parity test againstpm_bot_logins/pm_is_bot_actor.Implementation Checklist
tests/persona_mention.bats: inline helper (python3 + yaml) that loads the workflow and emits a job'sif:split into&&clauses (whitespace-normalised) — no file-text grep, so YAML comments can't satisfy an assertionscripts/lib/persona-mention.shTests to Write
route.ifis a pure&&conjunction (no||) and contains, as whole clauses:!contains(github.event.comment.body, '<!-- persona:'),contains(github.event.comment.body, '@petry-projects/'), both bot-login exclusions, the trust-floorfromJSONlistroute-pull-request.ifis a pure&&conjunction and contains: event-name check, same-repo head check, bot exclusions on bothpull_request.user.loginandgithub.actor, trust-floor listcontains(github.event.pull_request.bodyand!startsWith(...)— note: feat: implement issue #1286 — persona-mention router skips any PR whose body quotes '<!-- persona:' — narrow the PR-path marker check to a body authored with the marker #1287 (which switches the workflow tostartsWith) is still open, andmainstill uses!contains(...). Because this issue forbids touching the workflow, the PR-marker assertion is tied to the library's observable axis-2 semantics: ifpm_pr_should_routeroutes a body that quotes the marker mid-text (post-feat: implement issue #1286 — persona-mention router skips any PR whose body quotes '<!-- persona:' — narrow the PR-path marker check to a body authored with the marker #1287 prefix semantics), the workflow must use!startsWithand contain nocontains(github.event.pull_request.body; otherwise it must use!contains. This keeps CI green on today'smainand pins exactly the AC 3 form as soon as feat: implement issue #1286 — persona-mention router skips any PR whose body quotes '<!-- persona:' — narrow the PR-path marker check to a body authored with the marker #1287 lands — and it also catches the workflow and library drifting apart.if:s, the set of!= '<login>'literals equalspm_bot_logins(the setpm_is_bot_actorrejects)Test Command
bats tests/persona_mention.batsdon-petry commentedon Oct 10, 2026
Dev-Lead: Progress (in progress — not yet durable)
Plan Execution (intended)
# --- workflow job-level if: pre-filters (#1288)section totests/persona_mention.bats: python3 + PyYAML loads the workflow, takesjobs[<job>]["if"], splits it on&&into whitespace-normalised clauses, and the tests assert on whole clauses. Text in YAML comments can't satisfy them, and a negated copy of a clause doesn't match either.scripts/lib/persona-mention.share unchanged.Per-assertion bite list (each one-line edit to
persona-mention-reusable.ymlwas applied locally; the named test went red; the edit was reverted)|| truetoroute.if!contains(github.event.comment.body, '<!-- persona:')→!startsWith(…); or delete that line; or move it into a YAML commentcontains(…'@petry-projects/')→!contains(…)!= 'donpetry-bot'line (or thegithub-actions[bot]one)"CONTRIBUTOR"to thefromJSONlist|| true!contains→!startsWith; with #1287 applied,!startsWith→!contains; or move it into a YAML commenthead.repo.full_name == github.repositoryline (or theevent_nameline)pull_request.user.login != 'donpetry-bot', orgithub.actor != 'github-actions[bot]'"CONTRIBUTOR"to the list!= 'other-bot'; addgithub.triggering_actor != 'donpetry-bot'Notes
mainstill has!contains(github.event.pull_request.body, …), and this issue forbids touching the workflow. A hardstartsWithpin would turn CI red, so the PR marker test reads the library's behaviour instead. Ifpm_pr_should_routeroutes a body that quotes the marker mid-text (feat: implement issue #1286 — persona-mention router skips any PR whose body quotes '<!-- persona:' — narrow the PR-path marker check to a body authored with the marker #1287's prefix semantics), the test requires!startsWith(…)and nocontains(github.event.pull_request.body. Otherwise it requires!contains(…). I checked this against feat: implement issue #1286 — persona-mention router skips any PR whose body quotes '<!-- persona:' — narrow the PR-path marker check to a body authored with the marker #1287's head files: the test passes there, and changingstartsWithback tocontainsfails it. Once feat: implement issue #1286 — persona-mention router skips any PR whose body quotes '<!-- persona:' — narrow the PR-path marker check to a body authored with the marker #1287 merges, AC 3 is pinned exactly. Before that, the test still fails if the workflow and the library disagree.pm_bot_logins(the listpm_is_bot_actoriterates) withPERSONA_BOT_LOGINSunset.tests/. That meanspersona-mention-tests.yml's path filter already covers them.don-petry commentedon Oct 10, 2026
Dev-Lead: Implementation Complete — PR #1289
The implementation for issue #1288 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.
6a40a13cfd6caa731e211745454be481443affaemain: 1 file(s), +150/-0 linesReview happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.
feat: implement issue #1288 — persona-mention: the job-level if: cond…