Skip to content

feat(agents): test bedrock connection#2687

Open
jordan-umusu wants to merge 3 commits into
mainfrom
feat/agents-test-bedrock
Open

feat(agents): test bedrock connection#2687
jordan-umusu wants to merge 3 commits into
mainfrom
feat/agents-test-bedrock

Conversation

@jordan-umusu
Copy link
Copy Markdown
Collaborator

@jordan-umusu jordan-umusu commented May 13, 2026

Summary by cubic

Add a Bedrock connection test so users can verify a model or inference profile before saving. Defaults to the Converse API and auto-injects org-scoped AWS External IDs for Bedrock role assumptions.

  • New Features
    • Backend: POST /organization/agent-catalog/bedrock/test verifies unsaved targets with control-plane checks (profile ACTIVE or model available/authorized) and a Converse runtime ping. Supports AWS role (server builds External ID from org ID when missing), static keys, or AWS_BEARER_TOKEN_BEDROCK. Requires AWS_REGION, returns non-sensitive details, and handles missing org credentials gracefully.
    • Frontend: Added “Test connection” for Bedrock in Org Settings. Enabled only when exactly one of inference profile ID or model ID is set. Shows spinner and success/failure toasts.
    • Credentials & defaults: use_converse now defaults to true across create/update/test and the form. Server injects org-scoped External ID for Bedrock role credentials across provider, catalog, and runtime; workspace-scoped External ID is used only for workspace runtime. Error messages reference “AWS External ID.”

Written for commit 524ed65. Summary will update on new commits. Review in cubic

image

@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 13, 2026 21:26 — with GitHub Actions Inactive
@jordan-umusu jordan-umusu added priority:medium Medium priority ticket agents LLM agents labels May 13, 2026
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 13, 2026 21:27 — with GitHub Actions Inactive
@zeropath-ai
Copy link
Copy Markdown

zeropath-ai Bot commented May 13, 2026

No security or compliance issues detected. Reviewed everything up to 524ed65.

Security Overview
Detected Code Changes
Change Type Relevant files
Configuration changes ► frontend/src/client/schemas.gen.ts
    Update default for use_converse to true
► frontend/src/client/types.gen.ts
    Add BedrockCatalogTest and BedrockCatalogTestResponse types
► frontend/src/components/organization/org-settings-agent.tsx
    Update default for use_converse to true
Enhancement ► frontend/src/client/schemas.gen.ts
    Add BedrockCatalogTest schema
► frontend/src/client/services.gen.ts
    Add testBedrockCatalogTarget service function
► frontend/src/client/types.gen.ts
    Add BedrockCatalogTest and BedrockCatalogTestResponse types
► frontend/src/components/organization/org-settings-agent.tsx
    Implement "Test connection" button for Bedrock models
► tests/unit/test_agent_catalog_service.py
    Add tests for Bedrock create and test defaults
► tests/unit/test_bedrock_catalog_validation.py
    Add tests for Bedrock catalog target verification
► tracecat/agent/catalog/bedrock_validation.py
    Implement Bedrock catalog target verification logic
► tracecat/agent/catalog/router.py
    Add Bedrock catalog target test endpoint
► tracecat/agent/catalog/schemas.py
    Add BedrockCatalogTest and BedrockCatalogTestResponse schemas
► tracecat/agent/service.py
    Add Bedrock external ID logic for organization and workspace
► tracecat/integrations/aws_assume_role.py
    Add build_organization_external_id function
Refactor ► frontend/src/client/schemas.gen.ts
    Update use_converse default to true
► frontend/src/client/types.gen.ts
    Update use_converse default to true
► frontend/src/components/organization/org-settings-agent.tsx
    Update use_converse default to true
► tests/unit/test_agent_gateway.py
    Update assertion message for Bedrock external ID
► tracecat/agent/gateway.py
    Update Bedrock external ID message
► tracecat/agent/service.py
    Refactor Bedrock external ID logic
Other ► frontend/src/client/schemas.gen.ts
    Add BedrockCatalogTest schema
► frontend/src/client/services.gen.ts
    Add testBedrockCatalogTarget service function
► frontend/src/client/types.gen.ts
    Add BedrockCatalogTest and BedrockCatalogTestResponse types
► tracecat/agent/catalog/router.py
    Add Bedrock catalog target test endpoint
► tracecat/agent/catalog/schemas.py
    Add BedrockCatalogTest and BedrockCatalogTestResponse schemas

@jordan-umusu jordan-umusu force-pushed the feat/agents-test-bedrock branch from f91f191 to 72eaed5 Compare May 13, 2026 21:53
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 13, 2026 21:53 — with GitHub Actions Inactive
@jordan-umusu jordan-umusu marked this pull request as ready for review May 13, 2026 21:53
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 13, 2026 21:53 — with GitHub Actions Inactive
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72eaed5667

ℹ️ 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".

Comment thread frontend/src/components/organization/org-settings-agent.tsx Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 9 files

Confidence score: 3/5

  • There is moderate merge risk because tracecat/agent/catalog/router.py introduces a concrete permission mismatch: requiring agent:update for pre-save Bedrock testing can block users who only have create rights from validating before creating entries.
  • tracecat/agent/catalog/bedrock_validation.py should normalize HTTPX/network failures into BedrockVerificationError; otherwise timeout/connection errors may surface as untyped exceptions and create inconsistent error handling.
  • frontend/src/components/organization/org-settings-agent.tsx uses a global last-viewed workspace cookie in the test payload, which can send the wrong workspace context and trigger false 404 verification failures across org/workspace boundaries.
  • Pay close attention to tracecat/agent/catalog/router.py, tracecat/agent/catalog/bedrock_validation.py, frontend/src/components/organization/org-settings-agent.tsx - permission gating, exception wrapping, and workspace scoping can all directly affect Bedrock verification behavior.

