Skip to content

feat: implement issue #2187 — [qa-lead reach] Subscribe this repo's persona-mention stub to pull_request — the router-served PR advisory is live on v1-next but nothing triggers it - #2188

Merged
don-petry merged 3 commits into
mainfrom
dev-lead/issue-2187-20261010-1147
Oct 10, 2026
Merged

don-petry merged 3 commits into
mainfrom
dev-lead/issue-2187-20261010-1147

Conversation

@don-petry

@don-petry don-petry commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

[qa-lead reach] Subscribe this repo's persona-mention stub to pull_request — the router-served PR advisory is live on v1-next but nothing triggers it

From the issue: Epic #1643, Phase 2. The router now serves the pull_request event (petry-projects/.github#1167, merged as fb556603) and that code is live on this repo's channel: persona-mention/v1-next → persona-mention/v1.19.0 → fb556603 (cut 2026-10-10 11:26Z). The runner side is on main too (#1909). Nothing triggers it, because .github/workflows/persona-mention.yml here does not subscribe to pull_request.

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

Tests added/updated: tests/persona_reach_check.bats,tests/test_validate_interaction_model.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

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

Closes #2187

View guided diff Turn on auto-fix

…ersona-mention stub to pull_request — the router-served PR advisory is live on v1-next but nothing triggers it
@don-petry
don-petry requested a review from a team as a code owner October 10, 2026 12:06
@don-petry don-petry added the auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label) label Oct 10, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

This comment has been minimized.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the QA Lead persona configuration and validation scripts to support the transition of the pull_request event surface to the shared persona-mention router while temporarily keeping the local workflow active. It ensures that mention-only personas do not automatically inherit the pull_request trigger from the shared router by updating the reachability checks and interaction model validation. Feedback on the changes suggests refactoring the conditional check in scripts/validate-interaction-model.sh to avoid executing command substitutions directly inside conditional lists under set -e and to prevent masking exit codes during local variable assignment.

Comment thread scripts/validate-interaction-model.sh Outdated
Comment thread scripts/validate-interaction-model.sh Outdated
@codeant-ai

codeant-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown

CodeAnt Nitpicks

1 code suggestion

1. dev-lead and pr-review both declare pull_request:synchronize, but this trigger omits it, so the router never receives those declared events.

Incomplete implementation · .github/workflows/persona-mention.yml:65-66

@codeant-ai

codeant-ai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

CodeAnt PR Risk: Medium Risk

  • The PR needs attention before merging because the interaction-model validator can treat a pull_request surface as served even when its declared actions do not overlap the router's subscribed actions.
  • In scripts/validate-interaction-model.sh, the check only requires enabled: true, so a contract may pass despite the router never dispatching that persona for pull_request events.

Assessed commit: 96e6e51c42aa

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@don-petry

Copy link
Copy Markdown
Collaborator Author

QA Lead — test-risk advisory

Risk tier: LOW. This adds a trigger and a validator exemption that one revert undoes. The worst realistic outcome is a duplicate or stray advisory comment on this repo, which is noise and doesn't damage any data. The validator changes have good positive and negative unit coverage.

What I'd shore up (highest leverage first):

  • The "one advisory, not two" claim only holds if both paths build the same concurrency group, and no test checks that. The reusable's group is persona-runner-<persona>-<repo>-<item_number>-<comment_url>, so the router's item_number and comment_url must render exactly like the local job's ('', no type or format drift). Add a contract test that builds the group string from both callers' with: inputs and asserts they match. On the first real PR after merge, confirm exactly one advisory posts and the other run logs skip:already-advised.
  • Edge cases for the imv_persona_serves_pr_surface awk parser. A false negative quietly strips pull_request from a contract and lets drift through. Add fixtures for enabled: written before surface: in the list item, a quoted 'pull_request', enabled: "true", and a - surface: under a different top-level key.
  • Make the "don't burn runners" rule enforceable. The stub subscribes to reopened, but qa-lead doesn't declare it. Add a test that the stub's pull_request.types is a subset of all events personas declare on pull_request, so the rationale is checked by CI and not just written in a comment.
  • Router-path skip parity. Add one table-driven case for persona_event_pregate with a draft opened followed by ready_for_review, matching AC feat: add Copilot engine support via REVIEW_ENGINE toggle #5 in the local gate.

Escalate? no

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture
(bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

…cribed action overlap

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (applied)

Changes committed and pushed. Requested items addressed:

  • scripts/validate-interaction-model.sh:576 — applied
  • scripts/validate-interaction-model.sh:420 — applied

@don-petry
don-petry enabled auto-merge October 10, 2026 12:44
@sonarqubecloud

Copy link
Copy Markdown

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-10T14:03:13Z.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 10, 2026 13:27
@don-petry
don-petry merged commit 4977028 into main Oct 10, 2026
70 of 82 checks passed
@don-petry
don-petry deleted the dev-lead/issue-2187-20261010-1147 branch October 10, 2026 13:28
@don-petry

Copy link
Copy Markdown
Collaborator Author

This is a rate-limit notice from CodeRabbit: the bot has used its free OSS review quota for the current window. The notice indicates no actionable code findings — only that the code review automation was throttled, not that the PR has issues. CI checks are passing and all other reviews are complete.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.

don-petry added a commit that referenced this pull request Oct 10, 2026
…sory when the posting-identity read fails — an API error body is read as a wrong login (first router-served PR dispatch, #2188) (#2198)

* feat: implement issue #2196 — persona-runner discards a finished advisory when the posting-identity read fails — an API error body is read as a wrong login (first router-served PR dispatch, #2188)

* fix(reviews): mktemp error handling and redact-before-truncate in pr_read_posting_login

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

* fix(template-drift): sync dev-lead.yml with standards/v1-stable

Removed workflow-tuning guidance and ring-pinning comments that are
specific to petry-projects/.github standards but not part of the
adopting repo's stub. This file has no OIDC byte-for-byte constraint
and may be freely modified on PR branches, unlike claude.yml.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(ci): restore dev-lead.yml caller stub to main (out-of-scope edit tripped caller-stub-freeze/template-drift)

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[qa-lead reach] Subscribe this repo's persona-mention stub to pull_request — the router-served PR advisory is live on v1-next but nothing triggers it

2 participants