Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .github/workflows/persona-mention-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,13 @@ jobs:
# triggered. Same two recursion axes and conservative §4 default floor as the
# comment path's pre-filter — a PR event is no laxer:
# 1. bot actor — a PR opened by an agent identity never routes
# 2. marker — a PR body carrying '<!-- persona:' never routes
# 2. marker — a PR body AUTHORED as a persona (it starts with
# '<!-- persona:', pr_ensure_marker's convention) never routes.
# A marker quoted later in the body does not count (#1286):
# no persona writes mid-body, so that is a quotation. This
# startsWith is the cheap pre-filter; pm_pr_should_route
# (pm_is_agent_authored_body) stays authoritative and also
# skips leading blank lines and whitespace.
# There is deliberately NO '@petry-projects/' check: the PR path derives the
# firing personas from their manifests, it does not read a handle from the body.
#
Expand All @@ -412,7 +418,7 @@ jobs:
github.event.pull_request.user.login != 'github-actions[bot]' &&
github.actor != 'donpetry-bot' &&
github.actor != 'github-actions[bot]' &&
!contains(github.event.pull_request.body, '<!-- persona:') &&
!startsWith(github.event.pull_request.body, '<!-- persona:') &&
contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.pull_request.author_association)

steps:
Expand Down
21 changes: 20 additions & 1 deletion scripts/lib/persona-mention.sh
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@
esac
}

# pm_is_agent_authored_body <body> — 0 if this body was AUTHORED as a persona:
# its first non-blank line begins with the agent marker (leading blank lines and
# leading whitespace ignored). Axis 2 of the PR path's recursion guard. Unlike a
# comment, a PR body is never written by a persona mid-text — the runner's
# pr_ensure_marker prepends the marker as the first line — so a marker anywhere
# else is a quotation, and substring matching would skip that PR silently (#1286).
pm_is_agent_authored_body() {
local line
while IFS= read -r line || [ -n "$line" ]; do

Check failure on line 89 in scripts/lib/persona-mention.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.

See more on https://sonarcloud.io/project/issues?id=petry-projects_.github&issues=AaEnt95b5rlBucsWitai&open=AaEnt95b5rlBucsWitai&pullRequest=1287
line="${line#"${line%%[![:space:]]*}"}"
[ -n "$line" ] || continue

Check failure on line 91 in scripts/lib/persona-mention.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.

See more on https://sonarcloud.io/project/issues?id=petry-projects_.github&issues=AaEnt95b5rlBucsWitaj&open=AaEnt95b5rlBucsWitaj&pullRequest=1287
case "$line" in
"$PERSONA_AGENT_MARKER"*) return 0 ;;
*) return 1 ;;
esac
done <<< "$1"

Check warning on line 96 in scripts/lib/persona-mention.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Assign this positional parameter to a local variable.

See more on https://sonarcloud.io/project/issues?id=petry-projects_.github&issues=AaEnt95b5rlBucsWitak&open=AaEnt95b5rlBucsWitak&pullRequest=1287
return 1
}

# pm_extract_slugs <body> — emit each DISTINCT persona slug addressed in body.
#
# Matches '@<org>/<slug>' where slug is kebab-case, mirroring the schema's
Expand Down Expand Up @@ -533,7 +552,7 @@

pm_is_bot_actor "$author" && return 1 # axis 1a: bot PR author
pm_is_bot_actor "$actor" && return 1 # axis 1b: bot pusher/reopener (github.actor)
pm_is_agent_comment "$body" && return 1 # axis 2: agent marker
pm_is_agent_authored_body "$body" && return 1 # axis 2: body authored with the marker
pm_trust_ok "$assoc" OWNER MEMBER COLLABORATOR || return 1
return 0
}
Expand Down
69 changes: 69 additions & 0 deletions tests/persona_mention.bats
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,41 @@ MENTION_ON=' - surface: mention
[ "$status" -ne 0 ]
}

# pm_is_agent_authored_body is the PR-path axis 2: a PR body is AUTHORED as a
# persona only when the marker leads it (pr_ensure_marker's convention). A marker
# anywhere else is a quotation and must not suppress routing (#1286).

@test "pm_is_agent_authored_body matches a body whose first line is the marker" {
run pm_is_agent_authored_body $'<!-- persona:qa-lead -->\n## Summary\nautomated PR'
[ "$status" -eq 0 ]
}

