Fix: Support snapshots for non-default clusters#2947
Merged
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit d92ff3f. Bugbot is set up for automated code reviews on this repo. Configure here. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the orchestrator to propagate and store the ClusterID when pausing instances and creating snapshot templates. Specifically, it conditionally passes the cluster ID (if it is not the local cluster ID) to the database queries for upserting snapshots and creating snapshot template environments, and updates the corresponding SQL queries to insert the cluster_id into the envs table. There are no review comments, and we have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
❌ 4 Tests Failed:
View the top 1 failed test(s) by shortest run time
View the full list of 3 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — clean fix that propagates ClusterID into the snapshot envs row, following the same nil-as-LocalClusterID idiom already used in register_build.go.
Extended reasoning...
Overview
This PR fixes snapshot creation for sandboxes running on non-default (non-local) clusters by populating the envs.cluster_id column on the new envs rows inserted by UpsertSnapshot and CreateSnapshotTemplateEnv. The change touches two Go callsites in packages/api/internal/orchestrator and the two corresponding sqlc-generated query files plus their source .sql files.
Security risks
None. The change only routes an additional cluster identifier (already trusted, derived from the sandbox record) into a new column on the envs table. The column is a nullable FK to clusters(id), and the Go code maps LocalClusterID (uuid.Nil) to a Go nil pointer / SQL NULL, which is the same convention used elsewhere in the codebase (register_build.go:163-166).
Level of scrutiny
Low. This is a small, mechanical data-propagation fix following an established pattern. The SQL parameter renumbering in both generated files was tedious but correct: every $N after position 2 shifts by +1 in both queries, and the Go struct/QueryRow argument order matches the new placeholder order. The migration 20250624001048_cluster_for_templates.sql confirms envs.cluster_id is nullable, so existing local-cluster behavior (NULL cluster_id) is preserved.
Other factors
The bug hunting system reported no findings. The new code mirrors an existing pattern that's covered by other tests (create_instance_test.go, routing_test.go). No callers/contracts changed — only an additional optional field is set on insert.
jakubno
approved these changes
Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add cluster ID (if not default) to snaphost, allowing to resume template when using BYOC.