Refactor: Update envs queries to account for env_build_assignments relation migration #237
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ben-fornefeld
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![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: 010a73d879
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| query = query.or( | ||
| `env_build_assignments.env_id.eq.${resolvedEnvId},id.eq.${buildIdOrTemplate}` |
There was a problem hiding this comment.
When buildIdOrTemplate is a template UUID, this branch builds or(env_build_assignments.env_id...,id...), which mixes an embedded-table predicate with a top-level predicate in one or clause. In that path, template-id searches can fail or return incorrect rows because the relation-scoped condition is not reliably evaluated alongside the base-table condition; this is the hot path for UUID template filters and should be split into separate filters/queries.
Useful? React with 👍 / 👎.
| build: RawListedBuildWithEnvAndAliasesDB | ||
| ): ListedBuildDTO { | ||
| const alias = build.envs.env_aliases?.[0]?.alias | ||
| const assignment = build.env_build_assignments[0] |
There was a problem hiding this comment.
This mapping takes env_build_assignments[0] without any ordering or context filter, so builds with multiple assignments can be labeled with an arbitrary template/templateId depending on returned row order. That makes build rows unstable and can route users to the wrong template context in downstream build-detail/log flows.
Useful? React with 👍 / 👎.
ben-fornefeld
force-pushed
the
refactor/envs-assignments-relation-migration
branch
from
feb4cdc to
ec70993
Compare
ben-fornefeld
force-pushed
the
refactor/envs-assignments-relation-migration
branch
from
3afa4c5 to
b6ae384
Compare
- Removed unnecessary team_id from envs in RawListedBuildWithEnvAndAliasesDB type. - Updated build query functions to utilize a new helper for fetching team environment IDs. - Adjusted listBuilds and getBuildInfo functions to filter builds based on team-specific environment IDs. - Enhanced error handling for cases where no environments are found for a team.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| if (statuses.length < ALL_BUILD_STATUSES.length) { | ||
| query = query.in('status', statuses) | ||
| } |
There was a problem hiding this comment.
Length-based status filter skipped on duplicate statuses
Medium Severity
The conditional statuses.length < ALL_BUILD_STATUSES.length uses array length to decide whether to apply the status filter. The caller in the router uses flatMap(mapBuildStatusDTOToDatabaseBuildStatus), which expands 'building' into ['building', 'waiting']. If the client sends duplicate DTO statuses (e.g., ['building', 'building']), the flattened array has 4 elements — equal to ALL_BUILD_STATUSES.length — so the filter is incorrectly skipped, returning builds of all statuses. The old code always applied .in('status', statuses), where SQL's IN clause naturally deduplicates.
1 participant
Note
Medium Risk
Touches build listing/lookup queries and alias resolution logic, which could change which builds are returned or how they’re labeled; overall behavior should be equivalent but depends on the new join data being present and correct.
Overview
Updates build listing and lookup to use the new
env_build_assignmentsjoin instead ofenv_builds.env_id, including team scoping, filtering by template, and fetching aliases separately for display.Refactors
listBuildsto resolve template IDs by either ID or alias, handle the “build UUID vs template” ambiguity via merged queries, and centralizes query construction/cursor filtering.Separately normalizes
spec/openapi.infra.yamlquoting (response codes and$refs) and updates generated DB types to match current Supabase schema metadata.Written by Cursor Bugbot for commit aa2dea8. This will update automatically on new commits. Configure here.