amadan
settings-4 task 1: fix round 1: NextBuildState keeps the newest resolution; guard tests
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+24
−1
internal/release/buildstate.go+12 −1internal/release/buildstate_test.go+12 −0
diff --git a/internal/release/buildstate.go b/internal/release/buildstate.go| index 6a1fb7a7c..a8370147f 100644 |
| --- a/internal/release/buildstate.go |
| +++ b/internal/release/buildstate.go |
| @@ -51,6 +51,8 @@ type BuildStateCopy struct { |
| ReadErr string `json:"read_err,omitempty"` |
| } |
| +// ParseBuildStateCopy reads the registry column. An empty column is the zero |
| +// copy: Known false, nothing read yet. |
| func ParseBuildStateCopy(col string) (BuildStateCopy, error) { |
| if col == "" { |
| return BuildStateCopy{}, nil |
| @@ -62,6 +64,7 @@ func ParseBuildStateCopy(col string) (BuildStateCopy, error) { |
| return c, nil |
| } |
| +// Column is the registry column text for c; ParseBuildStateCopy reverses it. |
| func (c BuildStateCopy) Column() string { |
| b, _ := json.Marshal(c) |
| return string(b) |
| @@ -87,6 +90,9 @@ type Consumed struct { |
| At time.Time `json:"at"` |
| } |
| +// ConsumedPath is the path for host's consumed file. The caller must pass a |
| +// host already validated with controlname.ValidHostShape (ReadConsumed and |
| +// WriteConsumed do); an unvalidated host could escape consumed/. |
| func ConsumedPath(optDir, host string) string { return filepath.Join(optDir, "consumed", host) } |
| // ReadConsumed is (nil, nil) when root has never written one for host. |
| @@ -156,7 +162,12 @@ func NextBuildState(cur *BuildState, c *Consumed, build string) BuildState { |
| next.Resolved = copyResolved(cur.Resolved) |
| } |
| if c != nil { |
| - if c.Resolved != nil && c.Resolved.ID > Watermark(nil, &next) { |
| + // Carry root's resolution only when it is newer than everything the |
| + // state already knows: above the watermark of both sources and above |
| + // the state's own resolution. An instance that moved A to B and back |
| + // meets A's old consumed file, which must not overwrite B's newer R. |
| + if c.Resolved != nil && c.Resolved.ID > Watermark(c, &next) && |
| + (next.Resolved == nil || c.Resolved.ID > next.Resolved.ID) { |
| next.Resolved = copyResolved(c.Resolved) |
| } |
| switch { |
diff --git a/internal/release/buildstate_test.go b/internal/release/buildstate_test.go| index 611c35f8b..06acc9315 100644 |
| --- a/internal/release/buildstate_test.go |
| +++ b/internal/release/buildstate_test.go |
| @@ -115,6 +115,18 @@ func TestNextBuildState(t *testing.T) { |
| &BuildState{Schema: 1, Host: "h", Completed: 6}, |
| &Consumed{Completed: 4}, "v1", |
| BuildState{Schema: 1, Host: "h", LastExecuted: "v1", Completed: 6}}, |
| + {"i: a resolution below the consumed Completed never lowers the watermark", |
| + &BuildState{Schema: 1, Host: "h", Resolved: &ResolvedRequest{ID: 3, Target: "v3"}, Completed: 2}, |
| + &Consumed{Completed: 5, CompletedFailed: true}, "v3", |
| + BuildState{Schema: 1, Host: "h", LastExecuted: "v3", Resolved: &ResolvedRequest{ID: 3, Target: "v3"}, Completed: 5, CompletedFailed: true}}, |
| + {"ii: a closed request whose target runs again stays closed and failed", |
| + &BuildState{Schema: 1, Host: "h", Resolved: &ResolvedRequest{ID: 3, Target: "v3"}, Completed: 3, CompletedFailed: true}, |
| + nil, "v3", |
| + BuildState{Schema: 1, Host: "h", LastExecuted: "v3", Resolved: &ResolvedRequest{ID: 3, Target: "v3"}, Completed: 3, CompletedFailed: true}}, |
| + {"j: an older resolution in this box's consumed file does not replace the state's newer one", |
| + &BuildState{Schema: 1, Host: "h", Resolved: &ResolvedRequest{ID: 5, Target: "v5"}, Completed: 3}, |
| + &Consumed{Resolved: &ResolvedRequest{ID: 4, Target: "v4"}, Completed: 3}, "v5", |
| + BuildState{Schema: 1, Host: "h", LastExecuted: "v5", Resolved: &ResolvedRequest{ID: 5, Target: "v5"}, Completed: 5}}, |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| var curBefore BuildState |