template-builder: pass flatten=true to CreateSnapshot

[meAmitPatil]

Summary

Tells the forked firecracker (superserve-ai/firecracker#11) to bake each overlay's dirty blocks into its base.ext4 and zero the side-car bitmap as part of snapshot creation. Sandboxes restored from templates built after this lands start with an empty per-VM overlay — no apply_delta replay on create.

Single new flatten bool arg on vm.CreateSnapshot. Template-builder passes true; the two other callers (regular snapshot + diff snapshot) pass false.

Deploy order: firecracker PR #11 must land + new binary deployed first. With old firecracker, flatten: true is silently ignored (the field is #[serde(default)]), so the worst case during rollout is templates that aren't flat yet — no breakage.

---

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[stage-review]

Ready to review this PR? Stage has broken it down into 4 individual chapters for you:

	Title
1	Add flatten field to Firecracker snapshot model
2	Introduce SnapshotMode and update CreateSnapshot signature
3	Enable flattening in template-builder and update callers
4	Test snapshot serialization and validation logic

_{Chapters generated by Stage for commit d6335b0 on May 20, 2026 7:03pm UTC.}

[pavitrabhalla]

Overall this is clean and well-scoped — the zero-value-is-safe enum design and the deploy-order note are both good. A few things worth addressing before merge:

1. Missing guard: flatten=true requires non-empty blockDeltaDir

The model comment says "Requires block_delta_dir" but CreateSnapshot doesn't enforce it. A caller passing SnapshotFlatten with blockDeltaDir="" will send {"flatten":true} with no block_delta_dir and get a confusing error back from Firecracker. Suggest adding an early return in CreateSnapshot:

if mode == SnapshotFlatten && blockDeltaDir == "" {
    return fmt.Errorf("flatten mode requires a non-empty blockDeltaDir")
}

2. Data race in the test

capturedBody is written in the HTTP handler goroutine and read on the test goroutine after CreateSnapshot returns. It works in practice (response is sent after body is read, so write happens-before return), but go test -race will flag it. A sync.Mutex around the read/write fixes it.

3. Server readiness race in the test

go srv.Serve(ln) is called immediately before CreateSnapshot with no guarantee the server is ready. Usually fine, but can produce flaky results. A small net.Dial retry loop or a ready channel would harden it.

[meAmitPatil]

@pavitrabhalla Pushed Fixes.

[pavitrabhalla]

Blocking: nothing.

Non-blocking suggestions:

• TestCreateSnapshot_FlattenFieldInJSONBody passes a non-empty blockDeltaDir for both cases, but real PauseVM/CreateVMSnapshot calls use an empty one. Worth adding a SnapshotNormal + "" case to cover that path.

• For the normal-mode test case, json.Unmarshal returns false for both "field absent" and "flatten": false — so the assertion wouldn't catch an accidentally dropped omitempty tag. A tighter check would assert !strings.Contains(string(body), "flatten") for that case.