You’re at about 97% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tracecat/agent/catalog/bedrock_validation.py">

<violation number="1" location="tracecat/agent/catalog/bedrock_validation.py:211">
P2: Wrap HTTPX request failures so network/timeout errors are returned as BedrockVerificationError instead of leaking as untyped exceptions.</violation>
</file>

<file name="frontend/src/components/organization/org-settings-agent.tsx">

<violation number="1" location="frontend/src/components/organization/org-settings-agent.tsx:356">
P2: Using a global last-viewed workspace cookie directly in the Bedrock test payload can cause false 404 verification failures when the cookie points to a stale or other-org workspace.</violation>
</file>

<file name="tracecat/agent/catalog/router.py">

<violation number="1" location="tracecat/agent/catalog/router.py:105">
P2: The new Bedrock pre-save test route requires `agent:update`, which can block users who have create permission but not update permission from validating before creating a catalog entry.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread tracecat/agent/catalog/bedrock_validation.py Outdated
Comment thread frontend/src/components/organization/org-settings-agent.tsx Outdated
Comment thread tracecat/agent/catalog/router.py Outdated
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 14, 2026 14:57 — with GitHub Actions Inactive
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 14, 2026 14:57 — with GitHub Actions Inactive
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6510b6f5d

ℹ️ 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".

Comment thread frontend/src/components/organization/org-settings-agent.tsx Outdated
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 14, 2026 17:02 — with GitHub Actions Inactive
@jordan-umusu jordan-umusu temporarily deployed to internal-registry-ci May 14, 2026 17:03 — with GitHub Actions Inactive
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d837164118

ℹ️ 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".

Comment thread tracecat/agent/catalog/bedrock_validation.py
Comment thread tracecat/agent/catalog/router.py
@jordan-umusu jordan-umusu force-pushed the feat/agents-test-bedrock branch from d837164 to eb6da60 Compare May 14, 2026 18:53
@jordan-umusu jordan-umusu added priority:low Low priority ticket and removed priority:medium Medium priority ticket labels May 14, 2026
@jordan-umusu jordan-umusu force-pushed the feat/agents-test-bedrock branch from eb6da60 to 832ac68 Compare May 21, 2026 20:00
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 832ac68168

ℹ️ 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".

Comment thread tracecat/agent/catalog/router.py Outdated
@jordan-umusu jordan-umusu force-pushed the feat/agents-test-bedrock branch from 832ac68 to 524ed65 Compare May 21, 2026 20:18
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 524ed65fc4

ℹ️ 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".

Comment thread tracecat/agent/catalog/router.py
@jordan-umusu jordan-umusu requested a review from daryllimyt May 21, 2026 20:37
inference_profile_id: z.string().trim().optional(),
model_id: z.string().trim().optional(),
use_converse: z.boolean().default(false),
use_converse: z.boolean().default(true),
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.

is making this default intentional?

"""Resolve configured Bedrock credentials into an explicit auth shape."""
if credentials.get("AWS_ROLE_ARN"):
try:
return await asyncio.to_thread(_assume_bedrock_role, credentials)
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.

nit: could use aioboto3 instead

Comment on lines +136 to +145
bedrock = session.client("bedrock", region_name=region)
runtime = session.client("bedrock-runtime", region_name=region)
if is_inference_profile:
details = _check_profile_response(
bedrock.get_inference_profile(inferenceProfileIdentifier=target_id)
)
else:
details = _check_model_availability_response(
bedrock.get_foundation_model_availability(modelId=target_id)
)
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.

these should be typed

Comment on lines +203 to +243
async def _verify_with_api_key(
*,
api_key: str,
region: str,
target_id: str,
is_inference_profile: bool,
use_converse: bool,
) -> dict[str, Any]:
encoded_target = quote(target_id, safe="")
if is_inference_profile:
details = _check_profile_response(
await _request_bedrock_api_key(
method="GET",
url=f"https://bedrock.{region}.amazonaws.com/inference-profiles/{encoded_target}",
api_key=api_key,
)
)
else:
details = _check_model_availability_response(
await _request_bedrock_api_key(
method="GET",
url=f"https://bedrock.{region}.amazonaws.com/foundation-model-availability/{encoded_target}",
api_key=api_key,
)
)

if not use_converse:
raise BedrockVerificationError(
"Runtime verification requires the Bedrock Converse API. Enable Use Converse API and try again."
)

await _request_bedrock_api_key(
method="POST",
url=f"https://bedrock-runtime.{region}.amazonaws.com/model/{encoded_target}/converse",
api_key=api_key,
json={
"messages": _PING_MESSAGES,
"inferenceConfig": _PING_INFERENCE_CONFIG,
},
)
return details | {"runtime_test": "converse"}
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.

we recently updated boto3, are there any sdk methods that can replace these?


try:
return await asyncio.to_thread(
_verify_with_boto3,
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.

nit: aioboto3

params: BedrockCatalogTest = Body(...),
) -> BedrockCatalogTestResponse:
"""Verify an unsaved Bedrock catalog target."""
assert role.organization_id is not None # OrgUserRole guarantees this
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.

throw http exception instead

Comment thread tracecat/agent/service.py
credentials = await self.get_provider_credentials(provider)
if credentials is None:
return None
credentials = self._with_bedrock_organization_external_id(
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.

why can't this live inside _augment_runtime_provider_credentials? i don't think the service needs to leak bedrock concepts outside of the provider-specific metch-case

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

Labels

agents LLM agents priority:low Low priority ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants