Skip to content

feat: implement issue #1078 — Register agents.solution-architect in canary-rings.json (cross-repo prerequisite for .github-private#1631) - #1079

Closed
don-petry wants to merge 3 commits into
mainfrom
dev-lead/issue-1078-20260907-1433
Closed

don-petry wants to merge 3 commits into
mainfrom
dev-lead/issue-1078-20260907-1433

Conversation

@don-petry

@don-petry don-petry commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

User description

Closes #1078

Implemented by dev-lead agent. Please review.


CodeAnt-AI Description

Register solution-architect for staged canary rollout

What Changed

  • Adds solution-architect to the canary rollout registry using the shared persona runtime
  • Enables staged promotion through next, ring0, ring1, and stable environments with standard soak and sampling gates
  • Adds solution-architect to the rollout workflow’s selectable agents
  • Adds checks confirming ring membership, promotion settings, and shared-runtime behavior

Impact

✅ Solution-architect can be promoted through canary rings
✅ Standard rollout safeguards for persona changes
✅ Selectable solution-architect canary operations

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…anary-rings.json (cross-repo prerequisite for .github-private#1631)
@don-petry
don-petry requested a review from a team as a code owner September 7, 2026 14:48
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@codeant-ai

codeant-ai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR dd71819 Sep 07, 2026 · 14:48 14:51

@codeant-ai

codeant-ai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 3e7ddc67-b134-4c9d-8e9e-e0016e4ced92

📥 Commits

Reviewing files that changed from the base of the PR and between edbac25 and 88e8e93.

⛔ Files ignored due to path filters (2)
  • node_modules/.package-lock.json is excluded by !**/node_modules/**
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • .github/workflows/canary-rollout.yml
  • standards/canary-rings.json
  • tests/canary_rollout.bats

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Sep 7, 2026
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) September 7, 2026 14:49

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request onboards the solution-architect persona to the shared persona runtime in standards/canary-rings.json and adds a corresponding BATS test suite in tests/canary_rollout.bats to verify its configuration. The feedback highlights two key improvements: first, updating the agent input choices in .github/workflows/canary-rollout.yml to prevent a BATS test failure, and second, utilizing dynamic tokens "$host" and "$org_infra" instead of hardcoded repository names to maintain consistency and robustness.

Comment thread standards/canary-rings.json
Comment thread standards/canary-rings.json
@don-petry
don-petry disabled auto-merge September 7, 2026 14:51
Comment thread standards/canary-rings.json Outdated
Comment on lines +1085 to +1086
"reusable": ".github/workflows/persona-runner-reusable.yml",
"run_workflow": "Persona Runtime",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Persona Runtime is shared by multiple personas, but the gate filters only by repository and workflow name, so unrelated persona runs contaminate solution-architect sampling and failure counts. [api mismatch]

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** standards/canary-rings.json
**Line:** 1085:1086
**Comment:**
	*Api Mismatch: `Persona Runtime` is shared by multiple personas, but the gate filters only by repository and workflow name, so unrelated persona runs contaminate solution-architect sampling and failure counts.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged and documented explicitly. The shared run_workflow: "Persona Runtime" sampling all personas' runs against the shared runner blob is intentional per CONSEQUENCE 2 — the gate tracks runner health, not per-persona prompt health. The _note now contains an explicit NOTE section stating this is a deliberate design choice (not an oversight), so it is clear to future reviewers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Customized review instruction saved!

Instruction:

Do not flag shared workflow sampling across personas when the canary gate is intentionally tracking shared runner health rather than per-persona prompt health.

Applied to:

  • standards/canary-rings.json

💡 To manage or update this instruction, visit: CodeAnt AI Settings

Comment thread tests/canary_rollout.bats
Comment on lines +872 to +873
# run_workflow = the persona-runner.yml caller display name the gate samples on the ring tiers.
run jq -e '.agents["solution-architect"].run_workflow == "Persona Runtime"' "$RINGS"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The test approves a shared workflow name without checking persona filtering, so unrelated Persona Runtime runs can contaminate solution-architect health decisions while all assertions pass. [api mismatch]

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/canary_rollout.bats
**Line:** 872:873
**Comment:**
	*Api Mismatch: The test approves a shared workflow name without checking persona filtering, so unrelated Persona Runtime runs can contaminate solution-architect health decisions while all assertions pass.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test correctly asserts run_workflow == "Persona Runtime" because that is the actual value — the design is intentional (gate tracks shared runner health across all personas). The _note has been updated in standards/canary-rings.json with an explicit NOTE section documenting that the shared workflow name is by design, so this is no longer an undocumented gap.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Customized review instruction saved!

Instruction:

Do not require persona-specific filtering when canary tests intentionally validate the shared Persona Runtime health across all personas.

Applied to:

  • tests/canary_rollout.bats

💡 To manage or update this instruction, visit: CodeAnt AI Settings

Comment thread tests/canary_rollout.bats
Comment on lines +897 to +899
# AC #2: inline _note documents (a) no dedicated reusable / shared runtime, and (b) autocut does
# not fire on prompt-only changes — naming .github-private#1592 and the persona_reach_check.sh control.
run jq -e '.agents["solution-architect"]._note | test("1592") and test("persona_reach_check.sh") and test("shared"; "i")' "$RINGS"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The test only checks that the note claims prompt changes do not autocut; it never exercises autocut, so it passes while prompts/ changes still create candidates. [possible bug]

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/canary_rollout.bats
**Line:** 897:899
**Comment:**
	*Possible Bug: The test only checks that the note claims prompt changes do not autocut; it never exercises autocut, so it passes while `prompts/` changes still create candidates.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in tests/canary_rollout.bats: added a behavioral assertion (after line 902) that verifies .agents["solution-architect"].agent_ref_paths contains "scripts/" and "personas/" but does NOT contain "prompts/". This replaces the prior string-content-only check with an actual structural test of the override that suppresses autocut on prompt-only changes.

@donpetry-bot

Copy link
Copy Markdown
Contributor

CI checks on this PR are still running. Once they complete, re-mention @donpetry-bot to trigger a fresh review.

Posted by the donpetry-bot PR-review cascade.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) September 7, 2026 15:01
@sonarqubecloud

sonarqubecloud Bot commented Sep 7, 2026

Copy link
Copy Markdown

@don-petry
don-petry disabled auto-merge September 7, 2026 15:02
@donpetry-bot

Copy link
Copy Markdown
Contributor

CI checks on this PR are still running. Once they complete, re-mention @donpetry-bot to trigger a fresh review.

Posted by the donpetry-bot PR-review cascade.

@don-petry

Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@donpetry-bot

Copy link
Copy Markdown
Contributor

CI checks on this PR are still running. Once they complete, re-mention @donpetry-bot to trigger a fresh review.

Posted by the donpetry-bot PR-review cascade.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) September 7, 2026 15:12
@don-petry

Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
don-petry disabled auto-merge September 7, 2026 15:22
@don-petry

Copy link
Copy Markdown
Contributor Author

Closing unmerged — superseded by ADR-0006, approved in combsmith/.github-private#1688 — Decision: how should shared-runtime personas be represented in the canary ring model?

No fault in this implementation. It was faithful to issue #1078, including a _note I instructed it to write that was factually wrong (autocut does fire on prompt-only changes — agent_ref_paths covers prompts/). It also caught something the issue missed: the canary-rollout.yml dispatch enum needed the agent too. The problem was the premise, not the work.

What changed the answer: persona-runner.yml consumes the runtime by local relative path with no agent_ref input, so no caller pins a per-persona channel tag — the registered rings would advance tags nothing reads. And a persona is a caller of the shared runtime, not a reusable, so registering each one inverts ADR-0001's two-tier boundary.

Replaced by:

The branch is retained if any of the entry is ever wanted for a persona that gains its own reusable — for which ADR-0002 still applies in full.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — rebase (applied)

Rebase completed and pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Register agents.solution-architect in canary-rings.json (cross-repo prerequisite for .github-private#1631)

2 participants