perf: use sqlc directly to retrieve deployment release targets#867
perf: use sqlc directly to retrieve deployment release targets#867adityachoudhari26 merged 2 commits intomainfrom
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a new API GET /v1/deployments/{deploymentId}/release-targets and associated ReleaseTargetItem schema, implements DB queries and server handlers to assemble release-targets with versions and verification metrics, updates verification schemas/types, and updates SDK and frontend to consume the new endpoint. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant SDK as "Workspace SDK"
participant API as "workspace-engine HTTP"
participant DB as "Postgres"
Frontend->>SDK: request listReleaseTargets(deploymentId)
SDK->>API: GET /v1/deployments/{id}/release-targets
API->>DB: ListReleaseTargetsByDeploymentID(deploymentId)
DB-->>API: release target rows
API->>DB: ListCurrentVersionsByDeploymentID(deploymentId)
DB-->>API: current version rows
API->>DB: ListVerificationMetricsByJobIDs(jobIds)
DB-->>API: metrics + measurements
API-->>SDK: 200 { items: [...] } (assembled ReleaseTargetItem objects)
SDK-->>Frontend: return items
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/workspace-engine/pkg/db/job_verification_metric.sql.go (1)
207-225:⚠️ Potential issue | 🟠 MajorPersist the new policy-rule metric FK on insert.
Lines 208-211 still write only the original columns, and
InsertJobVerificationMetricParamshas no field forpolicy_rule_verification_metric_id. That means every row created through this path stores the new relation asNULL, so any downstream join on that FK will miss newly created metrics. Update the source SQL and params, then regenerate this file.Also applies to: 227-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/db/job_verification_metric.sql.go` around lines 207 - 225, The INSERT SQL constant insertJobVerificationMetric is not including the policy_rule_verification_metric_id column and the InsertJobVerificationMetricParams struct lacks a corresponding field, so new rows persist that FK as NULL; update the SQL to include policy_rule_verification_metric_id in the column list, add a matching parameter placeholder in VALUES and include it in the RETURNING list, add a PolicyRuleVerificationMetricID field of type uuid.UUID (or the appropriate type) to InsertJobVerificationMetricParams, then regenerate the generated file; apply the same change to the other insert block mentioned (lines 227-245) so both SQL constants and their Params structs include and return policy_rule_verification_metric_id.packages/trpc/src/routes/deployments.ts (2)
133-155:⚠️ Potential issue | 🟠 Major
query/limit/offsetare now dead inputs.Lines 135-138 still expose filtering and pagination on the tRPC contract, but Lines 141-155 ignore all three and always return the full engine list. That silently changes caller behavior and removes the only guard against very large responses for deployments with many targets. Either plumb these through to the new endpoint or keep applying them in this route until callers are migrated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/routes/deployments.ts` around lines 133 - 155, The route's input schema declares query/limit/offset but the handler ignores them; update the GET call to pass these as query params (e.g., include { params: { path: { deploymentId: input.deploymentId }, query: { q: input.query, limit: input.limit, offset: input.offset } } } or whatever the backend expects) so the backend applies filtering/pagination, or if the new endpoint doesn't support them yet, apply filtering and slicing on result.data.items using input.query, input.limit and input.offset before returning; touch the GET to "/v1/deployments/{deploymentId}/release-targets", the input fields deploymentId/query/limit/offset, and the returned result.data.items.
133-151:⚠️ Potential issue | 🟠 MajorMalformed deployment IDs now turn into 500s.
This input was relaxed to
z.string(), and any upstream failure is rethrown asINTERNAL_SERVER_ERROR. A baddeploymentIdshould be rejected as client input, not sent to the workspace-engine and surfaced as a 500. KeepdeploymentIdvalidated as a UUID here and preserve upstream 4xx semantics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/routes/deployments.ts` around lines 133 - 151, The route currently relaxes deploymentId to z.string() and always converts upstream errors into INTERNAL_SERVER_ERROR; change the input schema for this procedure to validate deploymentId as a UUID (use z.string().uuid()) so malformed IDs are rejected as client errors, and adjust the error handling after getClientFor().GET("/v1/deployments/{deploymentId}/release-targets") to preserve upstream 4xx semantics by mapping result.status in the 400–499 range to a TRPCError with a client error code (e.g., BAD_REQUEST or a code derived from the status) and only using INTERNAL_SERVER_ERROR for non-4xx failures.
🧹 Nitpick comments (1)
apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go (1)
12-25: Reduce the exported surface of this helper layer.
ReleaseTargetResult,Getter, andPostgresGetterare only consumed insideapps/workspace-engine/svc/http/server/openapi/release_targets, so exporting them creates package API that now needs docs and compatibility support for no clear benefit. Making them unexported would better match the current usage.As per coding guidelines, "Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go` around lines 12 - 25, Rename the exported symbols to unexported ones to reduce package API surface: change ReleaseTargetResult -> releaseTargetResult, Getter -> getter, and PostgresGetter -> postgresGetter (and likewise rename ListReleaseTargets to listReleaseTargets if it's only used internally); then update all references/usages and any method receivers to the new unexported identifiers so the package compiles and retains identical behavior. Ensure JSON tags remain unchanged and update any tests or callers inside the package to use the new names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/workspace-engine/oapi/openapi.json`:
- Around line 1727-1729: The latestJob property currently references the plain
Job schema which lacks verifications; update the OpenAPI so latestJob uses a
richer schema that includes verification results (e.g., create a new
components/schemas/JobWithVerifications or a components/schemas/JobDetailed that
allOf: [`#/components/schemas/Job`] plus a verifications array/object property),
then change the latestJob $ref to point to that new schema (replace "$ref":
"#/components/schemas/Job" with the new schema ref) so generated SDK clients can
access verification data.
In `@apps/workspace-engine/pkg/db/queries/release_targets.sql`:
- Around line 67-78: The query's LEFT JOINs (aliases rtdr, rel, dv, rj, j)
produce nullable columns but the generated scanner in
apps/workspace-engine/pkg/db/release_targets.sql.go expects non-nullable UUIDs
(DesiredReleaseID, DesiredReleaseVersionID, DesiredVersionID, LatestJobID,
MeasurementID); update the SQL to make those selected columns scan-safe by
either: a) changing the selected columns to nullable driver types (use
pgtype.UUID/pgtype.Int8 in your SQLC types mapping) or b) wrapping each nullable
column with a COALESCE sentinel (e.g. COALESCE(rel.id,
'00000000-0000-0000-0000-000000000000') for UUIDs or COALESCE(measurement.id, 0)
for ints) so the query always returns non-null values; apply the chosen approach
consistently for rtdr.desired_release_id, rel.version_id, dv.id, rj.job_id/j.id,
and measurement.id to prevent scan errors.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go`:
- Around line 163-166: The current code swallows DB/query failures from
ListVerificationMetricsByJobIDs by returning verificationsMap (empty) on error;
change this to propagate the error instead of treating it as an empty
result—when ListVerificationMetricsByJobIDs returns a non-nil err, return that
error (or a wrapped error) to the caller so the endpoint can surface a failure
or mark the response partial; update the surrounding function's signature/return
path accordingly so callers of the getter can handle/return the error rather
than silently dropping verification data.
- Around line 112-132: The response for LatestJob (built into item.LatestJob
using row.LatestJob* fields and verificationsMap) does not match the generated
API type Job nor JobVerification; either construct and return a payload that
exactly matches the generated Job/JobVerification types (populate missing fields
like metadata, releaseId, workflowJobId on the Job and ensure each verification
includes id, message, createdAt and any other required properties) or introduce
a new dedicated schema for the flattened job+verifications payload and update
the OpenAPI types and marshalling accordingly; locate the assembly of LatestJob
(the jobH map, item.LatestJob assignment, and verificationsMap usage) and either
add the missing Job fields from available row.* fields or change the response
type to the new schema and populate verifications entries to conform to
JobVerification.
- Around line 113-125: The JSONB fields in the job response (jobH map built in
getters.go) are currently raw []byte (jobAgentConfig, dispatchContext, provider,
data) which will be JSON-encoded as base64; unmarshal those []byte blobs into
concrete JSON values (e.g., map[string]any or a typed struct) before inserting
into jobH so the HTTP response emits proper JSON objects; locate the creation of
jobH (fields: LatestJobAgentConfig, LatestJobDispatchContext, LatestJobProvider,
LatestJobData / keys "jobAgentConfig","dispatchContext","provider","data") and
replace the direct []byte assignment with json.Unmarshal into a value and use
that value (or nil on empty) or update sqlc.yaml overrides to map those columns
to map[string]any so the generated types are already decoded.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/server.go`:
- Around line 18-21: The handler that calls rt.getter.ListReleaseTargets
currently returns 500 and echoes the raw error; change it to detect validation
errors from PostgresGetter.ListReleaseTargets (use errors.Is / errors.As to
match the getter's validation/UUID error or the specific validation error type
returned) and respond with c.JSON(http.StatusBadRequest, gin.H{"error":"invalid
deployment id"}) for those cases; for all other errors, log the detailed error
server-side and return c.JSON(http.StatusInternalServerError,
gin.H{"error":"internal server error"}) instead of echoing internal messages.
Ensure you update the code around the call to rt.getter.ListReleaseTargets and
use c.Request.Context(), c.JSON and proper logging when handling the
non-validation branch.
---
Outside diff comments:
In `@apps/workspace-engine/pkg/db/job_verification_metric.sql.go`:
- Around line 207-225: The INSERT SQL constant insertJobVerificationMetric is
not including the policy_rule_verification_metric_id column and the
InsertJobVerificationMetricParams struct lacks a corresponding field, so new
rows persist that FK as NULL; update the SQL to include
policy_rule_verification_metric_id in the column list, add a matching parameter
placeholder in VALUES and include it in the RETURNING list, add a
PolicyRuleVerificationMetricID field of type uuid.UUID (or the appropriate type)
to InsertJobVerificationMetricParams, then regenerate the generated file; apply
the same change to the other insert block mentioned (lines 227-245) so both SQL
constants and their Params structs include and return
policy_rule_verification_metric_id.
In `@packages/trpc/src/routes/deployments.ts`:
- Around line 133-155: The route's input schema declares query/limit/offset but
the handler ignores them; update the GET call to pass these as query params
(e.g., include { params: { path: { deploymentId: input.deploymentId }, query: {
q: input.query, limit: input.limit, offset: input.offset } } } or whatever the
backend expects) so the backend applies filtering/pagination, or if the new
endpoint doesn't support them yet, apply filtering and slicing on
result.data.items using input.query, input.limit and input.offset before
returning; touch the GET to "/v1/deployments/{deploymentId}/release-targets",
the input fields deploymentId/query/limit/offset, and the returned
result.data.items.
- Around line 133-151: The route currently relaxes deploymentId to z.string()
and always converts upstream errors into INTERNAL_SERVER_ERROR; change the input
schema for this procedure to validate deploymentId as a UUID (use
z.string().uuid()) so malformed IDs are rejected as client errors, and adjust
the error handling after
getClientFor().GET("/v1/deployments/{deploymentId}/release-targets") to preserve
upstream 4xx semantics by mapping result.status in the 400–499 range to a
TRPCError with a client error code (e.g., BAD_REQUEST or a code derived from the
status) and only using INTERNAL_SERVER_ERROR for non-4xx failures.
---
Nitpick comments:
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go`:
- Around line 12-25: Rename the exported symbols to unexported ones to reduce
package API surface: change ReleaseTargetResult -> releaseTargetResult, Getter
-> getter, and PostgresGetter -> postgresGetter (and likewise rename
ListReleaseTargets to listReleaseTargets if it's only used internally); then
update all references/usages and any method receivers to the new unexported
identifiers so the package compiles and retains identical behavior. Ensure JSON
tags remain unchanged and update any tests or callers inside the package to use
the new names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d50fb179-73d9-4741-ae47-0c8bdba6a3b9
📒 Files selected for processing (16)
apps/workspace-engine/oapi/openapi.jsonapps/workspace-engine/oapi/spec/main.jsonnetapps/workspace-engine/oapi/spec/paths/release_targets.jsonnetapps/workspace-engine/oapi/spec/schemas/release_targets.jsonnetapps/workspace-engine/pkg/db/job_verification_metric.sql.goapps/workspace-engine/pkg/db/models.goapps/workspace-engine/pkg/db/queries/release_targets.sqlapps/workspace-engine/pkg/db/queries/schema.sqlapps/workspace-engine/pkg/db/release_targets.sql.goapps/workspace-engine/pkg/db/sqlc.yamlapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/svc/http/server/openapi/release_targets/getters.goapps/workspace-engine/svc/http/server/openapi/release_targets/server.goapps/workspace-engine/svc/http/server/openapi/server.gopackages/trpc/src/routes/deployments.tspackages/workspace-engine-sdk/src/schema.ts
| LEFT JOIN release_target_desired_release rtdr | ||
| ON rtdr.deployment_id = d.id | ||
| AND rtdr.resource_id = r.id | ||
| AND rtdr.environment_id = e.id | ||
| LEFT JOIN release rel | ||
| ON rtdr.desired_release_id = rel.id | ||
| LEFT JOIN deployment_version dv | ||
| ON rel.version_id = dv.id | ||
| LEFT JOIN release_job rj | ||
| ON rel.id = rj.release_id | ||
| LEFT JOIN job j | ||
| ON rj.job_id = j.id |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "release_targets.sql*" | head -20Repository: ctrlplanedev/ctrlplane
Length of output: 178
🏁 Script executed:
wc -l ./apps/workspace-engine/pkg/db/queries/release_targets.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 128
🏁 Script executed:
sed -n '67,78p' ./apps/workspace-engine/pkg/db/queries/release_targets.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 416
🏁 Script executed:
sed -n '129,130p' ./apps/workspace-engine/pkg/db/queries/release_targets.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 163
🏁 Script executed:
cat -n ./apps/workspace-engine/pkg/db/queries/release_targets.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 5761
🏁 Script executed:
wc -l ./apps/workspace-engine/pkg/db/release_targets.sql.goRepository: ctrlplanedev/ctrlplane
Length of output: 123
🏁 Script executed:
cat -n ./apps/workspace-engine/pkg/db/release_targets.sql.goRepository: ctrlplanedev/ctrlplane
Length of output: 14957
🏁 Script executed:
python3 << 'EOF'
# Check for NULL-capable fields from LEFT JOINs in the Go struct
go_fields = {
'DesiredReleaseID': ('uuid.UUID', 'rel.id'), # Line 200, from LEFT JOIN release
'DesiredReleaseVersionID': ('uuid.UUID', 'rel.version_id'), # Line 201, from LEFT JOIN release
'DesiredVersionID': ('uuid.UUID', 'dv.id'), # Line 203, from LEFT JOIN deployment_version
'LatestJobID': ('uuid.UUID', 'j.id'), # Line 213, from LEFT JOIN job
'MeasurementID': ('uuid.UUID', 'm.id'), # Line 338, from LEFT JOIN job_verification_metric_measurement
}
print("Fields from LEFT JOINs with non-nullable UUID types:")
for field, (go_type, sql_col) in go_fields.items():
print(f" {field}: {go_type} (from {sql_col})")
print("\n✓ Issue confirmed: Non-nullable UUID fields will fail to scan NULL values from LEFT JOINs")
EOFRepository: ctrlplanedev/ctrlplane
Length of output: 431
Make these outer-joined columns scan-safe.
These LEFT JOINs are selecting columns that can be NULL, but the generated rows in apps/workspace-engine/pkg/db/release_targets.sql.go are still non-nullable scalars (DesiredReleaseID uuid.UUID, DesiredReleaseVersionID uuid.UUID, DesiredVersionID uuid.UUID, LatestJobID uuid.UUID, and MeasurementID uuid.UUID). sqlc documents that nullable results should map to nullable pgx/database types, and the common failure mode here is a scan error when a NULL is read into a non-null destination, so a target with no desired release/latest job or a metric with no measurements will fail at scan time. Prefer nullable scan types like pgtype.UUID or explicit COALESCE sentinels in the query. (docs.sqlc.dev)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/pkg/db/queries/release_targets.sql` around lines 67 -
78, The query's LEFT JOINs (aliases rtdr, rel, dv, rj, j) produce nullable
columns but the generated scanner in
apps/workspace-engine/pkg/db/release_targets.sql.go expects non-nullable UUIDs
(DesiredReleaseID, DesiredReleaseVersionID, DesiredVersionID, LatestJobID,
MeasurementID); update the SQL to make those selected columns scan-safe by
either: a) changing the selected columns to nullable driver types (use
pgtype.UUID/pgtype.Int8 in your SQLC types mapping) or b) wrapping each nullable
column with a COALESCE sentinel (e.g. COALESCE(rel.id,
'00000000-0000-0000-0000-000000000000') for UUIDs or COALESCE(measurement.id, 0)
for ints) so the query always returns non-null values; apply the chosen approach
consistently for rtdr.desired_release_id, rel.version_id, dv.id, rj.job_id/j.id,
and measurement.id to prevent scan errors.
| if row.LatestJobID != nilUUID { | ||
| jobH := gin.H{ | ||
| "id": row.LatestJobID.String(), | ||
| "status": row.LatestJobStatus.JobStatus, | ||
| "message": row.LatestJobMessage.String, | ||
| "reason": row.LatestJobReason.JobReason, | ||
| "createdAt": row.LatestJobCreatedAt.Time, | ||
| "startedAt": row.LatestJobStartedAt.Time, | ||
| "completedAt": row.LatestJobCompletedAt.Time, | ||
| "updatedAt": row.LatestJobUpdatedAt.Time, | ||
| "externalId": row.LatestJobExternalID.String, | ||
| "jobAgentId": row.LatestJobAgentID, | ||
| "jobAgentConfig": row.LatestJobAgentConfig, | ||
| "dispatchContext": row.LatestJobDispatchContext, | ||
| } | ||
| if v, ok := verificationsMap[row.LatestJobID.String()]; ok { | ||
| jobH["verifications"] = v | ||
| } else { | ||
| jobH["verifications"] = []gin.H{} | ||
| } | ||
| item.LatestJob = jobH |
There was a problem hiding this comment.
latestJob doesn't match any generated API type.
The contract generated in this PR models latestJob as Job, but this code sends a partial job (metadata, releaseId, and workflowJobId are missing) plus a synthesized verifications array whose entries also are not JobVerifications (createdAt and message are missing, and id is derived from rule/job ids). That leaves the OpenAPI/SDK types incorrect for callers. Either return the documented shape exactly, or introduce a dedicated schema for this flattened job+verification payload.
Also applies to: 209-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go`
around lines 112 - 132, The response for LatestJob (built into item.LatestJob
using row.LatestJob* fields and verificationsMap) does not match the generated
API type Job nor JobVerification; either construct and return a payload that
exactly matches the generated Job/JobVerification types (populate missing fields
like metadata, releaseId, workflowJobId on the Job and ensure each verification
includes id, message, createdAt and any other required properties) or introduce
a new dedicated schema for the flattened job+verifications payload and update
the OpenAPI types and marshalling accordingly; locate the assembly of LatestJob
(the jobH map, item.LatestJob assignment, and verificationsMap usage) and either
add the missing Job fields from available row.* fields or change the response
type to the new schema and populate verifications entries to conform to
JobVerification.
| jobH := gin.H{ | ||
| "id": row.LatestJobID.String(), | ||
| "status": row.LatestJobStatus.JobStatus, | ||
| "message": row.LatestJobMessage.String, | ||
| "reason": row.LatestJobReason.JobReason, | ||
| "createdAt": row.LatestJobCreatedAt.Time, | ||
| "startedAt": row.LatestJobStartedAt.Time, | ||
| "completedAt": row.LatestJobCompletedAt.Time, | ||
| "updatedAt": row.LatestJobUpdatedAt.Time, | ||
| "externalId": row.LatestJobExternalID.String, | ||
| "jobAgentId": row.LatestJobAgentID, | ||
| "jobAgentConfig": row.LatestJobAgentConfig, | ||
| "dispatchContext": row.LatestJobDispatchContext, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go | sed -n '110,130p'Repository: ctrlplanedev/ctrlplane
Length of output: 1039
🏁 Script executed:
cat -n apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go | sed -n '195,225p'Repository: ctrlplanedev/ctrlplane
Length of output: 1587
🏁 Script executed:
# Check if the sqlc-generated file exists and inspect type mappings
ls -la apps/workspace-engine/pkg/db/ | grep release_targetsRepository: ctrlplanedev/ctrlplane
Length of output: 139
🏁 Script executed:
# Search for JSONB column definitions in the sql file
fd "release_targets" apps/workspace-engine/ --type f | grep -E "\.(sql|go)$"Repository: ctrlplanedev/ctrlplane
Length of output: 242
🏁 Script executed:
# Find the type definitions in the sqlc-generated file
rg "LatestJobAgentConfig|LatestJobDispatchContext|MeasurementData|MetricProvider" apps/workspace-engine/pkg/db/release_targets.sql.go -A 2 -B 2Repository: ctrlplanedev/ctrlplane
Length of output: 963
🏁 Script executed:
# Get the struct definitions that contain these fields
rg "type.*struct" apps/workspace-engine/pkg/db/release_targets.sql.go -A 20 | head -150Repository: ctrlplanedev/ctrlplane
Length of output: 2420
🏁 Script executed:
# Search for any unmarshaling of these JSONB fields before they're added to gin.H
rg "jobAgentConfig|dispatchContext|MeasurementData|MetricProvider" apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go -B 5 -A 5Repository: ctrlplanedev/ctrlplane
Length of output: 2941
🏁 Script executed:
# Check if there's any JSON unmarshaling in the getters.go file at all
rg "json.Unmarshal|Unmarshal" apps/workspace-engine/svc/http/server/openapi/release_targets/getters.goRepository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Check the sqlc.yaml configuration to see if there's an override for JSONB types
find apps/workspace-engine -name "sqlc.yaml" -o -name "sqlc.yml" | xargs catRepository: ctrlplanedev/ctrlplane
Length of output: 6549
Decode JSONB blobs before adding them to gin.H.
jobAgentConfig, dispatchContext, provider, and data fields are mapped to []byte in the sqlc-generated types. When Go's JSON encoder serializes []byte values, they become base64 strings instead of structured JSON. This breaks the advertised OpenAPI shapes. Unmarshal these fields to concrete types (e.g., map[string]any or a struct) before adding them to gin.H, or update the sqlc overrides in sqlc.yaml to use map[string]any for consistency with other JSONB columns (e.g., deployment_version.job_agent_config).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go`
around lines 113 - 125, The JSONB fields in the job response (jobH map built in
getters.go) are currently raw []byte (jobAgentConfig, dispatchContext, provider,
data) which will be JSON-encoded as base64; unmarshal those []byte blobs into
concrete JSON values (e.g., map[string]any or a typed struct) before inserting
into jobH so the HTTP response emits proper JSON objects; locate the creation of
jobH (fields: LatestJobAgentConfig, LatestJobDispatchContext, LatestJobProvider,
LatestJobData / keys "jobAgentConfig","dispatchContext","provider","data") and
replace the direct []byte assignment with json.Unmarshal into a value and use
that value (or nil on empty) or update sqlc.yaml overrides to map those columns
to map[string]any so the generated types are already decoded.
| vRows, err := queries.ListVerificationMetricsByJobIDs(ctx, jobIDs) | ||
| if err != nil { | ||
| return verificationsMap | ||
| } |
There was a problem hiding this comment.
Don't treat verification lookup failures as "no verifications".
Returning an empty map here makes a database/query failure indistinguishable from a legitimate empty verification set. That silently drops data from the endpoint and will hide backend issues from the UI. Propagate the error or mark the response as partial instead of pretending the lookup succeeded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/getters.go`
around lines 163 - 166, The current code swallows DB/query failures from
ListVerificationMetricsByJobIDs by returning verificationsMap (empty) on error;
change this to propagate the error instead of treating it as an empty
result—when ListVerificationMetricsByJobIDs returns a non-nil err, return that
error (or a wrapped error) to the caller so the endpoint can surface a failure
or mark the response partial; update the surrounding function's signature/return
path accordingly so callers of the getter can handle/return the error rather
than silently dropping verification data.
| items, err := rt.getter.ListReleaseTargets(c.Request.Context(), deploymentId) | ||
| if err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
| return |
There was a problem hiding this comment.
Return 400 for invalid deployment IDs instead of 500.
PostgresGetter.ListReleaseTargets turns a bad UUID into a validation error, but this handler maps every error to 500 and echoes the raw message. That makes client input bugs look like server outages and leaks internal details. Split validation failures into 400 and keep 500 responses generic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/svc/http/server/openapi/release_targets/server.go`
around lines 18 - 21, The handler that calls rt.getter.ListReleaseTargets
currently returns 500 and echoes the raw error; change it to detect validation
errors from PostgresGetter.ListReleaseTargets (use errors.Is / errors.As to
match the getter's validation/UUID error or the specific validation error type
returned) and respond with c.JSON(http.StatusBadRequest, gin.H{"error":"invalid
deployment id"}) for those cases; for all other errors, log the detailed error
server-side and return c.JSON(http.StatusInternalServerError,
gin.H{"error":"internal server error"}) instead of echoing internal messages.
Ensure you update the code around the call to rt.getter.ListReleaseTargets and
use c.Request.Context(), c.JSON and proper logging when handling the
non-validation branch.
1 participant
Summary by CodeRabbit
New Features
API Changes
Infrastructure