merge fix: a run records its serving build before its command starts
Second merge review, finding 1. Main's settings work requires the build about to touch an instance's database to be recorded first (settings spec 4.2, recordBuild), and refuses the start when that write fails. RunCmd started the command without it. If a successor B recorded its build and then failed before it replaced the serving generation A, a later command ran A under a build state that still named B, so a held recovery could pick the wrong build. RunCmd now does what main's sidecar start does: - It records the serving generation's build before StartCmd, under the starting hold. A stop that begins meanwhile waits for the start, and no overlap swap is admitted, so the generation recorded is the one serving when the unit starts. - A write that fails refuses the launch (errBuildStateUnwritten). Nothing starts, the hold is released, and the run ends as the existing start-failed result. No new copy. - After the write it checks again that the instance is awake and owned, that the generation is unchanged, and that the run is not cancelled or past its deadline. One bound, cmdStartBound, covers the write and the start. Tests: successor B records v2 and fails its health check; a command then records v1 before its unit starts, so the build state names the serving build. A failed write launches no command and leaves no hold. A stop or the run's own cancel during the write starts nothing. With the record call removed, the first test reads "the build state names v2".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+181
−2
internal/activator/cmd.go+42 −1internal/activator/cmd_swap_test.go+139 −1
CI passed — run details
CI log
=== gate ===
cd launchctx && go vet ./... && go test ./... -count=1
ok amadan.net/carlos/platform.git/launchctx 0.011s
? amadan.net/carlos/platform.git/launchctx/controlname [no test files]
? amadan.net/carlos/platform.git/launchctx/wireid [no test files]
go vet ./...
gofmt -l .: clean
go test ./... -count=1 -timeout 30m
ok github.com/carlosframework/platform 349.567s
? github.com/carlosframework/platform/cmd/changelog [no test files]
ok github.com/carlosframework/platform/cmd/console 3.081s
ok github.com/carlosframework/platform/cmd/moveprobe 92.211s
ok github.com/carlosframework/platform/internal/activator 237.414s
ok github.com/carlosframework/platform/internal/agentsock 0.018s
ok github.com/carlosframework/platform/internal/alerting 0.005s
ok github.com/carlosframework/platform/internal/analytics 0.020s
ok github.com/carlosframework/platform/internal/appconfig 2.378s
ok github.com/carlosframework/platform/internal/apperr 0.071s
ok github.com/carlosframework/platform/internal/applog 0.128s
ok github.com/carlosframework/platform/internal/awsinv 0.004s
ok github.com/carlosframework/platform/internal/boxchoice 0.006s
ok github.com/carlosframework/platform/internal/boxid 0.922s
? github.com/carlosframework/platform/internal/boxid/boxidtest [no test files]
? github.com/carlosframework/platform/internal/boxid/internal/signtest [no test files]
ok github.com/carlosframework/platform/internal/boxvolumes 0.009s
ok github.com/carlosframework/platform/internal/caps 0.012s
ok github.com/carlosframework/platform/internal/certwarm 0.002s
ok github.com/carlosframework/platform/internal/cgroup 0.200s
ok github.com/carlosframework/platform/internal/changelog 0.290s
ok github.com/carlosframework/platform/internal/clamd 0.055s
? github.com/carlosframework/platform/internal/cliflag [no test files]
ok github.com/carlosframework/platform/internal/console 372.774s
? github.com/carlosframework/platform/internal/controlname [no test files]
? github.com/carlosframework/platform/internal/controlsock [no test files]
ok github.com/carlosframework/platform/internal/dbdoor 5.967s
ok github.com/carlosframework/platform/internal/dbdoor/dbdoortest 0.009s
ok github.com/carlosframework/platform/internal/dbverify 0.004s
ok github.com/carlosframework/platform/internal/dnsrec 0.003s
ok github.com/carlosframework/platform/internal/docsite 0.005s
ok github.com/carlosframework/platform/internal/domains 2.134s
ok github.com/carlosframework/platform/internal/doorkey 0.006s
ok github.com/carlosframework/platform/internal/economics 0.004s
ok github.com/carlosframework/platform/internal/edge 26.121s
? github.com/carlosframework/platform/internal/edgeproof [no test files]
ok github.com/carlosframework/platform/internal/edgesfile 0.008s
ok github.com/carlosframework/platform/internal/fleetapi 0.123s
ok github.com/carlosframework/platform/internal/gate 1.656s
ok github.com/carlosframework/platform/internal/genselect 0.015s
ok github.com/carlosframework/platform/internal/geo 0.018s
ok github.com/carlosframework/platform/internal/geoip 0.030s
ok github.com/carlosframework/platform/internal/host 82.326s
ok github.com/carlosframework/platform/internal/hostactors 0.384s
ok github.com/carlosframework/platform/internal/hostbox 1.801s
ok github.com/carlosframework/platform/internal/hostclaim 0.500s
ok github.com/carlosframework/platform/internal/hostevidence 0.016s
ok github.com/carlosframework/platform/internal/hostexec 0.302s
ok github.com/carlosframework/platform/internal/hostfence 0.029s
ok github.com/carlosframework/platform/internal/hostfiles 10.483s
ok github.com/carlosframework/platform/internal/hostfloor 0.016s
ok github.com/carlosframework/platform/internal/hostguard 0.136s
ok github.com/carlosframework/platform/internal/hostops 1.402s
? github.com/carlosframework/platform/internal/hostops/fencerelease [no test files]
ok github.com/carlosframework/platform/internal/hostops/internal/fencerelease 0.016s
ok github.com/carlosframework/platform/internal/hostquarantine 0.017s
ok github.com/carlosframework/platform/internal/hostrelocate 0.441s
ok github.com/carlosframework/platform/internal/hostresolve 0.140s
ok github.com/carlosframework/platform/internal/hostwrite 0.017s
ok github.com/carlosframework/platform/internal/infraview 0.016s
ok github.com/carlosframework/platform/internal/instancectl 2.402s
ok github.com/carlosframework/platform/internal/instanceexec 2.722s
ok github.com/carlosframework/platform/internal/instancefs 0.020s
ok github.com/carlosframework/platform/internal/instancekey 0.019s
ok github.com/carlosframework/platform/internal/instanceupdates 0.023s
ok github.com/carlosframework/platform/internal/layout 1.419s
ok github.com/carlosframework/platform/internal/lease 9.450s
ok github.com/carlosframework/platform/internal/lease/leasetest 0.005s
ok github.com/carlosframework/platform/internal/ledger 0.014s
ok github.com/carlosframework/platform/internal/mail 0.163s
ok github.com/carlosframework/platform/internal/maintenance 0.078s
ok github.com/carlosframework/platform/internal/maintenanceexec 5.516s
ok github.com/carlosframework/platform/internal/maintenanceguard 0.075s
ok github.com/carlosframework/platform/internal/metrics 0.005s
ok github.com/carlosframework/platform/internal/moves 20.966s
ok github.com/carlosframework/platform/internal/moves/archive 0.778s
ok github.com/carlosframework/platform/internal/objstore 0.164s
ok github.com/carlosframework/platform/internal/ops 0.162s
ok github.com/carlosframework/platform/internal/park 0.198s
ok github.com/carlosframework/platform/internal/privatefile 0.004s
ok github.com/carlosframework/platform/internal/provision 3.341s
ok github.com/carlosframework/platform/internal/publicrelease 0.006s
? github.com/carlosframework/platform/internal/regionname [no test files]
ok github.com/carlosframework/platform/internal/registry 4.013s
ok github.com/carlosframework/platform/internal/relay 7.229s
ok github.com/carlosframework/platform/internal/release 19.253s
ok github.com/carlosframework/platform/internal/rollout 3.592s
ok github.com/carlosframework/platform/internal/rolloutwords 0.004s
ok github.com/carlosframework/platform/internal/rootaws 0.007s
ok github.com/carlosframework/platform/internal/runs 0.563s
ok github.com/carlosframework/platform/internal/s3compat 0.005s
ok github.com/carlosframework/platform/internal/safefile 0.004s
ok github.com/carlosframework/platform/internal/schedule 2.270s
ok github.com/carlosframework/platform/internal/sim/hostlife 8.335s
ok github.com/carlosframework/platform/internal/sim/litestreamfacts 4.149s
ok github.com/carlosframework/platform/internal/status 0.041s
ok github.com/carlosframework/platform/internal/steering 0.006s
ok github.com/carlosframework/platform/internal/stopevidence 0.162s
ok github.com/carlosframework/platform/internal/svcrole 0.002s
ok github.com/carlosframework/platform/internal/telemetry 0.004s
ok github.com/carlosframework/platform/internal/termfence 0.004s
ok github.com/carlosframework/platform/internal/tunnelclient 0.004s
ok github.com/carlosframework/platform/internal/unixdial 0.002s
ok github.com/carlosframework/platform/internal/updatewords 0.004s
ok github.com/carlosframework/platform/internal/usage 0.004s
ok github.com/carlosframework/platform/internal/version 0.001s
ok github.com/carlosframework/platform/internal/vet 1.576s
=== cross ===
--- shard 0 of 5 ---
cross-compiling darwin/arm64
--- shard 1 of 5 ---
cross-compiling darwin/amd64
--- shard 2 of 5 ---
cross-compiling linux/arm64
--- shard 3 of 5 ---
cross-compiling windows/amd64
--- shard 4 of 5 ---
cross-compiling windows/arm64
=== minio-contract ===
minio contract: arm64, minio RELEASE.2025-09-07T16-13-09Z, mc RELEASE.2025-08-13T08-35-41Z
minio: downloading https://github.com/minio/minio/releases/download/RELEASE.2025-09-07T16-13-09Z/minio.linux-arm64.RELEASE.2025-09-07T16-13-09Z
minio: downloaded, digest verified
mc: downloading https://github.com/minio/mc/releases/download/RELEASE.2025-08-13T08-35-41Z/mc.linux-arm64.RELEASE.2025-08-13T08-35-41Z
mc: downloaded, digest verified
minio healthy on 127.0.0.1:20999
Added `local` successfully.
Bucket created successfully `local/contract-bucket`.
Bucket created successfully `local/denied-bucket`.
Created policy `contract-only` successfully.
Added user `scopeduser` successfully.
Attached Policies: [contract-only]
To User: scopeduser
buckets and scoped user ready
=== RUN TestDeliveredContractMinio
=== RUN TestDeliveredContractMinio/put
=== RUN TestDeliveredContractMinio/get_round-trips_the_body
=== RUN TestDeliveredContractMinio/list_shows_the_key
=== RUN TestDeliveredContractMinio/head_and_list_agree_on_the_version
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed
=== RUN TestDeliveredContractMinio/delete_removes_it
=== RUN TestDeliveredContractMinio/cross-bucket_denial
--- PASS: TestDeliveredContractMinio (0.03s)
--- PASS: TestDeliveredContractMinio/put (0.00s)
--- PASS: TestDeliveredContractMinio/get_round-trips_the_body (0.00s)
--- PASS: TestDeliveredContractMinio/list_shows_the_key (0.00s)
--- PASS: TestDeliveredContractMinio/head_and_list_agree_on_the_version (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route (0.02s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/delete_removes_it (0.00s)
--- PASS: TestDeliveredContractMinio/cross-bucket_denial (0.00s)
PASS
ok github.com/carlosframework/platform/internal/objstore 0.031s
=== RUN TestT4StorageProbeMinio
2026/10/13 15:24:33 moves: T4: STORE: list: operation error S3: ListObjectsV2, https response error StatusCode: 403, RequestID: 18DE202BAEF44DF6, HostID: 7c420053e0fbcd7f41653fdd92ffa2802c903d05303b1aa15adc2e8bb65d183f, api error SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided. Check your key and signing method.
--- PASS: TestT4StorageProbeMinio (0.01s)
PASS
ok github.com/carlosframework/platform/internal/host 0.027s
minio contract: PASS
diff --git a/internal/activator/cmd.go b/internal/activator/cmd.go| index b81107160..4027fa0fd 100644 |
| --- a/internal/activator/cmd.go |
| +++ b/internal/activator/cmd.go |
| @@ -613,9 +613,50 @@ func (a *Service) RunCmd(ctx context.Context, rt registry.Route, spec CmdSpec) ( |
| spec.genKeyLayer = lp.LaunchKeyLayer() |
| } |
| a.mu.Unlock() |
| + // One bound covers the build-state write and the start, so a stop |
| + // waiting on the start still has the settle's budget after it |
| + // (cmdStartBound). |
| sctx, scancel := context.WithTimeout(ctx, cmdStartBound) |
| + defer scancel() |
| + // The build about to touch the instance's database is recorded first |
| + // (settings spec §4.2), as the sidecar's is (startSidecar): a command |
| + // runs its serving generation's build against the data, and the build |
| + // state must name that build before it does. A failed successor may |
| + // have left the record naming its own build; without this write a |
| + // command on the incumbent would run under a record that names |
| + // another, and a held recovery could pick the wrong one. A write that |
| + // fails refuses the launch (errBuildStateUnwritten): nothing starts. |
| + // |
| + // The starting hold is kept throughout. A stop that begins meanwhile |
| + // waits for this start to return, and no overlap swap is admitted |
| + // (cmdStartInFlightLocked), so the generation recorded is still the |
| + // one serving when the unit starts. The write takes time, so |
| + // ownership and the run's own cancel and deadline are checked again |
| + // after it, before anything starts. |
| + if err := a.recordBuild(sctx, rt.Host, h.gen.Build); err != nil { |
| + return abandon(err) |
| + } |
| + a.mu.Lock() |
| + var refused error |
| + switch { |
| + case e.state != actAwake: |
| + refused = &CmdStoppedError{Reason: e.stopWhy} |
| + case ctx.Err() != nil: |
| + refused = ctx.Err() |
| + case e.gen.N != h.gen.N: |
| + // Not reached while the starting hold keeps swaps out; a |
| + // generation that changed anyway is never started against. |
| + refused = errSwapBusy |
| + default: |
| + if why := cmdOwnershipLocked(a, e); why != "" { |
| + refused = &CmdStoppedError{Reason: why} |
| + } |
| + } |
| + a.mu.Unlock() |
| + if refused != nil { |
| + return abandon(refused) |
| + } |
| u, err := cb.StartCmd(sctx, rt, spec) |
| - scancel() |
| h.unit = u |
| close(h.started) |
| if err != nil { |
diff --git a/internal/activator/cmd_swap_test.go b/internal/activator/cmd_swap_test.go| index 39277a56a..d957f3b6a 100644 |
| --- a/internal/activator/cmd_swap_test.go |
| +++ b/internal/activator/cmd_swap_test.go |
| @@ -22,16 +22,20 @@ type cmdSwapBackend struct { |
| specs []CmdSpec |
| gate chan struct{} // when set, StartCmd blocks until it closes |
| entered chan struct{} // when set, closed as StartCmd is entered |
| + onStart func() // when set, runs inside StartCmd, before the unit is handed back |
| } |
| func (b *cmdSwapBackend) StartCmd(_ context.Context, _ registry.Route, spec CmdSpec) (CmdUnit, error) { |
| b.cmu.Lock() |
| - gate, entered := b.gate, b.entered |
| + gate, entered, onStart := b.gate, b.entered, b.onStart |
| b.entered = nil |
| b.cmu.Unlock() |
| if entered != nil { |
| close(entered) |
| } |
| + if onStart != nil { |
| + onStart() |
| + } |
| if gate != nil { |
| <-gate |
| } |
| @@ -407,3 +411,137 @@ func readPinPaths(t *testing.T, path, host string) []string { |
| time.Sleep(10 * time.Millisecond) |
| } |
| } |
| + |
| +// buildRecorder is a Service.BeforeExec that keeps every record in order: |
| +// its last entry for a host is that host's last_executed. |
| +type buildRecorder struct { |
| + mu sync.Mutex |
| + builds []string |
| + err error |
| + hook func() // runs inside the write, when set |
| +} |
| + |
| +func (r *buildRecorder) record(_ context.Context, _, build string) error { |
| + r.mu.Lock() |
| + hook, err := r.hook, r.err |
| + if err == nil { |
| + r.builds = append(r.builds, build) |
| + } |
| + r.mu.Unlock() |
| + if hook != nil { |
| + hook() |
| + } |
| + return err |
| +} |
| + |
| +func (r *buildRecorder) last() string { |
| + r.mu.Lock() |
| + defer r.mu.Unlock() |
| + if len(r.builds) == 0 { |
| + return "" |
| + } |
| + return r.builds[len(r.builds)-1] |
| +} |
| + |
| +// A successor B records its build and then fails its health check, so the |
| +// incumbent A goes on serving under a build state that names B. A command |
| +// then runs A: its build is recorded before its unit starts, so the build |
| +// state names A again, as it does for the sidecar (settings spec §4.2; |
| +// second merge review, finding 1). |
| +func TestARunRecordsItsServingBuildBeforeItStarts(t *testing.T) { |
| + f, cb := newCmdSwapFixture(t) |
| + rec := &buildRecorder{} |
| + f.a.BeforeExec = rec.record |
| + f.a.genHealth = func(context.Context, Gen) error { return errors.New("502 from the successor") } |
| + if res := f.a.Swap(context.Background(), f.attempt(ModeOverlap), f.newest); res.Outcome != SwapFailed || !res.Executed { |
| + t.Fatalf("Swap = %+v, want the successor to have run and failed", res) |
| + } |
| + if got := rec.last(); got != "v2" { |
| + t.Fatalf("after the failed successor the build state names %q, want its v2", got) |
| + } |
| + if cur, _ := f.a.genStateForTest(f.host); cur.N != f.incumbent.N { |
| + t.Fatalf("generation %d is serving, want the incumbent", cur.N) |
| + } |
| + // What the build state names at the moment the unit starts. |
| + var atStart string |
| + cb.cmu.Lock() |
| + cb.onStart = func() { atStart = rec.last() } |
| + cb.cmu.Unlock() |
| + h, err := f.a.RunCmd(context.Background(), testRoute(f.host), CmdSpec{RunID: "run_a"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h.Release() |
| + if got := rec.last(); got != f.incumbent.Build { |
| + t.Fatalf("after the command the build state names %q, want the serving %q", got, f.incumbent.Build) |
| + } |
| + if specs := cb.started(); len(specs) != 1 || specs[0].serving == nil || specs[0].serving.Path != f.incumbent.StagedPath { |
| + t.Fatalf("the command started %+v, want one unit on the incumbent's build", specs) |
| + } |
| + if atStart != f.incumbent.Build { |
| + t.Fatalf("the unit started with the build state naming %q: the record must come first", atStart) |
| + } |
| +} |
| + |
| +// A build state that cannot be written launches no command (settings spec |
| +// §4.2): the run is refused, the hold is released, and the instance and its |
| +// next run are unaffected. A stop, or the run's own cancel, that lands |
| +// during the write starts nothing either. |
| +func TestARunWhoseBuildRecordFailsOrIsOvertakenNeverStarts(t *testing.T) { |
| + t.Run("the write fails", func(t *testing.T) { |
| + f, cb := newCmdSwapFixture(t) |
| + rec := &buildRecorder{err: errors.New("bucket: 503")} |
| + f.a.BeforeExec = rec.record |
| + if _, err := f.a.RunCmd(context.Background(), testRoute(f.host), CmdSpec{RunID: "run_a"}); !errors.Is(err, errBuildStateUnwritten) { |
| + t.Fatalf("RunCmd = %v, want errBuildStateUnwritten", err) |
| + } |
| + if n := len(cb.started()); n != 0 { |
| + t.Fatalf("%d units started over an unwritten build state", n) |
| + } |
| + rec.mu.Lock() |
| + rec.err = nil |
| + rec.mu.Unlock() |
| + h, err := f.a.RunCmd(context.Background(), testRoute(f.host), CmdSpec{RunID: "run_b"}) |
| + if err != nil { |
| + t.Fatalf("the next run: %v (the refused one must leave no hold)", err) |
| + } |
| + h.Release() |
| + }) |
| + t.Run("a stop lands during the write", func(t *testing.T) { |
| + f, cb := newCmdSwapFixture(t) |
| + rec := &buildRecorder{} |
| + rec.hook = func() { f.a.Bounce(f.host) } |
| + f.a.BeforeExec = rec.record |
| + _, err := f.a.RunCmd(context.Background(), testRoute(f.host), CmdSpec{RunID: "run_a"}) |
| + var se *CmdStoppedError |
| + if !errors.As(err, &se) || se.Reason != StopRestart { |
| + t.Fatalf("RunCmd = %v, want CmdStoppedError{%q}", err, StopRestart) |
| + } |
| + if n := len(cb.started()); n != 0 { |
| + t.Fatalf("%d units started under a stopping instance", n) |
| + } |
| + rec.mu.Lock() |
| + rec.hook = nil |
| + rec.mu.Unlock() |
| + waitFor(t, "the instance stopped", func() bool { return f.a.entryCount() == 0 }) |
| + }) |
| + t.Run("the run is cancelled during the write", func(t *testing.T) { |
| + f, cb := newCmdSwapFixture(t) |
| + ctx, cancel := context.WithCancel(context.Background()) |
| + defer cancel() |
| + rec := &buildRecorder{hook: cancel} |
| + f.a.BeforeExec = rec.record |
| + if _, err := f.a.RunCmd(ctx, testRoute(f.host), CmdSpec{RunID: "run_a"}); !errors.Is(err, context.Canceled) { |
| + t.Fatalf("RunCmd = %v, want context.Canceled", err) |
| + } |
| + if n := len(cb.started()); n != 0 { |
| + t.Fatalf("%d units started for a cancelled run", n) |
| + } |
| + f.a.mu.Lock() |
| + attached := f.a.entries[f.host].cmd != nil |
| + f.a.mu.Unlock() |
| + if attached { |
| + t.Fatal("the cancelled run is still attached") |
| + } |
| + }) |
| +} |