Personas are roles, and roles are @-addressable via org teams#752
Conversation
…rg teams Two coupled changes to the agentic persona standard. 1. A persona is a role, not a person (new principle 1.6) `id`/`name` now name the ROLE — `qa-lead`, not `murat`. This is the naming half of the existing "unify over the frameworks" principle: an upstream framework may give its agent a person-name, and that stays upstream. We reference upstream agents only by their technical skill id (`framework.skill: bmad-tea`) and never mirror the person-name into our namespace. The schema previously mandated the opposite — `name` was documented as "MUST match the locked framework identity" — so every framework-backed persona was required to import upstream's cast list. That clause is gone. `dev-lead` and `pr-review` were already role-named; only the framework-backed personas drifted. One rule now, no exceptions. 2. Addressing: `@petry-projects/<role>` (new section 4.1, new `address` block) A persona that enables the `mention` surface must declare how it is addressed. The handle MUST be an org TEAM, never a user account. That constraint is load-bearing, not stylistic. GitHub's user namespace is global and first-come, and `@qa-lead`, `@dev-lead`, `@scrum-master`, `@business-analyst` and `@murat` are all real, live accounts today — none of them ours. A workflow keying on `contains(body, '@qa-lead')` would fire correctly and look perfectly healthy while publicly tagging a stranger and notifying them on every mention, from a fleet of bots, across public repos. The failure is silent on our side and visible only on theirs. An org team slug cannot collide, autocompletes when you type '@petry-projects/' in any comment box, and renders as a real link. Schema: - `address.handle` — `org/team-slug`, pattern-enforced to reject a bare user handle; slug MUST equal `id` so routing is a prefix-strip and the role name is written exactly once. - `address.aliases[]` — the rename migration path. - `address` is conditionally REQUIRED when a `mention` surface is enabled (mirrors the existing `mode: write` -> `gate_label` conditional). Live team properties (exists / privacy: closed / notifications_disabled) are deliberately NOT in validate-personas.py: that validator is hermetic with a hermetic bats suite, and its hermeticity is worth more than the check. They move to a CI step and the section 7 DoD instead. Also documents the recursion hazard for the router that will consume this: agent comments posted via a PAT re-trigger workflows, and .github-private#860 burned 1,481 acks in 4.5h from a single self-loop. With N mutually addressable personas the cycles are combinatorial, so the two-axis guard (bot actor + comment marker) that pr-review-mention.yml already carries is mandatory. Verification: - persona.schema.json is a valid Draft 2020-12 schema. - Conditional verified: mention+no-address FAILS, mention+address PASSES, no-mention-surface PASSES, bare `qa-lead` handle FAILS on pattern. - TEMPLATE/persona.yml validates against the updated schema. - markdownlint-cli2 clean on both changed .md files. The companion PR in .github-private renames the worked example murat -> qa-lead and fills in its address block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPersona standards now require role-based identities and define team-based addressing for enabled mention triggers. The schema and template add conditional ChangesPersona identity and mention addressing
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #752 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the persona standards, templates, and JSON schema to enforce role-based naming (e.g., 'qa-lead' instead of 'murat') and introduces a schema-enforced 'address' block for personas that use the 'mention' surface. This ensures mentions route to internal organization teams rather than external global GitHub accounts. The feedback suggests a minor grammatical correction in the standards document to pluralize 'rule 2 and 3' to 'rules 2 and 3'.
There was a problem hiding this comment.
Pull request overview
Updates the org’s agentic persona contract to treat personas as roles (not person-names) and to make mention-enabled personas explicitly @-addressable via an address block, with accompanying schema + template + standards documentation updates.
Changes:
- Define and document the “persona is a role, not a person” rule, updating template guidance and standards wording accordingly.
- Add an
addressblock concept (team-handle based) and document mention routing/recursion safety requirements in the standards. - Extend
persona.schema.jsonto conditionally requireaddresswhen thementionsurface is enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| standards/personas/TEMPLATE/README.md | Updates onboarding steps + worked example to be role-based and to include team addressing guidance for mention. |
| standards/personas/TEMPLATE/persona.yml | Adds the address scaffold and updates inline guidance for mention surfaces. |
| standards/personas/persona.schema.json | Adds address schema + conditional requirement when mention is enabled; updates id/name descriptions to be role-based. |
| standards/persona-standards.md | Documents the role naming principle and formalizes @-mention addressing rules + recursion hazard guidance. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #752 |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: fc3c44c3cf7886bdc0a56e2cee4b8652886d5ed7
Review mode: triage-approved (single reviewer)
Summary
Docs + template + JSON-schema change codifying two persona-standard rules: (1) persona id/name must name the role, never an upstream person-name (§1.6), and (2) personas with the 'mention' surface must declare an 'address' block whose handle is an org team (§4.1), preventing bare @ mentions from notifying stranger accounts in GitHub's global user namespace. No executable code changes. Triage's low-risk assessment is confirmed.
Linked issue analysis
No closing issue is linked; the PR body is self-contained with thorough rationale. Companion petry-projects/.github-private#1279 (rename murat → qa-lead, validator cross-file checks) is correctly sequenced to merge after this PR, since its validator fetches this schema from .github@main.
Findings
- Schema conditional is correct: 'address' becomes required only when a triggers.surfaces entry has surface=mention AND enabled=true (both fields required inside 'contains', so partial entries cannot false-match).
- Prior bot-review findings are all resolved: handle/alias pattern tightened to ^petry-projects/[a-z0-9]+(-[a-z0-9]+)*$ (Copilot x2) and the 'rules 2 and 3' grammar nit (Gemini), fixed in fc3c44c. All 3 review threads resolved; CodeRabbit approved.
- The slug==id and fleet-wide handle-uniqueness invariants are cross-file and deliberately deferred to validate-personas.py (companion PR) — documented in §4.1, consistent with the validator's hermeticity constraint.
- The recursion hazard note (§4.1) mandates the two-axis guard (bot actor + agent-comment marker) for any mention router, consistent with the #860 incident.
- Secret scanning MCP tool unavailable in this run; gitleaks CI check passed (no secrets in diff — docs/schema only).
CI status
All substantive checks green: Lint, ShellCheck, CodeQL, Analyze (actions), AgentShield, Agent Security Scan, Secret scan (gitleaks), SonarCloud (+ Quality Gate), npm audit, CodeRabbit. Ecosystem audits (cargo/pip/pnpm/govulncheck) skipped as not applicable. One 'dev-lead / dispatch' run shows cancelled — a concurrency-superseded agent run; the subsequent run on the same head SHA (fc3c44c) succeeded. 'dev-lead / ci-relay' skipped per the [skip ci-relay] commit trailer.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: fc3c44c3cf7886bdc0a56e2cee4b8652886d5ed7
Review mode: triage-approved (single reviewer)
Summary
Standards/documentation change: personas must be named after roles (not upstream person-names), and mention-enabled personas must declare an address block whose handle is an org team (@petry-projects/<id>). Touches 4 files: the persona standard (new §1.6 and §4.1), the TEMPLATE README/persona.yml, and persona.schema.json (new address property plus a conditional making it required when the mention surface is enabled). No executable code, no workflows, no secrets. The triage tier's low-risk assessment is confirmed.
Linked issue analysis
No linked issues (closingIssuesReferences is empty). The PR body itself documents the motivation thoroughly: a naming inconsistency (framework-backed personas imported upstream person-names) and a live hazard (bare @<role> handles collide with real first-come GitHub user accounts, silently notifying strangers). The diff substantively delivers both: the person-name mandate is removed from the schema, and the org-team addressing rule is schema-enforced.
Findings
- Schema conditional is well-formed: the new
allOfentry requiresaddressonly whentriggers.surfacescontains an enabledmentionsurface — correct JSON Schemacontainsusage, and the template's exampleaddress.handlematches the enforced pattern (^petry-projects/[a-z0-9]+(-[a-z0-9]+)*$). - Prior bot review feedback addressed: all 3 review threads (gemini grammar nit, Copilot's two handle/alias pattern-tightening suggestions) are resolved and outdated — the fix-reviews commit tightened both
handleandaliasesto the org-scoped pattern. - Cross-file uniqueness deferred to
validate-personas.pyand live team properties deferred to a CI step — explicitly documented as out of schema scope; reasonable division. - Secret scan (MCP):
run_secret_scanningtool not available in this environment; the gitleaks CI check is green and the diff contains no credential-like content. - No security-sensitive paths touched (no auth, secrets, workflows, or migrations).
CI status
All substantive checks green: Lint, ShellCheck, CodeQL, SonarCloud (quality gate passed), AgentShield, Agent Security Scan, gitleaks secret scan, npm audit, CodeRabbit. Two dev-lead dispatch/ci-relay runs show CANCELLED, but these are the org's own agent-orchestration jobs superseded by later SUCCESS runs of the same checks — not code-quality gates. CodeRabbit and donpetry-bot both approved; overall reviewDecision is APPROVED.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: fc3c44c3cf7886bdc0a56e2cee4b8652886d5ed7
Review mode: triage-approved (single reviewer)
Summary
Standards/docs + declarative JSON-schema change establishing role-based persona naming (§1.6) and org-team @-addressing (§4.1, new address block). 4 files: persona-standards.md, TEMPLATE/README.md, TEMPLATE/persona.yml, persona.schema.json. No runtime code, workflows, or scripts touched. Triage assessment confirmed.
Linked issue analysis
No linked issues (closingIssuesReferences is empty). Issue references in the PR body (#571, #860) are context citations for the discussion-bridge and recursion-hazard notes, not issues this PR closes.
Findings
- No blocking findings.
- Schema conditional (mention surface enabled →
addressrequired) uses correctallOf/if/then+containssemantics; schema file is valid JSON at the reviewed SHA. - All 3 prior review threads are resolved and outdated: the two substantive Copilot findings (unconstrained
address.handle/aliasespatterns) were fixed — both now enforce^petry-projects/[a-z0-9]+(-[a-z0-9]+)*$. - The team-not-user-account constraint is load-bearing and well-justified (global first-come user namespace); the recursion-hazard router requirements (bot-actor + agent-comment marker exclusion) match existing practice.
- Secret scan:
run_secret_scanningMCP tool unavailable in this environment; gitleaks CI check passed and the diff contains no credential-like content.
CI status
All substantive checks green at fc3c44c3cf7886bdc0a56e2cee4b8652886d5ed7: CodeQL, Secret scan (gitleaks), ShellCheck, Lint, SonarCloud (x2), Agent Security Scan, AgentShield, npm audit, Analyze (actions). Ecosystem audits (cargo/pip/pnpm/govulncheck) skipped — not applicable. One cancelled dev-lead / dispatch and dev-lead / ci-relay run each are superseded agent-automation runs; the same checks have later successful runs.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Summary
Two coupled changes to the agentic persona standard, in response to two problems — one a naming inconsistency, one a live hazard.
1. A persona is a role, not a person (new principle §1.6)
id/namenow name the role —qa-lead, notmurat. This is the naming half of the existing "unify over the frameworks" principle: an upstream framework may give its agent a person-name, and that stays upstream. We reference upstream agents only by their technical skill id (framework.skill: bmad-tea) and never mirror the person-name into our namespace.The schema previously mandated the opposite —
namewas documented as "MUST match the locked framework identity" — so every framework-backed persona was required to import upstream's cast list. That clause is gone.dev-leadandpr-reviewwere already role-named; only the framework-backed personas drifted. One rule now, no exceptions.Why roles, concretely:
@petry-projects/qa-leadtells a reader in a PR comment who is being addressed and why.@petry-projects/muratrequires knowing the BMAD cast list.2. Addressing:
@petry-projects/<role>(new §4.1, newaddressblock)A persona that enables the
mentionsurface must declare how it is addressed. The handle MUST be an org team, never a user account.That constraint is load-bearing, not stylistic:
An org team slug lives in the org's namespace and cannot collide. It also buys two things a bare convention cannot: autocomplete (typing
@petry-projects/in any comment box lists every persona — the addressing scheme is the discovery mechanism) and a real link instead of grey text.Schema changes
address.handle—org/team-slug, pattern-enforced to reject a bare user handle. Slug MUST equalid, so routing is a prefix-strip and the role name is written exactly once ("register once").address.aliases[]— the rename migration path; roles outlive their spelling.addressis conditionally REQUIRED when amentionsurface is enabled — mirroring the existingmode: write→gate_labelconditional.id/namedescriptions carry the §1.6 rule; the "MUST match the locked framework identity" clause is removed.Team requirements (
privacy: closed— secret teams are unmentionable;notification_setting: notifications_disabled— the handle routes a webhook, it should not page humans) are live properties. They are deliberately not invalidate-personas.py: that validator is hermetic with a hermetic bats suite, and its hermeticity is worth more than the check. They move to a CI step and the §7 DoD instead.Recursion — documented for the router that will consume this
Agent comments posted via a PAT re-trigger workflows (unlike
GITHUB_TOKEN), and.github-private#860burned 1,481 identical acks in 4.5 hours from a single self-loop. With N mutually addressable personas the cycles are combinatorial, not self-loops —qa-leadanswering a thread that mentionsdev-leadis enough. §4.1 makes the two-axis guard (bot actor + comment marker) thatpr-review-mention.ymlalready carries mandatory for any router.§4.1 also states that mention handling is centralised — one router reads the manifest and dispatches. Personas do not each ship a mention workflow; that is exactly the per-agent drift the manifest exists to prevent.
Verification
persona.schema.jsonis a valid Draft 2020-12 schema.qa-leadhandle FAILS on pattern.TEMPLATE/persona.ymlvalidates against the updated schema.markdownlint-cli2clean on both changed.mdfiles.Notes for reviewers
This is the contract only — it defines how a persona declares its address. No router and no runtime ship here, so nothing dispatches on a mention yet. That separation is deliberate.
Companion: petry-projects/.github-private#1279 renames the worked example
murat→qa-lead, fills itsaddressblock, and adds the two cross-file invariants tovalidate-personas.py. This PR must merge first — that validator fetches the schema from.github@main.🤖 Generated with Claude Code
Summary by CodeRabbit