feat: implement issue #1288 — persona-mention: the job-level if: conditions are barely tested — an accidental narrowing of the comment job's recursion guard would pass CI

[don-petry]

Problem

persona-mention: the job-level if: conditions are barely tested — an accidental narrowing of the comment job's recursion guard would pass CI

From the issue: .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.

Risk

Low — 1 file(s) changed; no CI- or runtime-critical paths touched.

Test plan

Tests added/updated: tests/persona_mention.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's CI checks (Lint + bats) are the success/regression signal.

Closes #1288

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[gemini-code-assist]

Code Review

This pull request introduces comprehensive BATS tests to validate GitHub Actions workflow job-level if: pre-filters for the route and route-pull-request jobs, ensuring they align with expected shell pre-filter logic and exclude bot logins. The feedback suggests extracting the regular expression pattern in bot_logins_compared into a local variable to improve readability and portability across Bash versions.

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed. Requested items addressed:

• tests/persona_mention.bats:840 — applied
• tests/persona_mention.bats:883 — applied
• tests/persona_mention.bats:891 — applied

[don-petry]

Acknowledged: the Codex usage-limit notice contains no review finding; no action needed.

[don-petry]

Acknowledged: SonarCloud quality gate passed; nothing to act on.

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

- **Tier 1 blockers:** none. No review has `CHANGES_REQUESTED`. The four cancelled `dev-lead / dispatch` checks are most likely superseded runs, not failures, and `npm audit` is still queued.
- **Issue comments:** I posted one disposition reply on each of the four open bot comments.
  - **codeant-ai:** `fixed`, citing c781ea5. Its three findings were against 8c8fa75. c781ea5 added exact clause-count tests (5 for `route`, 8 for `route-pull-request`) and pinned the marker policy to `!contains` explicitly in `tests/persona_mention.bats`.
  - **codex:** `informational` (usage-limit notice).
  - **coderabbit:** `informational` (rate-limit notice, no findings in any section).
  - **sonarqubecloud:** `informational` (quality gate passed).
- **Skipped:** the `don-petry` dev-lead status comment, since it is our own automation.
- **Tests:** I ran only `bats tests/persona_mention.bats`. The tail of its output shows tests 112–116 passing. I did not check the rest of that file's output or run the full suite.
- **Files changed:** none.
The `fixed` disposition on the codeant-ai comment may not verify. c781ea5 may not remove a token that comment names, and the harness checks for that. If it doesn't verify, the comment stays open.

[don-petry]

No description provided.

[don-petry]

No description provided.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[don-petry]

No description provided.

[donpetry-bot]

No description provided.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

## Summary
**Bot:** coderabbitai[bot]  
**Issues addressed:** 0  
**Findings:** None — comment is a rate-limit notice only  
The CodeRabbit comment reports a rate-limit block: the free OSS review quota was temporarily exhausted. This is a non-actionable operational notice with **no code findings or suggestions to address**. The PR's changed files have no defects flagged, all CI checks pass, and no reviews have requested changes.
**Disposition posted:** Marked the original comment as `informational` (no code changes needed). The harness will minimize it once verified.

[donpetry-bot]

No description provided.

[don-petry]

No description provided.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

No description provided.

[don-petry]

Re-read after edit: CodeAnt's current body is a Low Risk summary for 8fac7e3 with no findings; nothing to act on.

[don-petry]

Re-read after edit: still only the rate-limit block plus the Autofix checkbox. No Security Architecture section, no actionable/outside-diff/nitpick findings; nothing to act on.

[don-petry]

Acknowledged: SonarCloud Quality Gate passed with 0 new issues and 0 hotspots; nothing to act on.

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

Addressed 0 threads. There were no open review threads, so I made no code changes and no commits.
- **Tier 1 blockers:** none. Every CI check is success, neutral or skipped, and no review has `CHANGES_REQUESTED`.
- **Phase 1b, issue comments:** I posted three `informational` dispositions, one each for:
  - **CodeAnt:** its body was edited after my last disposition, and the current body is a Low Risk summary with no findings.
  - **CodeRabbit:** it was also edited after my last disposition, and it still holds only the rate-limit block and the Autofix checkbox, with no findings.
  - **SonarCloud:** it is new, and the quality gate passed with 0 new issues.
