Drop address.aliases from the contract (half 2 of 2) — HOLD until .github-private#1289 merges#769
Conversation
Second half of the ordered cross-repo pair. petry-projects/.github-private#1289 removed every USE of the field first; this removes the field itself. MERGE ORDER: #1289 MUST land before this. The schema is additionalProperties:false, so dropping `aliases` while qa-lead's manifest still carries `aliases: []` fails validate-personas on main FLEET-WIDE. Verified the rejection is real: a manifest with `aliases` now yields "Additional properties are not allowed ('aliases' was unexpected)". Why aliases are gone (see .github#755 finding 1) They existed so a RENAMED role kept routing — qa-lead listing `petry-projects/murat` so old comments still reached it. Two reasons it goes: 1. Structural: index-free routing could never honour it. The router resolves '@petry-projects/murat' to personas/murat/persona.yml — a 404 after the rename. The alias is declared INSIDE the renamed persona's own manifest, which the router never opens, because it does not know to look there. Chicken-and-egg; the pm_persona_aliases attempt in #754 was unreachable for exactly this reason. 2. Measured: there is nothing live left to route. GitHub stops rendering a renamed team's old handle as a mention at all. Probed it directly — created a team, rendered its mention, renamed it, re-rendered the OLD handle: before rename: <a class="team-mention" …>@petry-projects/probe</a> after rename: <p>@petry-projects/probe please review</p> The old handle becomes PLAIN TEXT. So it is not a silently-broken mention that looks live — the reader sees grey text and knows instantly it did not resolve, and the fix is local: mention the new handle. Aliases would only route text GitHub itself no longer treats as a mention. Not worth a derived index, an extra fetch on every 404, and a contract the router cannot honour. Rule 4 becomes a theorem, not a check "Handles and aliases MUST be unique across all personas" is replaced by "Handles are unique BY CONSTRUCTION": the pattern pins the org, slug == id, id == the persona directory name, and directory names are unique — so two personas cannot claim the same handle. #1289 removed the corresponding check_handles_unique for the same reason: it could never fire. Rule 5 ("a rename keeps the old handle as an alias") is replaced by a §4.1 "Renames" section documenting what to change on a rename and stating plainly that old mentions stop resolving VISIBLY — which is the correct signal, not a regression. Changes - standards/personas/persona.schema.json — drop address.aliases (edited as TEXT, not re-serialized: a json round-trip reformats every inline array and rewrote 264 untouched lines. Diff is +2/-10.) - standards/persona-standards.md — §4.1 rules 4-5, the example, and the new Renames section - standards/personas/TEMPLATE/persona.yml — drop the aliases scaffold line Verification - persona.schema.json is a valid Draft 2020-12 schema; diff is +2/-10. - Conditional behaviour verified: qa-lead's post-#1289 shape (handle only) PASSES; a manifest carrying `aliases` FAILS — which is exactly why #1289 goes first. - TEMPLATE/persona.yml validates against the updated schema. - markdownlint-cli2: clean. - Zero `aliases` references remain outside the deliberate explanatory notes. 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: 26 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)
📝 WalkthroughWalkthroughThe persona addressing contract now supports only a primary ChangesPersona addressing contract
Estimated code review effort: 2 (Simple) | ~10 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: #769 |
|
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 removes the aliases field from the persona standards, template, and JSON schema. The documentation is updated to explain that aliases are structurally unsupported by index-free routing and are rendered obsolete by GitHub's behavior on team renames. Handle uniqueness is now guaranteed by construction because the slug matches the unique directory name. I have no feedback to provide on these changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@standards/persona-standards.md`:
- Around line 256-258: Update the “To rename a role” checklist in
persona-standards.md to include address.handle alongside id and the other
renamed role metadata. State that address.handle must be updated to keep the org
team slug aligned with id, preserving the existing rename procedure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 483a5696-752f-4587-8605-4879d8afd5bd
📒 Files selected for processing (3)
standards/persona-standards.mdstandards/personas/TEMPLATE/persona.ymlstandards/personas/persona.schema.json
💤 Files with no reviewable changes (1)
- standards/personas/TEMPLATE/persona.yml
There was a problem hiding this comment.
Pull request overview
Removes the address.aliases field from the persona addressing contract across the schema and standards docs, aligning the public contract with the (already-updated) implementation that no longer supports alias-based routing.
Changes:
- Dropped
address.aliasesfrom the persona YAML template example. - Removed the
aliasesproperty frompersona.schema.jsonand updated theaddressdescription to document the intentional absence. - Updated
persona-standards.mdto removealiasesfrom the addressing example and add a Renames section explaining why aliases are not supported.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| standards/personas/TEMPLATE/persona.yml | Removes aliases from the starter manifest snippet so new personas don’t include an invalid field. |
| standards/personas/persona.schema.json | Deletes the address.aliases property from the schema and documents that the contract intentionally has no aliases. |
| standards/persona-standards.md | Removes aliases from the addressing docs and adds guidance explaining rename behavior without aliases. |
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
|
@coderabbitai resolve |
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #769 |
✅ Action performedComments resolved and changes approved. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 0f42c43aaf6f5b967b17caea86b0ed7b6fcd7cd0
Review mode: triage-approved (single reviewer)
Summary
Removes the deprecated address.aliases field from the persona contract (schema, template, and standards doc) — half 2 of an ordered cross-repo pair. The declared merge-order dependency (.github-private#1289, which removed all uses of the field) is MERGED (2026-07-17T12:55:57Z), so the fleet-wide validate-personas breakage risk described in the PR body is cleared. Diff is docs + JSON Schema only, no executable code. Rationale is well-evidenced (router structurally cannot honour aliases; GitHub renders renamed-team handles as plain text).
Linked issue analysis
No formally linked closing issue. The PR substantively addresses .github#755 finding 1 (drop aliases) and follows through on the #1289 pairing exactly as described. The rename procedure is now documented in §4.1.1 with the correct signal semantics (old handles visibly stop resolving).
Findings
- No blocking findings.
- All 3 bot review threads (CodeRabbit rename-checklist nit, 2× Copilot §4.1.1 cross-reference numbering) are resolved; fix commit 0f42c43 renumbered the Renames section to §4.1.1 as requested. CodeRabbit's CHANGES_REQUESTED review was dismissed after resolution.
- Minor non-blocking nit: the §4.1.1 rename checklist lists id, name, canary.agent, opt_out_label, evals.path and the org team, but does not explicitly name the manifest's address.handle field (it is implied by 'slug must keep matching id'). Cosmetic only.
- Branch is BEHIND main; a standard update-branch before merge is sufficient.
- Secret scan: gitleaks CI check passed. The run_secret_scanning MCP tool was not available in this run (no finding fabricated); diff contains no credential-like content.
CI status
All checks green at head 0f42c43: Analyze (actions)/CodeQL pass, Lint pass, ShellCheck pass, Secret scan (gitleaks) pass, Agent Security Scan pass, agent-shield pass, SonarCloud quality gate pass, npm audit pass, dev-lead / dispatch pass; ecosystem audits skipped (not applicable).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: b593da53a3b7aa5e7cf05fd22398771d591118ae
Review mode: triage-approved (single reviewer)
Summary
Removes the address.aliases field from the persona contract (schema, template, and standards doc) — half 2 of an ordered cross-repo pair. The PR's stated HOLD condition is satisfied: petry-projects/.github-private#1289 (half 1, which removed every use of the field) merged at 2026-07-17T12:55:57Z, so tightening the schema (additionalProperties: false) cannot break validate-personas fleet-wide. The head commit is a merge of main; the substantive change was already reviewed and approved at 0f42c43.
Linked issue analysis
No formally linked (closing) issues. The PR substantively addresses finding 1 of #755: it removes the unreachable aliases routing mechanism, documents in new §4.1.1 why index-free routing could never honour aliases and why renamed team handles degrade to plain text (with measured evidence), and replaces the cross-file uniqueness check rationale with a by-construction argument (org pinned + slug == id + id == directory name).
Findings
No blocking findings.
- Merge-order dependency verified: .github-private#1289 is merged, and a repo-wide grep of .github-private shows no remaining
address.aliasesusage in persona manifests. - Schema change is a pure field removal plus description updates;
additionalProperties: falsenow correctly rejectsaliases, which is the documented intended end state. - Docs, template, and schema are mutually consistent (schema description points at the new §4.1.1, which exists).
- All 3 review threads (CodeRabbit rename-checklist, 2× Copilot §-numbering) are resolved; the §4.1.1 numbering suggestion was adopted.
- Secret scanning MCP tool not available in this session; gitleaks CI check passed and the diff contains no secret-like content.
CI status
All required checks green: Lint, ShellCheck, CodeQL (actions), SonarCloud Quality Gate, CodeRabbit, Agent Security Scan, agent-shield, Secret scan (gitleaks), npm audit. Remaining checks skipped by ecosystem detection (pip/cargo/go/pnpm audits, dependabot-automerge) — expected.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Second half of the ordered cross-repo pair.
petry-projects/.github-private#1289removed every use of the field; this removes the field itself.Why aliases are gone (#755 finding 1)
They existed so a renamed role kept routing —
qa-leadlistingpetry-projects/muratso old comments still reached it. Two reasons it goes, one structural and one measured:1. Index-free routing could never honour it. The router resolves
@petry-projects/murat→personas/murat/persona.yml→ 404 after the rename. The alias is declared inside the renamed persona's own manifest, which the router never opens because it doesn't know to look there. Thepm_persona_aliasesattempt in #754 was unreachable for exactly this reason.2. There is nothing live left to route. GitHub stops rendering a renamed team's old handle as a mention at all. I probed it — created a team, rendered its mention, renamed it, re-rendered the old handle:
<a class="team-mention" …>@petry-projects/probe</a><p>@petry-projects/probe please review</p>The old handle becomes plain text. So it isn't a silently-broken mention that looks live — the reader sees grey text and knows instantly it didn't resolve, and the fix is local: mention the new handle. Aliases would only route text GitHub itself no longer treats as a mention.
Rule 4 becomes a theorem; rule 5 becomes a Renames section
Rule 4 — "Handles and aliases MUST be unique across all personas" → "Handles are unique BY CONSTRUCTION": the pattern pins the org,
slug == id,id ==the persona directory name, and directory names are unique. Two personas cannot claim the same handle. #1289 removed the correspondingcheck_handles_uniquefor the same reason — it could never fire.Rule 5 — "a rename keeps the old handle as an alias" → a §4.1 Renames section documenting exactly what to change on a rename, and stating plainly that old mentions stop resolving visibly, which is the correct signal rather than a regression.
Verification
persona.schema.jsonis a valid Draft 2020-12 schema; diff is +2/-10canary-rings.json)qa-lead's post-#1289 shape (handle only) PASSES; a manifest carryingaliasesFAILSTEMPLATE/persona.ymlvalidates against the updated schemamarkdownlint-cli2cleanaliasesreferences remain outside the deliberate explanatory notes🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Standards