@test "pm_is_agent_authored_body ignores leading blank lines and indentation" {
run pm_is_agent_authored_body $'\n\n \n<!-- persona:qa-lead -->\nbody'
[ "$status" -eq 0 ]
run pm_is_agent_authored_body $' \t<!-- persona:dev-lead -->\nbody'
[ "$status" -eq 0 ]
run pm_is_agent_authored_body $'\r\n<!-- persona:dev-lead -->\r\nbody'
[ "$status" -eq 0 ]
}

@test "pm_is_agent_authored_body does not match a marker quoted mid-body in prose" {
run pm_is_agent_authored_body $'## Summary\n\nThe `<!-- persona:solution-architect -->` marker is unchanged.'
[ "$status" -eq 1 ]
}

@test "pm_is_agent_authored_body does not match a marker inside a later fenced code block" {
run pm_is_agent_authored_body $'## Summary\n\n```\n<!-- persona:qa-lead -->\n```'
[ "$status" -eq 1 ]
}

@test "pm_is_agent_authored_body does not match an empty or blank body" {
run pm_is_agent_authored_body ''
[ "$status" -eq 1 ]
run pm_is_agent_authored_body $'\n \n'
[ "$status" -eq 1 ]
}

@test "pm_is_bot_actor recognises the agent identities" {
run pm_is_bot_actor donpetry-bot
[ "$status" -eq 0 ]
Expand Down Expand Up @@ -693,6 +728,11 @@ runtime:
# Same two recursion axes and trust floor as pm_should_route, but WITHOUT the
# @-mention requirement: a PR is derived, not addressed. A PR event must not
# become an ungated write surface, so the guards apply on this path too (AC #4).
#
# The workflow's pull_request pre-filter uses !startsWith(body, '<!-- persona:')
# for axis 2 (not substring contains). The tests below verify this prefix-matching
# semantics: a marker quoted mid-body does NOT block routing, but a marker at the
# start (or after leading blank lines, per pm_is_agent_authored_body) DOES block.
Comment thread
don-petry marked this conversation as resolved.

@test "pm_pr_should_route allows a trusted human's PR with no @-mention in the body" {
run pm_pr_should_route don-petry don-petry OWNER "Ordinary PR description, no handles."
Expand All @@ -716,11 +756,40 @@ runtime:
[ "$status" -eq 1 ]
}

@test "pm_pr_should_route allows a trusted human's PR that QUOTES the marker on a later line (#1286)" {
run pm_pr_should_route don-petry don-petry OWNER $'## Summary\n\nThe `<!-- persona:solution-architect -->` marker is unchanged.'
[ "$status" -eq 0 ]
}

@test "pm_pr_should_route blocks a PR whose body leads with the marker after blank lines (axis 2)" {
run pm_pr_should_route don-petry don-petry OWNER $'\n\n<!-- persona:qa-lead -->\nautomated PR'
[ "$status" -eq 1 ]
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
@test "pm_pr_should_route blocks a PR from an author below the default floor" {
run pm_pr_should_route drive-by drive-by CONTRIBUTOR "PR body"
[ "$status" -eq 1 ]
}

# The workflow's route-pull-request job uses a cheap if: condition that must match
# the authoritative pm_pr_should_route function. The condition uses startsWith for
# the marker axis (not contains), matching pm_is_agent_authored_body's semantics:
# a marker quoted mid-body is not considered agent-authored. These verifications
# ensure a regression from startsWith back to contains would be caught by CI.
@test "pull_request workflow pre-filter uses prefix matching (startsWith, not contains)" {
local workflow="$SCRIPT_DIR/.github/workflows/persona-mention-reusable.yml"
# Inspect each job's own `if:` so a match elsewhere in the file cannot mask a
# lost guard.
local pr_if comment_if
pr_if="$(yq '.jobs.route-pull-request.if' "$workflow")"
comment_if="$(yq '.jobs.route.if' "$workflow")"
[[ "$pr_if" == *"!startsWith(github.event.pull_request.body, '<!-- persona:')"* ]]
[[ "$pr_if" != *"contains(github.event.pull_request.body, '<!-- persona:')"* ]]
# The comment path keeps substring matching: any persona marker in a comment
# means the comment is agent-authored.
[[ "$comment_if" == *"!contains(github.event.comment.body, '<!-- persona:')"* ]]
}

# --- pm_pr_route_verdict (the composed PR gauntlet — AC #2/#4 proof) --------
# Given a manifest that DECLARES the pull_request surface enabled and its
# interaction contract, decide the verdict for one PR: stop-marker → write-gate
Expand Down
Loading