- **Gemini's regex suggestion:** the `$` anchor fix in `bot_logins_compared` (commit 3f7b1df) was already on the branch, so I did nothing further on it.
- **Test verification:** I did not run the suite, since I changed no code; CI's "Lint and bats" job is green.
Files changed: none.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: 3f7b1df8bc2b55ae9cbb6b79bb65c7f99e81b61c
Review mode: triage-approved (single reviewer)

Summary

Test-only PR (1 file, +156/-0 in tests/persona_mention.bats). It adds 13 bats tests that parse persona-mention-reusable.yml with PyYAML and pin jobs.route.if and jobs.route-pull-request.if clause by clause: pure && conjunction, exact clause counts, recursion-guard marker semantics, handle check, bot-login exclusions, same-repo check and trust floor. It also checks that the hardcoded bot logins match pm_bot_logins / pm_is_bot_actor. The workflow and library are untouched, so runtime behavior does not change.

Linked issue analysis

Closes #1288. AC1: met. The tests read jobs[]["if"] through yaml.safe_load, not the file's raw text, so a YAML comment cannot satisfy them. AC2: met. route.if has !contains(comment.body, '<!-- persona:'), the @petry-projects/ handle, both bot exclusions and the trust floor, and startsWith/endsWith variants are rejected. AC3: met. route-pull-request.if has !startsWith(pull_request.body…), a negative check for !contains(pull_request.body, the same-repo check, bot exclusions on both pull_request.user.login and github.actor, and the trust floor. AC4: met. Test 20 compares the logins excluded on each identity against pm_bot_logins, and fails on an unpaired exclusion on any other identity. AC5 (the PR description lists the workflow edit that makes each assertion fail): not met in the PR body. I verified it independently instead (see Findings), so the issue's intent of proving the assertions bite is satisfied.

Findings

No blocking findings.

Verification performed (at head 3f7b1df):

• Full suite: bats tests/persona_mention.bats gives 124/124 ok.
• Mutation checks against the workflow. Each was reverted afterwards, and the tree was clean when done:
  • route.if: !contains(comment.body, '<!-- persona:') changed to !startsWith(…) (the gap #1288 was filed for): 1 test fails ✓
  • route.if: the comment.user.login != 'donpetry-bot' line deleted: 3 tests fail ✓
  • route-pull-request.if: a draft == false clause added: 1 test fails (the clause-count test) ✓
  • route-pull-request.if: !startsWith(pull_request.body…) changed to !contains(…): 1 test fails ✓

Non-blocking notes:

• AC5 of #1288 asks the PR description to list, per assertion, the workflow edit that makes it fail. The description doesn't have that list. The mutation results above cover the most important cases; consider adding the full list to the PR body for the record.
• An earlier author comment says the marker test pins !contains semantics for the PR job. Later commit "align route-pull-request marker test with startsWith policy" correctly changed this to !startsWith, which matches AC3 and #1286. That comment is now out of date, but the code is correct.
• Secret scan: the run_secret_scanning MCP tool isn't available in this run, so it was skipped. gitleaks CI passed, and the diff contains no credentials.

All 3 review threads (gemini, codeant ×2) are resolved and were addressed in code. No CHANGES_REQUESTED reviews and no open human questions.

CI status

All checks pass: Lint and bats, ShellCheck, CodeQL (actions/python), SonarCloud (quality gate passed, 0 issues), Secret scan (gitleaks), AgentShield, Agent Security Scan, dependency-audit, duplicate-decl-gate. Some checks were skipped as not applicable (dependabot, pnpm/cargo/go/pip audits, dev-lead resume/ci-relay). cubic reported neutral.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

[donpetry-bot]

pr-review approved on PARTIAL advisory evidence: 4/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).