Persist snapshot scheduling metadata#2873
Open
ValentaTomas wants to merge 7 commits into
Open
Conversation
Return compact build-chain metadata from snapshot creation so API placement can later use real build ancestry for resume affinity.
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit e55b31b. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
❌ 5 Tests Failed:
View the full list of 6 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![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
In update_snapshot_scheduling_metadata.sql, using to_jsonb on a text parameter stores it as an escaped string scalar rather than a JSONB object, so the parameter should be cast directly to jsonb. In sandboxes.go, accessing res.SchedulingMetadata will cause a compilation error because the snapshotResult struct lacks this field. Additionally, NewSnapshotSchedulingMetadata in snapshot.go is missing nil checks for the Mapping fields of the headers, which can lead to a nil pointer dereference panic.
Populate pause/checkpoint responses from snapshot results and store scheduling metadata as JSONB.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Pause fails after orchestrator pause
- Added failSnapshotBuild call when updateSnapshotSchedulingMetadata fails after successful pause RPC to properly mark build status as failed.
- ✅ Fixed: Metadata error finishes snapshot success
- Added finish(err) call before returning when updateSnapshotSchedulingMetadata fails to prevent deferred finish(nil) from incorrectly marking snapshot as success.
Or push these changes by commenting:
@cursor push caa2284235
Preview (caa2284235)
diff --git a/packages/api/internal/orchestrator/pause_instance.go b/packages/api/internal/orchestrator/pause_instance.go
--- a/packages/api/internal/orchestrator/pause_instance.go
+++ b/packages/api/internal/orchestrator/pause_instance.go
@@ -52,6 +52,8 @@
}
if schedulingMetadata != nil {
if err := o.updateSnapshotSchedulingMetadata(ctx, sbx.SandboxID, schedulingMetadata); err != nil {
+ o.failSnapshotBuild(ctx, result.BuildID, err)
+
return err
}
}
diff --git a/packages/api/internal/orchestrator/snapshot_template.go b/packages/api/internal/orchestrator/snapshot_template.go
--- a/packages/api/internal/orchestrator/snapshot_template.go
+++ b/packages/api/internal/orchestrator/snapshot_template.go
@@ -106,6 +106,8 @@
if err := o.updateSnapshotSchedulingMetadata(ctx, sbx.SandboxID, metadata); err != nil {
o.failSnapshotBuild(ctx, upsertResult.BuildID, err)
+ finish(err)
+
return SnapshotTemplateResult{}, err
}
}You can send follow-ups to the cloud agent here.
Do not fail a successful pause or checkpoint if persisting optional scheduling metadata fails.
|
Bugbot Autofix prepared fixes for both issues found in the latest run.
Or push these changes by commenting: Preview (3c928ccbac)diff --git a/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql b/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql
--- a/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql
+++ b/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql
@@ -3,7 +3,7 @@
SET metadata = jsonb_set(
metadata,
'{snapshot_scheduling_metadata}',
- to_jsonb(@metadata::text),
+ (@metadata::text)::jsonb,
true
)
WHERE sandbox_id = @sandbox_id;
diff --git a/packages/db/queries/update_snapshot_scheduling_metadata.sql.go b/packages/db/queries/update_snapshot_scheduling_metadata.sql.go
--- a/packages/db/queries/update_snapshot_scheduling_metadata.sql.go
+++ b/packages/db/queries/update_snapshot_scheduling_metadata.sql.go
@@ -1,6 +1,6 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
-// sqlc v1.31.1
+// sqlc v1.29.0
// source: update_snapshot_scheduling_metadata.sql
package queries
@@ -14,7 +14,7 @@
SET metadata = jsonb_set(
metadata,
'{snapshot_scheduling_metadata}',
- to_jsonb($1::text),
+ ($1::text)::jsonb,
true
)
WHERE sandbox_id = $2
diff --git a/packages/orchestrator/pkg/server/sandboxes.go b/packages/orchestrator/pkg/server/sandboxes.go
--- a/packages/orchestrator/pkg/server/sandboxes.go
+++ b/packages/orchestrator/pkg/server/sandboxes.go
@@ -782,9 +782,10 @@
// snapshotResult holds the data produced by snapshotAndCacheSandbox that
// callers need to start the background remote storage upload.
type snapshotResult struct {
- meta metadata.Template
- upload *sandbox.Upload
- completeUpload func(ctx context.Context, uploadErr error)
+ meta metadata.Template
+ upload *sandbox.Upload
+ completeUpload func(ctx context.Context, uploadErr error)
+ SchedulingMetadata *orchestrator.SnapshotSchedulingMetadata
}
// snapshotAndCacheSandbox creates a snapshot of a sandbox and adds it to the
@@ -863,9 +864,10 @@
}
return &snapshotResult{
- meta: meta,
- upload: upload,
- completeUpload: completeUpload,
+ meta: meta,
+ upload: upload,
+ completeUpload: completeUpload,
+ SchedulingMetadata: snapshot.SchedulingMetadata,
}, nil
}You can send follow-ups to the cloud agent here. |
Use slices.SortFunc and remove an unnecessary conversion in metadata generation.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Limit one drops parent build
- Added condition to prevent base build from overwriting parent build when chain limit is 1 and last element is the parent.
Or push these changes by commenting:
@cursor push 527fa53cf5
Preview (527fa53cf5)
diff --git a/packages/orchestrator/pkg/sandbox/snapshot.go b/packages/orchestrator/pkg/sandbox/snapshot.go
--- a/packages/orchestrator/pkg/sandbox/snapshot.go
+++ b/packages/orchestrator/pkg/sandbox/snapshot.go
@@ -139,7 +139,7 @@
break
}
}
- if !hasBase {
+ if !hasBase && chain[len(chain)-1].buildID != parentBuildID {
if c, ok := contributions[baseBuildID]; ok {
chain[len(chain)-1] = *c
} else {You can send follow-ups to the cloud agent here.
Avoid replacing the only chain entry with the base build because base build is already stored separately.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Nested metadata breaks JSONBStringMap scans
- ✅ Fixed: Upsert clears scheduling metadata key
- Changed create_new_snapshot.sql conflict handler to use snapshots.metadata || excluded.metadata instead of excluded.metadata, preserving existing metadata keys including snapshot_scheduling_metadata during upsert.
Or push these changes by commenting:
@cursor push c71543e849
Preview (c71543e849)
diff --git a/packages/db/queries/create_new_snapshot.sql.go b/packages/db/queries/create_new_snapshot.sql.go
--- a/packages/db/queries/create_new_snapshot.sql.go
+++ b/packages/db/queries/create_new_snapshot.sql.go
@@ -55,7 +55,7 @@
now()
)
ON CONFLICT (sandbox_id) DO UPDATE SET
- metadata = excluded.metadata,
+ metadata = snapshots.metadata || excluded.metadata,
sandbox_started_at = excluded.sandbox_started_at,
allow_internet_access = COALESCE(excluded.allow_internet_access, snapshots.allow_internet_access),
origin_node_id = excluded.origin_node_id,
diff --git a/packages/db/queries/snapshots/create_new_snapshot.sql b/packages/db/queries/snapshots/create_new_snapshot.sql
--- a/packages/db/queries/snapshots/create_new_snapshot.sql
+++ b/packages/db/queries/snapshots/create_new_snapshot.sql
@@ -41,7 +41,7 @@
now()
)
ON CONFLICT (sandbox_id) DO UPDATE SET
- metadata = excluded.metadata,
+ metadata = snapshots.metadata || excluded.metadata,
sandbox_started_at = excluded.sandbox_started_at,
allow_internet_access = COALESCE(excluded.allow_internet_access, snapshots.allow_internet_access),
origin_node_id = excluded.origin_node_id,
diff --git a/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql b/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql
--- a/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql
+++ b/packages/db/queries/snapshots/update_snapshot_scheduling_metadata.sql
@@ -3,7 +3,7 @@
SET metadata = jsonb_set(
metadata,
'{snapshot_scheduling_metadata}',
- (@metadata::text)::jsonb,
+ to_jsonb(@metadata::text),
true
)
WHERE sandbox_id = @sandbox_id;
diff --git a/packages/db/queries/update_snapshot_scheduling_metadata.sql.go b/packages/db/queries/update_snapshot_scheduling_metadata.sql.go
--- a/packages/db/queries/update_snapshot_scheduling_metadata.sql.go
+++ b/packages/db/queries/update_snapshot_scheduling_metadata.sql.go
@@ -1,6 +1,6 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
-// sqlc v1.31.1
+// sqlc v1.29.0
// source: update_snapshot_scheduling_metadata.sql
package queries
@@ -14,7 +14,7 @@
SET metadata = jsonb_set(
metadata,
'{snapshot_scheduling_metadata}',
- ($1::text)::jsonb,
+ to_jsonb($1::text),
true
)
WHERE sandbox_id = $2You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit af997d5. Configure here.
Store scheduling metadata as a string value in the existing metadata map and keep it across snapshot upserts until refreshed.
dobrac
requested changes
May 30, 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.
Persist compact snapshot scheduling metadata from pause/checkpoint so future placement can use real build-chain ancestry. Metadata persistence is best-effort and does not change snapshot success semantics.