feat: v3 multi-model orchestrator (Bedrock generic + Anthropic routing)#30
Conversation
Adds model-aware routing to the orchestrator: based on the model_id input, exactly one executor runs per call. Avoids 2x spend on Anthropic-model selections that would otherwise trigger both paths. - claude-orchestrator.yml: new route job emits provider output (anthropic-api | anthropic-bedrock | bedrock-generic). Existing mention-detection / automatic-mode gating is preserved on the route job so downstream executors are skipped when the gate is closed. Two conditional executor jobs gate on the provider output. - claude-executor.yml: extended to support provider=anthropic-bedrock with OIDC + use_bedrock=true. ANTHROPIC_API_KEY is now optional at the schema level and only required when provider=anthropic-api. Input validation step fails fast on bad combinations. - bedrock-generic-executor.yml (NEW): reusable workflow_call port of the validated bedrock-code-review-poc workflow. Adds sticky_namespace input to prevent comment collisions when multiple review jobs run on one PR. Truncates diff at the last complete line before the byte cap. - .github/scripts/sticky-comment.sh (NEW): find-or-update helper for the generic executor's sticky comment. Numeric-id guard included. - CLAUDE.md: documents the multi-model routing table, sticky-comment marker scheme, and consumer examples for both new paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The default claude_args value contains embedded double quotes (--allowedTools "Bash(git status),Bash(git diff)"). Interpolating it directly into a shell assignment broke the quoting and caused "unexpected token" at the open-paren. Using env: keeps the value verbatim. Caught by the v3 routing test in bedrock-code-review-poc#9 — the routing invariant itself was confirmed: exactly one executor ran per call, the non-matching one was skipped. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Brings over the mermaid diagram from dotCMS/bedrock-code-review-poc that visualizes how the parts fit together — GitOps orchestrator, Anthropic vs dotCMS actions, Bedrock, and the LLM layer. Useful context for the v3 PR since it shows exactly which path is now flexible (dotCMS Action / generic Bedrock executor → any model) vs. constrained (Anthropic Action → Anthropic models only). Linked from CLAUDE.md alongside the existing ARCHITECTURE.md (which covers the repo-internal workflow architecture at a different abstraction level). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Folds the toolchain-wide mermaid diagram into ARCHITECTURE.md as a labeled
section ("Diagram 1") alongside a new repo-internal v3 routing diagram
("Diagram 2"). Single source of truth — AI_TOOLCHAIN_ARCHITECTURE.md
removed.
ARCHITECTURE.md content refreshed for v3:
- Repo-internal diagram converted from ASCII to mermaid and updated to show
the new route job + two executor paths, with new nodes shaded green
- Routing table added with the anchored anthropic. regex match
- Workflow Types section adds bedrock-generic-executor and the v3
provider modes for claude-executor
- Security Isolation reflects ANTHROPIC_API_KEY now being required:false
- Migration path shows v2 unchanged + two new v3 invocation patterns
- Permissions caveat noted (caller permissions must be union of
reachable executors)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Drops the sticky-comment.sh node, the GitHub API node, and the two auxiliary edges into them. The diagram now focuses on the routing decision and external compute targets; sticky-comment behavior is documented in prose under Workflow Types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sfreudenthaler
left a comment
sfreudenthaler
left a comment
There was a problem hiding this comment.
ready for humanz to approve
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f88487dbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
P1 — sticky-comment helper path was unreachable for external consumers. actions/checkout in a reusable workflow checks out the *consumer's* repo, so a relative path like .github/scripts/sticky-comment.sh would resolve against the consumer (where the file doesn't exist) instead of this repo. Inlined the helper into a Set up step that writes it to /tmp via heredoc, eliminating the cross-repo path dependency. The standalone .github/scripts/sticky-comment.sh is removed. P2 — PR-number resolution only handled pull_request / pull_request_target, but the orchestrator's @claude mention detection allows issue_comment, pull_request_review, and pull_request_review_comment events through. Under those events with a non-Anthropic model_id, the generic executor would exit instead of producing a review. Resolution now handles all four PR-context events; issue_comment is recognized as PR-context only when github.event.issue.pull_request.url is non-empty. Non-PR contexts still fail fast with a clear message. ARCHITECTURE.md + CLAUDE.md updated to reflect the inlined helper. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| model_id: ${{ inputs.model_id }} | ||
| bedrock_role_arn: ${{ inputs.bedrock_role_arn }} | ||
| aws_region: ${{ inputs.aws_region }} | ||
| prompt: ${{ inputs.prompt }} |
There was a problem hiding this comment.
Empty prompt from the orchestrator clobbers the bedrock-generic default
This one is silent, which makes it the most dangerous of the bunch.
bedrock-generic-executor.yml (lines 46-49) defines a thoughtful default review prompt:
prompt:
required: false
default: |
Review this PR diff. Flag anything that looks wrong...But the orchestrator invokes the executor with prompt: ${{ inputs.prompt }} (this line), and the orchestrator's own prompt input has no default: (required: false, no default → '').
In workflow_call, passing an empty input still sets it — it does not fall back to the callee's default. So if a consumer routes to a non-Anthropic model (Nova, Llama, etc.) and doesn't set prompt, the model receives only the diff, with no instructions. The executor's default prompt is effectively dead code on this path.
Suggested fix — fall back inside the bedrock-generic step, so it's robust regardless of caller:
# bedrock-generic-executor.yml, "Invoke Bedrock (Converse API)" step
- name: Invoke Bedrock (Converse API)
id: invoke
env:
REVIEW_PROMPT: ${{ inputs.prompt }}
run: |
set -euo pipefail
if [ -z "${REVIEW_PROMPT}" ]; then
REVIEW_PROMPT="Review this PR diff. Flag anything that looks wrong, risky, or worth a second look: bad assumptions, missing edge cases, design problems, security issues. Skip praise. If it is clean, say so in one line."
fi
...Alternatively, resolve it in the orchestrator with prompt: ${{ inputs.prompt != '' && inputs.prompt || '<default>' }} — but that would also affect the Anthropic path, where an empty prompt in interactive mode is intentional. Prefer the fallback in the callee.
Repro: route to a non-Anthropic model_id without passing prompt.
There was a problem hiding this comment.
Great catch — confirmed and fixed in b8d4064. Added a bash-level fallback in the Invoke Bedrock step that sets the default prompt when REVIEW_PROMPT is empty, with a comment explaining why the input-level default is unreachable from the orchestrator. Kept the fix in the callee as you suggested to avoid touching the Anthropic path.
workflow_call always forwards the caller's value even when it is an empty string, so the input-level default on prompt is never reached from the orchestrator. Add an explicit bash fallback in the Invoke Bedrock step so the model always receives instructions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
Adds model-aware routing to `claude-orchestrator.yml`. Based on the new `model_id` input, exactly one executor runs per call — avoiding 2x spend when an Anthropic model is selected and the generic Bedrock executor would otherwise also fire.
Routing table
Anthropic-family match is anchored (`^([a-z]+\.)?anthropic\.`) so a model like `us.not-anthropic.foo` won't be misrouted.
What changed
Backward compat
Test plan
actionlint clean across all four workflow files
shellcheck clean on `sticky-comment.sh`
Consumer-side test: all three routing variants validated end-to-end from `dotCMS/bedrock-code-review-poc` PR #9, run #26724181657:
IAM role trust policy updated: `sub StringLike repo:dotCMS/*` — any dotCMS org repo, no per-repo IAM changes needed for new consumers. Terraform applied to `arn:aws:iam::180208943277:role/GitHubActions-BedrockCodeReview`.
Tagged `v3.0.0-rc1` on `bb5aafe`
Follow-ups (not in this PR)
🤖 Generated with Claude Code