Skip to content

persona-mention router ignores stop_markers (needs-human-review, dev-lead:needs-human) — a human hold stops dev-lead but not a mentioned persona #1133

Closed

Description

@don-petry

Story

As a maintainer who applies needs-human-review to stop automation on an item,
I want a mentioned persona to honour that brake the way dev-lead does,
so that a human hold stops every persona surface, not only the event-driven ones.

Filed here from combsmith/.github-private#1745, because the mention path's gating lives in this repo's persona-mention-reusable.yml. .github-private reaches it through a thin caller stub pinned to persona-mention/v1-next, which AGENTS.md forbids patching. The consuming-side work (reconciling interaction.yml, the validator) stays on .github-private#1745 and must not land until this ships.

The gap (verified on main, 2026-09-13)

Each persona's contract in petry-projects/.github-private (personas/<id>/interaction.yml) declares stop markers. For example, qa-lead declares:

stop_markers:
  - qa-lead:hands-off
  - needs-human-review
  - dev-lead:needs-human

The router honours only the first. In .github/workflows/persona-mention-reusable.yml, the only label check in the per-slug loop is the opt-out:

if [ -n "$opt_out" ] && printf '%s\n' "$LABELS" | grep -qxF "$opt_out"; then
  echo "Item carries '${opt_out}' — persona '${slug}' is opted out here. Skipping."

needs-human-review and dev-lead:needs-human are never consulted, so a mention on a held item still dispatches the persona. The pull_request surface in .github-private (scripts/qa-lead-advisory-gate.sh) honours all three markers, which leaves the same persona inconsistent across its surfaces. This is fail-open in the direction that matters.

