fix(ci,api-server): fix control-plane build context and migration parameter mismatch#1428
Conversation
…ameter mismatch - Change control-plane Docker build context from ./components/ambient-control-plane to ./components in both CI workflows (Dockerfile copies sibling dirs) - Replace GORM AutoMigrate in agentSchemaExpansionMigration with explicit ALTER TABLE statements to avoid PreferSimpleProtocol parameter binding mismatch - Replace GORM FirstOrCreate in role seed migrations with raw INSERT ON CONFLICT to avoid primary key inference causing parameter count errors 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughTwo GitHub Actions workflows update Docker build contexts for the Changes
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/components-build-deploy.yml:
- Line 62: The workflow’s path filters for the component image list (see the
ambient-control-plane entry with name "ambient-control-plane" and context
"./components") are missing the SDK path; update the workflow triggers under
on.push.paths and on.pull_request.paths to include "components/ambient-sdk/**"
so changes inside components/ambient-sdk will trigger rebuilds of the
ambient-control-plane image.
In `@components/ambient-api-server/plugins/agents/migration.go`:
- Around line 48-60: The migration builds the SQL statements in the stmts slice
and currently creates owner_user_id as nullable, which conflicts with the
model's gorm:"not null"; update the corresponding statement in stmts to add "NOT
NULL" and a sensible DEFAULT (e.g., empty string) to avoid migration failures,
and likewise change the llm_model, llm_temperature and llm_max_tokens column
statements to include NOT NULL with appropriate DEFAULT values (e.g., '' for
llm_model, 0.0 for llm_temperature, 0 for llm_max_tokens) so the SQL schema
matches the non-pointer model fields; adjust the ALTER TABLE statements that
reference owner_user_id, llm_model, llm_temperature and llm_max_tokens in the
stmts slice accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0110428a-3f55-4c60-9d42-599605b5a5f6
📒 Files selected for processing (5)
.github/workflows/components-build-deploy.yml.github/workflows/prod-release-deploy.yamlcomponents/ambient-api-server/plugins/agents/migration.gocomponents/ambient-api-server/plugins/credentials/migration.gocomponents/ambient-api-server/plugins/roles/migration.go
| {"name":"public-api","context":"./components/public-api","image":"quay.io/ambient_code/vteam_public_api","dockerfile":"./components/public-api/Dockerfile"}, | ||
| {"name":"ambient-api-server","context":"./components/ambient-api-server","image":"quay.io/ambient_code/vteam_api_server","dockerfile":"./components/ambient-api-server/Dockerfile"}, | ||
| {"name":"ambient-control-plane","context":"./components/ambient-control-plane","image":"quay.io/ambient_code/vteam_control_plane","dockerfile":"./components/ambient-control-plane/Dockerfile"}, | ||
| {"name":"ambient-control-plane","context":"./components","image":"quay.io/ambient_code/vteam_control_plane","dockerfile":"./components/ambient-control-plane/Dockerfile"}, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Dockerfile inputs for ambient-control-plane ==="
rg -n '^\s*COPY\s+(ambient-api-server/|ambient-sdk/go-sdk/)' components/ambient-control-plane/Dockerfile
echo
echo "=== Current workflow path filters ==="
rg -n "components/ambient-(api-server|control-plane|sdk)/\*\*" .github/workflows/components-build-deploy.ymlRepository: ambient-code/platform
Length of output: 439
Add components/ambient-sdk/** to workflow path filters
The Dockerfile for ambient-control-plane copies from ambient-sdk/go-sdk/ (lines 7–8 of the Dockerfile), but the workflow's on.push.paths and on.pull_request.paths do not include components/ambient-sdk/**. Changes to the SDK will not trigger a rebuild of this image.
Add to both push and pull_request path filters
paths:
- '.github/workflows/components-build-deploy.yml'
- 'components/ambient-api-server/**'
+ - 'components/ambient-sdk/**'
- 'components/ambient-control-plane/**'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/components-build-deploy.yml at line 62, The workflow’s
path filters for the component image list (see the ambient-control-plane entry
with name "ambient-control-plane" and context "./components") are missing the
SDK path; update the workflow triggers under on.push.paths and
on.pull_request.paths to include "components/ambient-sdk/**" so changes inside
components/ambient-sdk will trigger rebuilds of the ambient-control-plane image.
| stmts := []string{ | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS parent_agent_id TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS owner_user_id TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS display_name TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS description TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS repo_url TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS workflow_id TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_model TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_temperature DOUBLE PRECISION`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_max_tokens INTEGER`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS bot_account_name TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS resource_overrides TEXT`, | ||
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS environment_variables TEXT`, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Model field nullability (non-pointer primitives expected non-null DB values):"
rg -n 'OwnerUserId|LlmModel|LlmTemperature|LlmMaxTokens' components/ambient-api-server/plugins/agents/model.go
echo
echo "Migration SQL currently defining these columns:"
rg -n 'owner_user_id|llm_model|llm_temperature|llm_max_tokens' components/ambient-api-server/plugins/agents/migration.goRepository: ambient-code/platform
Length of output: 1082
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for follow-up migrations after migration.go ==="
fd -e go migration | head -20
echo
echo "=== Checking if other fields have gorm NOT NULL constraints we missed ==="
rg -n 'gorm:"' components/ambient-api-server/plugins/agents/model.go | grep -E 'LlmModel|LlmTemperature|LlmMaxTokens|OwnerUserId'
echo
echo "=== Looking for any ALTER TABLE that adds NOT NULL to these columns ==="
rg -n 'ALTER TABLE agents' components/ambient-api-server/plugins/agents/ | grep -E 'owner_user_id|llm_model|llm_temperature|llm_max_tokens.*NOT NULL'Repository: ambient-code/platform
Length of output: 1300
Add NOT NULL constraint to owner_user_id column in migration.
Line 50 creates owner_user_id as nullable, but the model at line 12 declares it as gorm:"not null". This violates the model contract and risks scan failures.
Migration fix
stmts := []string{
`ALTER TABLE agents ADD COLUMN IF NOT EXISTS parent_agent_id TEXT`,
- `ALTER TABLE agents ADD COLUMN IF NOT EXISTS owner_user_id TEXT`,
+ `ALTER TABLE agents ADD COLUMN IF NOT EXISTS owner_user_id TEXT NOT NULL DEFAULT ''`,
`ALTER TABLE agents ADD COLUMN IF NOT EXISTS display_name TEXT`,Consider applying similar constraints to llm_model, llm_temperature, and llm_max_tokens (lines 55–57), since they are non-pointer primitives. Code-level defaults exist, but nullable columns create unnecessary null-check risk.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| stmts := []string{ | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS parent_agent_id TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS owner_user_id TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS display_name TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS description TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS repo_url TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS workflow_id TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_model TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_temperature DOUBLE PRECISION`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_max_tokens INTEGER`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS bot_account_name TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS resource_overrides TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS environment_variables TEXT`, | |
| stmts := []string{ | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS parent_agent_id TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS owner_user_id TEXT NOT NULL DEFAULT ''`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS display_name TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS description TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS repo_url TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS workflow_id TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_model TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_temperature DOUBLE PRECISION`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS llm_max_tokens INTEGER`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS bot_account_name TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS resource_overrides TEXT`, | |
| `ALTER TABLE agents ADD COLUMN IF NOT EXISTS environment_variables TEXT`, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ambient-api-server/plugins/agents/migration.go` around lines 48 -
60, The migration builds the SQL statements in the stmts slice and currently
creates owner_user_id as nullable, which conflicts with the model's gorm:"not
null"; update the corresponding statement in stmts to add "NOT NULL" and a
sensible DEFAULT (e.g., empty string) to avoid migration failures, and likewise
change the llm_model, llm_temperature and llm_max_tokens column statements to
include NOT NULL with appropriate DEFAULT values (e.g., '' for llm_model, 0.0
for llm_temperature, 0 for llm_max_tokens) so the SQL schema matches the
non-pointer model fields; adjust the ALTER TABLE statements that reference
owner_user_id, llm_model, llm_temperature and llm_max_tokens in the stmts slice
accordingly.
Merge Queue Status
This pull request spent 12 seconds in the queue, including 1 second running CI. Required conditions to merge |
Summary
./components/ambient-control-planeto./componentsin both CI workflows. The Dockerfile copies sibling directories (ambient-api-server/,ambient-sdk/go-sdk/), which requires the broader context.tx.AutoMigrate(&Agent{})with explicitALTER TABLE ADD COLUMN IF NOT EXISTSstatements.AutoMigratewithPreferSimpleProtocol: truecausespq: got 2 parameters but the statement requires 1when scanning*int32/*float64column types.FirstOrCreatewith rawINSERT ... ON CONFLICT (name) DO NOTHINGin bothroles/migration.goandcredentials/migration.goto avoid GORM primary key inference causing parameter count errors.Test plan
go vet ./...passesgo build ./...passesAMBIENT_ENV=integration_testing go test -p 1 ./...— 12/12 packages ok)🤖 Generated with Claude Code
Summary by CodeRabbit