Skip to content

Commit 8a10375

Browse files
committed
feat/batch-changes: drop unneeded v3 cache-key canonicalization
The Step.MarshalJSON canonicalization and v3 image→container parse-time mirroring are not required for server-side codingAgent execution: the server canonicalizes container on the wire, so src-cli receives container-only JSON and reads step.Container directly. These changes hardened src-cli's local handling of v3 specs (preview / validate / apply paths) but were already broken pre-PR; restoring that is out of scope here. Drop them to keep this PR focused on the codingAgent feature.
1 parent 02c8933 commit 8a10375

2 files changed

Lines changed: 3 additions & 73 deletions

File tree

lib/batches/batch_spec.go

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package batches
22

33
import (
4-
"encoding/json"
54
"fmt"
65
"strings"
76

@@ -113,23 +112,6 @@ type CodingAgentStep struct {
113112
Prompt string `json:"prompt,omitempty" yaml:"prompt"`
114113
}
115114

116-
// MarshalJSON canonicalizes the v3 `image:` field into `container:` on the
117-
// wire. Both fields exist on Step for ergonomic reasons (v3 specs use
118-
// `image:`, v1/v2 specs use `container:`), but src-cli's executor reads
119-
// `Container`. Without canonicalization, the prep-side cache key — computed
120-
// by JSON-marshaling Step — would include `image` while the executor side
121-
// (which round-trips through src-cli) would not, producing divergent keys
122-
// and silent cache misses for any v3 spec.
123-
func (s Step) MarshalJSON() ([]byte, error) {
124-
type stepAlias Step
125-
canon := stepAlias(s)
126-
if canon.Container == "" {
127-
canon.Container = canon.Image
128-
}
129-
canon.Image = ""
130-
return json.Marshal(canon)
131-
}
132-
133115
func (s *Step) IfCondition() string {
134116
switch v := s.If.(type) {
135117
case bool:
@@ -192,16 +174,6 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) {
192174
return nil, err
193175
}
194176

195-
if spec.Version == 3 {
196-
// Mirror v3 `image:` into `container:` so in-memory consumers that
197-
// read step.Container (e.g. the executor transform) keep working.
198-
// JSON serialization is canonicalized separately in Step.MarshalJSON
199-
// so prep-side cache hashing matches src-cli/executor serialization.
200-
for i := range spec.Steps {
201-
spec.Steps[i].Container = spec.Steps[i].Image
202-
}
203-
}
204-
205177
var errs error
206178

207179
if len(spec.Steps) != 0 && spec.ChangesetTemplate == nil {
@@ -225,8 +197,7 @@ func parseBatchSpec(schema string, data []byte) (*BatchSpec, error) {
225197
if step.CodingAgent != nil && step.Run != "" {
226198
errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent and run cannot be combined in the same step", i+1)))
227199
}
228-
if step.CodingAgent != nil && step.Container == "" {
229-
// v3 image: is mirrored into Container above.
200+
if step.CodingAgent != nil && step.Container == "" && step.Image == "" {
230201
errs = errors.Append(errs, NewValidationError(errors.Newf("step %d: codingAgent step requires an image", i+1)))
231202
}
232203
}

lib/batches/batch_spec_test.go

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package batches
22

33
import (
4-
"encoding/json"
54
"fmt"
65
"strings"
76
"testing"
@@ -27,46 +26,6 @@ changesetTemplate:
2726
`, stepsYAML))
2827
}
2928

30-
func TestStep_MarshalJSON_canonicalizesImageToContainer(t *testing.T) {
31-
v3FromImage, err := json.Marshal(Step{Image: "alpine:3", Run: "echo hi"})
32-
if err != nil {
33-
t.Fatalf("marshal v3-shaped step: %v", err)
34-
}
35-
v1FromContainer, err := json.Marshal(Step{Container: "alpine:3", Run: "echo hi"})
36-
if err != nil {
37-
t.Fatalf("marshal v1-shaped step: %v", err)
38-
}
39-
if string(v3FromImage) != string(v1FromContainer) {
40-
t.Errorf("canonical JSON differs:\n v3 image: %s\n v1 container: %s", v3FromImage, v1FromContainer)
41-
}
42-
var out map[string]any
43-
if err := json.Unmarshal(v3FromImage, &out); err != nil {
44-
t.Fatalf("unmarshal: %v", err)
45-
}
46-
if _, ok := out["image"]; ok {
47-
t.Errorf("expected no image key in canonical JSON, got %s", v3FromImage)
48-
}
49-
if got, _ := out["container"].(string); got != "alpine:3" {
50-
t.Errorf("container: got %v want alpine:3 (full=%s)", out["container"], v3FromImage)
51-
}
52-
}
53-
54-
func TestParseBatchSpec_v3_imageMirroredToContainer(t *testing.T) {
55-
got, err := ParseBatchSpec(v3SpecWith(" - run: echo hi\n image: alpine:3"))
56-
if err != nil {
57-
t.Fatalf("ParseBatchSpec failed: %v", err)
58-
}
59-
if len(got.Steps) != 1 {
60-
t.Fatalf("expected 1 step, got %d", len(got.Steps))
61-
}
62-
if got.Steps[0].Image != "alpine:3" {
63-
t.Errorf("Step.Image: got %q want %q", got.Steps[0].Image, "alpine:3")
64-
}
65-
if got.Steps[0].Container != "alpine:3" {
66-
t.Errorf("Step.Container (mirrored from image): got %q want %q", got.Steps[0].Container, "alpine:3")
67-
}
68-
}
69-
7029
func TestParseBatchSpec_v3_codingAgentRequiresImage(t *testing.T) {
7130
_, err := ParseBatchSpec(v3SpecWith(" - codingAgent:\n type: codex\n prompt: do the thing"))
7231
if err == nil {
@@ -95,7 +54,7 @@ func TestParseBatchSpec_v3_codingAgentStep(t *testing.T) {
9554
if step.CodingAgent.Prompt != "do the thing" {
9655
t.Errorf("CodingAgent.Prompt: got %q want %q", step.CodingAgent.Prompt, "do the thing")
9756
}
98-
if step.Container != "alpine:3" {
99-
t.Errorf("Step.Container (mirrored from image): got %q want %q", step.Container, "alpine:3")
57+
if step.Image != "alpine:3" {
58+
t.Errorf("Step.Image: got %q want %q", step.Image, "alpine:3")
10059
}
10160
}

0 commit comments

Comments
 (0)