Skip to content

[qa-lead S4] Make persona.yml's declared surfaces agree with what is actually deployed #1647

Closed
#1790
Closed
[qa-lead S4] Make persona.yml's declared surfaces agree with what is actually deployed#1647
#1790
Labels
dev-leadFor dev-lead agent pickupenhancementFeature requestsinitiativeEpic / initiative tracking issue

Description

@don-petry
don-petry
opened on Sep 1, 2026

Story

As a reader of personas/qa-lead/persona.yml,
I want its triggers.surfaces to describe what the runtime actually serves,
so that the manifest — which persona-standards §1.1 calls the index-of-record — stops advertising an interaction contract nothing honors.

Blocked by #1646, which decides which surfaces are genuinely served.

The problem

personas/qa-lead/persona.yml declares six surfaces as enabled: true, mode: advisory:

Surface Served?
mention yes
pull_request no (until #1646)
pull_request_review no
issues no
discussion no (declares a repository_dispatch bridge that nothing sends)
check_run no

personas/qa-lead/interaction.yml (#1404/#1418) already records the truth — "the contract records what is actually deployed, not the aspiration" — and cites combsmith/.github#755 finding 10. So the repo now holds two files that disagree, with the honest one in the newer, less-read location. The manifest is the one the standard calls authoritative.

This is not cosmetic. §4.1's whole argument for one router plus per-persona manifests is that the manifest is where per-persona decisions live and the router restates none of them. A manifest that claims six surfaces while one is wired means any future router reading it would fire five dead paths.

Acceptance Criteria

  1. After [qa-lead S3] Serve the pull_request surface so qa-lead runs without being summoned #1646, personas/qa-lead/persona.yml lists exactly the surfaces that are served as enabled: true. Every unserved surface is enabled: false — kept, not deleted, with a note recording that it is an intended future surface and what would be needed to serve it. Deleting them loses the design intent; leaving them true is the lie.
  2. A validator invariant makes this class of drift impossible to reintroduce: personas/validate-personas.py fails when a persona declares a surface enabled: true that no deployed workflow serves. The mapping from surface to serving workflow must be derived — from personas/<id>/interaction.yml's workflows[] and triggers.events[], which already exist for exactly this purpose — not hand-maintained per persona.
  3. The invariant is covered by tests/test_validate_personas.bats: a persona with an enabled-but-unserved surface fails; one whose enabled surfaces are all served passes; a persona with no interaction.yml is handled explicitly (skip or fail — decide and document, do not crash).
  4. The same audit is run across the other eight personas and the findings reported. sre-lead, devops-lead, and security-lead were onboarded from the same template ([bmad-onboard S1] Onboard sre-lead persona #1307/[bmad-onboard S2] Onboard devops-lead persona #1308/[bmad-onboard S3] Onboard security-lead persona #1309) and very likely carry the identical over-declaration. Fix them in this PR if the fix is mechanical; file a follow-up if it is not.
  5. personas/qa-lead/interaction.yml and persona.yml agree after the change, and a test asserts they cannot silently diverge again.

Tasks / Subtasks

Dev Notes

  • validate-personas.py today checks id == directory == canary.agent, the address.handle slug invariant, handle/alias uniqueness, definition.layers[].path existence, vendor_pin vs VENDOR.md, the eval split structure, and the non-draft canary registration. Adding a surface-liveness invariant fits that existing shape — cross-invariants the JSON schema cannot express.
  • Prefer failing closed. A surface declared enabled: true with no serving workflow should fail the validator, not warn. The precedent is Rename murat → qa-lead and make it @-addressable #1279's handle-collision check, which fails because a collision means "a dropped mention or a double-fire" — the same category of silent wrongness.
  • Do not solve this by deleting the surfaces. The discussion surface in particular encodes a real design decision (the repository_dispatch bridge, issue fix(engine): reorder cross-provider fallback to claude → copilot → gemini #571) that should survive as enabled: false.
  • Draft status does not exempt a persona from this. Eight of nine personas are draft; if draft meant "the manifest may lie," the manifest would be worthless during exactly the period it is being copied from.

Project Structure Notes

personas/qa-lead/persona.yml, personas/validate-personas.py, tests/test_validate_personas.bats, and likely the other seven personas/*/persona.yml.

References

Update — 2026-09-01, cross-session reconciliation

Update — 2026-09-13, budget guard (read before starting)

This issue has a real timeout risk. Sequence it so a budget overrun costs the fleet audit, never the core fix.

#1734 timed out attempting two ACs in one run and had to be split (#1775/#1776) after the work was
lost. Do not repeat that here. Implement in this order and commit as you go:

  1. AC test issue from agent #1–Add @claude delegation, auto-merge, and rebase handling #3 for qa-lead only — the manifest correction, the derived surface↔workflow invariant
    in validate-personas.py, and the bats coverage. This is the deliverable; it must be complete and
    green on its own.
  2. AC feat: add Copilot engine support via REVIEW_ENGINE toggle #5 — the manifest/contract agreement assertion.
  3. AC Optimize review: small-PR and incremental fast paths #4, the eight-persona audit, last. Fix only what the existing mechanical edit covers
    (flipping enabled: true → enabled: false with the reason note). If the remaining personas need
    per-persona judgment, or if you are running short on budget, stop, report the audit findings in
    the PR description, and open a follow-up issue     — a partial PR with an honest findings table is a
    good outcome; a timed-out run that loses everything is not.

Two further constraints:

  • AC Go-live improvements for PR review agent #2's invariant must not red-light the fleet on merge. Nine personas are in the tree and eight
    are draft. If the new check fails closed before those manifests are corrected, it gates every PR in
    this repo — the exact outage pattern of the ADR-0006 schema change on 2026-09-07. Either correct all
    nine manifests in this PR, or land the invariant and the corrections together in the same commit.
    Do not land a failing-closed validator ahead of the data it validates.
  • The one open question is already answered: for a persona with no interaction.yml, SKIP (do not
    fail) — and log a ::notice:: naming the persona. Rationale: interaction.yml is the derivation
    source, and failing on its absence would punish a persona for a missing contract file rather than
    for an over-declared surface, which is a different defect with its own issue. Document this in the
    validator docstring and cover it with the AC Add @claude delegation, auto-merge, and rebase handling #3 bats case.

Steering reaches this agent through the issue title and body only — a question posted as an issue
comment will not be read and will not be answered. If something is still ambiguous, pick the reading
that best satisfies the ACs above, implement it, and state the assumption in the PR description.

Activity

  1. don-petry
    added
    enhancementFeature requests
    initiativeEpic / initiative tracking issue
    on Sep 1, 2026
  2. don-petry
    added a parent issue on Sep 1, 2026
  3. don-petry
    marked this as blocked on Sep 1, 2026

  4. don-petry commented on Sep 8, 2026

    @don-petry
    don-petry
    on Sep 8, 2026
    CollaboratorAuthor

    Evidence: the discussion surface is not merely unwired — the runner explicitly refuses to post

    Found via a codeant-ai review finding on PR #1689 (2026-09-08) and verified against main.

    AC #1 asks that unserved surfaces be set enabled: false with the reason recorded. For discussion the reason is stronger and more specific than "the router does not wire it" — the shared runner has an explicit guard that skips the post:

    .github/workflows/persona-runner-reusable.yml, lines 285-286:

    if [ -z "${ITEM_NUMBER}" ]; then
  echo "::notice::discussion advisory not yet supported by the runner post-path — skipping post for ${SOURCE_REPO}"

    Discussions carry no item_number (the runner's own concurrency comment says so: "item_number is empty for discussions, so key on comment_url too"), so the guard always fires on that surface.

    What this means for the manifest. personas/qa-lead/persona.yml currently declares:

    - surface: discussion
  events: [labeled, commented]
  enabled: true
  mode: advisory
  bridge: repository_dispatch
  notes: "Discussions can't run the agent inline (#571) — must bridge (issue #571)."

    That note describes the inbound bridge, which is real — the router does dispatch. What it omits is that the outbound path is a no-op: the agent runs, burns a model call, and posts nothing. So a discussion mention today is pure cost with zero output, which is worse than a surface that simply never fires.

    Suggested AC #1 addendum: when setting discussion to enabled: false, record both halves — inbound bridging works, outbound posting is unimplemented — and cite the runner line above, so a future contributor re-enabling it knows the post-path is the work, not the router.

    Two related notes:

    Origin: unresolved review thread on #1689, dispositioned there rather than fixed in a docs-only PR.

  5. don-petry
    added
    dev-leadFor dev-lead agent pickup
    on Sep 13, 2026

  6. don-petry commented on Sep 13, 2026

    @don-petry
    don-petry
    on Sep 13, 2026
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #1647 — [qa-lead S4] Make persona.yml's declared surfaces agree with what is actually deployed

    Scope

    Correct qa-lead's manifest so every enabled: true surface is one a deployed workflow actually serves, add a derived surface↔workflow invariant to validate-personas.py (derived from each persona's interaction.yml), cover it with bats, and apply the mechanical fix across the other eight personas.

    Derivation (AC #2)

    A persona surface is served iff personas/<id>/interaction.yml's interaction.triggers.events[] contains an event that serves it:

    • pull_request→pull_request, pull_request_review→pull_request_review, issues→issues, check_run→check_run/check_suite, discussion→discussion, schedule→schedule
    • mention→ any comment event (issue_comment/pull_request_review_comment/discussion_comment) or a repository_dispatch:* bridge (covers pr-review).
      No interaction.yml → SKIP (log ::notice::, do not fail) — it is the derivation source; its absence is a different defect.

    Audit result (all 9 personas)

    • qa-lead: served = {mention, pull_request}. Flip pull_request_review, issues, discussion, check_run → enabled: false + notes.
    • business-analyst, devops-lead, scrum-master, security-lead, solution-architect, sre-lead (shared mention router): served = {mention}. Flip issues, discussion → enabled: false + notes. (mechanical — fixed here)
    • dev-lead: all declared surfaces served → no change.
    • pr-review: all declared surfaces served (mention via repository_dispatch:pr-review-mention) → no change.

    Implementation Checklist

    Tests to Write (tests/test_validate_personas.bats)

    Test Command

    scripts/run-bats.sh tests/test_validate_personas.bats and bash .dev-lead/scripts/dev-lead-lint.sh

  7. don-petry commented on Sep 13, 2026

    @don-petry
    don-petry
    on Sep 13, 2026
    CollaboratorAuthor

    Dev-Lead: Progress (in progress — not yet durable)

    Provisional. Nothing is pushed yet; the automation posts the authoritative
    completion record (with PR number + head SHA) after the push. Treat the notes
    below as intent, not delivered results.

    Plan Execution (intended)

    Local check output (provisional — not the pushed run)

    bats tests/test_validate_personas.bats  -> 45/45 ok (local)
validate-personas.py (all 9 real manifests, local schema/registry) -> "OK: 9 persona manifest(s) valid, invariants hold."
.dev-lead/scripts/dev-lead-lint.sh -> all checks passed (local)

    Files touched (this workspace, not yet pushed)

    • personas/validate-personas.py — surface-liveness invariant + docstring.
    • personas/qa-lead/persona.yml, personas/qa-lead/interaction.yml — reconciliation.
    • personas/{business-analyst,devops-lead,scrum-master,security-lead,solution-architect,sre-lead}/persona.yml — flip issues/discussion.
    • tests/test_validate_personas.bats — 7 new cases.

    Notes

    • No follow-up filed: the entire AC Optimize review: small-PR and incremental fast paths #4 audit was mechanical (no per-persona judgment needed), so nothing was deferred.
    • Assumption stated for the reviewer: the derived mapping treats the mention surface as served by the comment webhooks or a repository_dispatch:* bridge (so pr-review stays green), while discussion requires a literal discussion/dispatch event it does not have. This reproduces the issue's qa-lead truth table exactly.

  8. don-petry commented on Sep 13, 2026

    @don-petry
    don-petry
    on Sep 13, 2026
    CollaboratorAuthor

    Dev-Lead: Implementation Complete — PR #1790

    The implementation for issue #1647 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review 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.

  9. don-petry
    added a commit that references this issue on Sep 19, 2026

    feat: implement issue #1647 — [qa-lead S4] Make persona.yml's declare…

    b9a68d8
  10. don-petry
    closed this as completedin #1790on Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dev-leadFor dev-lead agent pickupenhancementFeature requestsinitiativeEpic / initiative tracking issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions