Skip to content

Allow colons in model IDs (versioned IDs and inference profiles) #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Andrey9kin merged 2 commits into from
Jul 4, 2026
Merged

Allow colons in model IDs (versioned IDs and inference profiles) #4

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6aa526b
Allow colons in model IDs (versioned IDs and inference profiles)
Andrey9kin Jul 4, 2026
52b942c
Clarify which inference profile formats the model input supports
Andrey9kin Jul 4, 2026
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
6 changes: 3 additions & 3 deletions .github/workflows/ai-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ on:
type: string
default: "us-east-1"
model:
description: "Bedrock model ID for the orchestrator and review subagents (OpenCode amazon-bedrock provider)"
description: "Bedrock model ID or cross-region inference profile ID (e.g. us.anthropic.claude-opus-4-8) for the orchestrator and review subagents (OpenCode amazon-bedrock provider). ARN-style application inference profiles are not supported."
required: false
type: string
default: "zai.glm-5"
Expand Down Expand Up @@ -268,7 +268,7 @@ jobs:
# The config file comes from the PR branch: sanitize every value
# that reaches prompts or CLI flags to prevent injection.
raw_model = str(cfg.get("model") or model)
if re.fullmatch(r"[A-Za-z0-9._-]+", raw_model):
if re.fullmatch(r"[A-Za-z0-9._:-]+", raw_model):

@fivexl-reviewer fivexl-reviewer Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The regex does not allow forward slashes, which are required for application inference profile ARNs (e.g., arn:aws:bedrock:us-east-1:123456789012:application-inference-profile/my-profile). This could prevent callers from using application inference profiles even though the description mentions 'inference profile ID'. Consider if / should be added to the allowed character set, or clarify in the description which inference profile formats are supported.

@Andrey9kin Andrey9kin Jul 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed in 52b942c by clarifying the description instead of widening the charset: the input now states that cross-region inference profile IDs (e.g. us.anthropic.claude-opus-4-8) are supported and ARN-style application inference profiles are not. Keeping / out of the sanitizer preserves the tight injection surface, and OpenCode's amazon-bedrock provider hasn't been verified to accept ARNs as model refs.

model = raw_model
else:
warn(f"Invalid model {raw_model!r}, using default {model!r}")
Expand Down Expand Up @@ -855,7 +855,7 @@ jobs:
# Defense in depth: the value lands in agent frontmatter via envsubst
# below. Callers are org-controlled, but validate anyway
# (same regex the repo-config sanitizer applies to `model`).
if ! [[ "${SUBAGENT_MODEL}" =~ ^[A-Za-z0-9._-]+$ ]]; then
if ! [[ "${SUBAGENT_MODEL}" =~ ^[A-Za-z0-9._:-]+$ ]]; then
echo "::error::Invalid subagent_model '${SUBAGENT_MODEL}'"
exit 1
fi
Expand Down
Loading