amadan
provision: a run id is never re-admitted, RunPick refuses malformed input (review round 1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+55
−0
internal/provision/run.go+9 −0internal/provision/run_test.go+46 −0
CI cancelled — superseded by a newer push — run details
diff --git a/internal/provision/run.go b/internal/provision/run.go| index aa0518a1c..b1c53d8c0 100644 |
| --- a/internal/provision/run.go |
| +++ b/internal/provision/run.go |
| @@ -114,6 +114,11 @@ func RunAdmit(p RunAdmission) func(*Record) error { |
| return ErrRunMoving |
| case rec.ActiveRun.Open(): |
| return &RunOpenError{Run: *rec.ActiveRun} |
| + case rec.ActiveRun != nil && rec.ActiveRun.ID == p.ID: |
| + // A terminal run's id is never reused: a box still retrying |
| + // RunPick for it would launch a run its caller cancelled or that |
| + // expired (codex R2-6). |
| + return fmt.Errorf("%w: run id already used", ErrRunConflict) |
| case rec.ImportUncommitted(): |
| return ErrRunImport |
| } |
| @@ -140,6 +145,10 @@ func runIn(rec *Record, id string, states ...string) (RunState, error) { |
| // own deadline (run spec §5.2). Only the box whose CAS lands launches. |
| func RunPick(id, box, boot string, deadline time.Time) func(*Record) error { |
| return func(rec *Record) error { |
| + if box == "" || boot == "" || deadline.IsZero() { |
| + // A zero deadline would make the run lost the moment it is picked. |
| + return fmt.Errorf("%w: malformed pick", ErrRunConflict) |
| + } |
| r, err := runIn(rec, id, RunQueued) |
| if err != nil { |
| return err |
diff --git a/internal/provision/run_test.go b/internal/provision/run_test.go| index 419494e82..5f8243a78 100644 |
| --- a/internal/provision/run_test.go |
| +++ b/internal/provision/run_test.go |
| @@ -207,3 +207,49 @@ func TestOrdinaryWritersCarryActiveRun(t *testing.T) { |
| t.Fatalf("UpdateRecord: run %s, want %s", got, want) |
| } |
| } |
| + |
| +// A terminal run's id is not admitted again, whatever its state: a box |
| +// retrying RunPick for it must not find it queued (codex R2-6). |
| +func TestRunAdmitRefusesAReusedID(t *testing.T) { |
| + s := release.NewDirStore(t.TempDir()) |
| + rec := runTestRecord(t, s) |
| + at := time.Now().Add(time.Minute) |
| + if _, err := setRun(t, s, rec, RunAdmit(RunAdmission{ID: "run_a", Deadline: at})); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := setRun(t, s, rec, RunCancelTransition("run_a")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := setRun(t, s, rec, RunAdmit(RunAdmission{ID: "run_a", Deadline: at})); !errors.Is(err, ErrRunConflict) { |
| + t.Fatalf("re-admitting a cancelled run's id: %v, want ErrRunConflict", err) |
| + } |
| + got, _, err := GetRecordETag(context.Background(), s, rec.Account, rec.App, rec.Host) |
| + if err != nil || got.ActiveRun.State != RunCancelled { |
| + t.Fatalf("the cancelled run must be untouched: %+v, %v", got.ActiveRun, err) |
| + } |
| + if _, err := setRun(t, s, rec, RunAdmit(RunAdmission{ID: "run_b", Deadline: at})); err != nil { |
| + t.Fatalf("a new id replaces the terminal run: %v", err) |
| + } |
| +} |
| + |
| +func TestRunPickRefusesMalformedInput(t *testing.T) { |
| + s := release.NewDirStore(t.TempDir()) |
| + rec := runTestRecord(t, s) |
| + at := time.Now().Add(time.Minute) |
| + if _, err := setRun(t, s, rec, RunAdmit(RunAdmission{ID: "run_a", Deadline: at})); err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, mut := range map[string]func(*Record) error{ |
| + "no box": RunPick("run_a", "", "boot-1", at), |
| + "no boot": RunPick("run_a", "box-1", "", at), |
| + "no deadline": RunPick("run_a", "box-1", "boot-1", time.Time{}), |
| + } { |
| + if _, err := setRun(t, s, rec, mut); !errors.Is(err, ErrRunConflict) { |
| + t.Errorf("%s: %v, want ErrRunConflict", name, err) |
| + } |
| + } |
| + got, _, err := GetRecordETag(context.Background(), s, rec.Account, rec.App, rec.Host) |
| + if err != nil || got.ActiveRun.State != RunQueued { |
| + t.Fatalf("a refused pick must leave the run queued: %+v, %v", got.ActiveRun, err) |
| + } |
| +} |