Decisions — pre-answered (implement them, do not re-ask)

  1. Put the check in the router (persona-mention-reusable.yml), in the per-slug loop right after the opt-out check. The router already holds $LABELS / $LABELS_KNOWN; the runner does not.
  2. Derive the markers from the persona's own contract; never write them as literals. Fetch personas/<slug>/interaction.yml from petry-projects/.github-private at the same ref and by the same unauthenticated raw.githubusercontent pattern as the manifest (pm_manifest_url). Add pm_interaction_url and a pm_stop_markers query to scripts/lib/persona-mention.sh alongside pm_mention_decision. The contract is then true by construction, and no marker list is duplicated across repos (.github-private#1745 AC Add multi-agent isolation strategy using git worktrees #2).
  3. Fetch failures mirror the manifest fetch exactly. A non-200, non-404 response is exit 1: never read a fetch failure as "no stop markers". A 404 for a persona whose manifest exists means refuse to route that slug with ::warning::no interaction contract for '<slug>'. Before relying on that, list which personas/*/ in .github-private lack interaction.yml and put the list in the PR. If any persona lacks one, say so plainly; do not silently fall back to routing on the opt-out alone.
  4. Discussions: LABELS_KNOWN=false, so stop markers cannot be evaluated, exactly as the opt-out cannot today. Keep the existing discussion semantics (read-mode routes; write-mode is already refused because the gate cannot be verified). Document that explicitly in the workflow's header comment and in standards/persona-standards.md next to §4 rule 4. Do not leave it implicit.
  5. Do not repoint any caller stub and do not cut a channel tag. After merge, a maintainer promotes persona-mention/v1-next via cut-release.sh; note that step in the PR description.

Acceptance Criteria

  1. A mention on an issue or PR carrying any of the persona's declared stop_markers is skipped with a log line naming the marker, and dispatches nothing.
  2. The marker list is read from interaction.yml; there is no literal needs-human-review in the routing logic.
  3. An interaction.yml fetch that is neither 200 nor 404 fails the job (fail-closed). A 404 refuses to route that slug.
  4. Discussion behaviour is stated in the workflow header and in persona-standards.md.
  5. Tests in the existing persona-mention test suite (persona-mention-tests.yml) cover: marker present → skipped; marker absent → routed; opt-out still honoured; fetch 5xx → failure; 404 → refused; discussion → documented behaviour.
  6. shellcheck --severity=warning -x is clean, and SHA pins are unchanged.

Budget guard

  1. Helper functions, the router check and tests (AC Addressing PR comments #1–Add coding standards & principles section to AGENTS.md #3, Add stacked PR strategy and Epic-level workflow guidance #5). This is the commit that matters.
  2. Documentation (AC Add workflow, environment, and orchestration guidance #4).

If you run low on budget, stop after step 1 with a green PR and list what remains.

References

Activity

  1. don-petry commented on Sep 13, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead Implementation Plan

    Issue: #1133 — persona-mention router ignores stop_markers

    Scope

    Teach the persona-mention router to honour each persona's declared stop_markers (read from its own interaction.yml contract) so a human hold — needs-human-review, dev-lead:needs-human, <id>:hands-off — stops a mentioned persona exactly as it stops the event-driven surfaces, with no marker list duplicated as a literal.

    Implementation Checklist

    • scripts/lib/persona-mention.sh: add pm_interaction_url <slug> (mirrors pm_manifest_url, path personas/<slug>/interaction.yml, same unauthenticated raw pattern).
    • scripts/lib/persona-mention.sh: add pm_stop_markers <interaction-yaml> — the pure query that emits the contract's declared stop_markers, one per line.
    • scripts/lib/persona-mention.sh: add pm_first_stop_marker <interaction-yaml> — reads the item's labels from stdin, emits the first declared marker present (the pure "is this item held" decision; newline-delimited whole-line match, safe for label names with spaces).
    • scripts/lib/persona-mention.sh: add pm_fetch_disposition <http_code> — pure classification (read | absent | fail) mirroring the manifest-fetch status handling, so the fetch decision is unit-testable.
    • .github/workflows/persona-mention-reusable.yml: in the per-slug loop, right after the opt-out check, fetch interaction.yml (unauthenticated, mirroring the manifest fetch: 200 → read; 404 → ::warning::no interaction contract for '<slug>' and refuse to route that slug; any other status → exit 1), then skip the slug (naming the marker) if any declared marker is present. Guarded by LABELS_KNOWN so discussions keep their existing semantics.
    • .github/workflows/persona-mention-reusable.yml: document the discussion behaviour in the header comment.
    • standards/persona-standards.md: document the discussion behaviour next to §4 rule 4.

    Tests to Write (tests/persona_mention.bats, gated by persona-mention-tests.yml)

    Test Command

    bats tests/persona_mention.bats (plus bash .dev-lead/scripts/dev-lead-lint.sh for shellcheck)

    Interaction contracts audit

    I will list which personas/*/ in .github-private lack an interaction.yml and include it in the PR (per decision 3). No caller stub is repointed; a maintainer promotes persona-mention/v1-next via cut-release.sh after merge.

  2. don-petry commented on Sep 13, 2026

    @don-petry
    ContributorAuthor

    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)

    • Added pm_interaction_url, pm_stop_markers, pm_first_stop_marker, and pm_fetch_disposition to scripts/lib/persona-mention.sh (all pure/network-free).
    • Wired the stop-marker check into persona-mention-reusable.yml, in the per-slug loop right after the opt-out check, guarded by LABELS_KNOWN. The interaction-contract fetch mirrors the manifest fetch exactly (200 → read; 404 → ::warning::no interaction contract + refuse this slug; anything else → exit 1).
    • Documented the discussion exception in the workflow header and in standards/persona-standards.md §4 rule 4.
    • Extended tests/persona_mention.bats.

    Interaction-contract audit (decision 3 — personas/*/ in .github-private @ main)

    Every persona declares an interaction.yml with stop_markers — none lack one, so the 404-refuse path is a safety net that does not fire for any current persona:

    persona interaction.yml stop_markers
    business-analyst ✅ 200 business-analyst:hands-off
    dev-lead ✅ 200 needs-human-review, dev-lead:needs-human, dev-lead:hands-off
    devops-lead ✅ 200 devops-lead:hands-off
    pr-review ✅ 200 needs-human-review, pr-review:hands-off
    qa-lead ✅ 200 qa-lead:hands-off, needs-human-review, dev-lead:needs-human
    scrum-master ✅ 200 scrum-master:hands-off
    security-lead ✅ 200 security-lead:hands-off
    solution-architect ✅ 200 solution-architect:hands-off
    sre-lead ✅ 200 sre-lead:hands-off

    Local check output (provisional — not the pushed run)

    bats tests/persona_mention.bats — 61/61 ok (local)
    shellcheck --severity=warning -x scripts/lib/persona-mention.sh — clean
    bash .dev-lead/scripts/dev-lead-lint.sh — all checks passed
    SHA pins in persona-mention-reusable.yml — unchanged
    

    Notes

    • No caller stub repointed and no channel tag cut (decision 5). After merge, a maintainer promotes persona-mention/v1-next via cut-release.sh; the consuming-side work stays on .github-private#1745 and must not land until this ships.
    • pm_stop_markers reads .interaction.stop_markers (the real nested shape) with a .stop_markers top-level fallback.
    • Fail-closed hardening: a 200 with an unparseable body propagates a failure rather than being read as "no markers", same direction as a 5xx.
  3. don-petry commented on Sep 13, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead: Implementation Complete — PR #1134

    The implementation for issue #1133 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.

  4. added a commit that references this issue on Sep 20, 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

    bugBug reportsdev-leadFor dev-lead agent pickup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions