amadan
console: restore announces itself, and a bump outlives its request
Two review findings on the serial, the deploy signal's other half. Restore was the one box-consumed door that wrote and said nothing. Pulling an app's trash marker re-enables instance minting, and minting from records is leg 1 of the fast pass's reconcile — so a restore without a bump leaves the app un-mintable until the hourly floor, the exact wait the delete side already announces its way out of. And bumpSerial inherited the REQUEST's context. By the time a bump runs, the write it announces has landed and cannot be taken back, so a client that hangs up in that window — a killed CI runner, a closed laptop, a proxy timeout — silently cancelled the announcement for a change that is permanently in the bucket; the fleet then waits out the hour for a deploy that already happened. context.WithoutCancel detaches it. The 2s clamp is unchanged: detached from cancellation, not time. Both are pinned. The cancellation test needs a store that HONOURS ctx (cancelAwareStore) — the DirStore is local disk and ignores it, so the test would otherwise have passed for the wrong reason. Verified by reverting each fix in turn: restore = 0 want 1, canceled bump = 0 want 1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 files changed,
+75
−1
internal/console/ownership.go+6 −0internal/console/serialbump.go+11 −1internal/console/serialbump_test.go+58 −0
diff --git a/internal/console/ownership.go b/internal/console/ownership.go| index e7ab12b33..6e6055b69 100644 |
| --- a/internal/console/ownership.go |
| +++ b/internal/console/ownership.go |
| @@ -243,6 +243,12 @@ func (s *Server) handleRestoreApp(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| s.audit(acct, app, addr, "restore", "restored "+app+" from the trash") |
| + // Box-consumed (carlos-deploy §3): removing the trash marker re-enables |
| + // instance minting for this app, and minting from records is leg 1 of the |
| + // fast pass's reconcile. Without the bump a restore is invisible to the |
| + // fleet until the hourly floor comes round — the same wait the delete side |
| + // already announces its way out of. |
| + s.bumpSerial(r.Context()) |
| // The anti-ghost rule, said where the person can read it: restoring brings |
| // back releases and channels and no routes at all (spec §3), so an app |
| // restored and left alone serves nothing and nobody can tell why. |
diff --git a/internal/console/serialbump.go b/internal/console/serialbump.go| index 9f3facea1..c6be80555 100644 |
| --- a/internal/console/serialbump.go |
| +++ b/internal/console/serialbump.go |
| @@ -18,11 +18,21 @@ import ( |
| // serial carries no authority at all (it is the one unsigned object in the |
| // bucket); the worst a spurious bump can do is make a box run its ordinary |
| // signature-verified pass set a little early. |
| +// |
| +// context.WithoutCancel is the whole point of the ctx handling here: the |
| +// caller's context is the REQUEST's, and by the time a bump runs the write it |
| +// announces has already landed and is irreversible. A client that hangs up in |
| +// that window (a CI runner killed mid-promote, a closed laptop, a proxy |
| +// timeout) would otherwise cancel the announcement for a change that is |
| +// permanently in the bucket — the fleet then waits out the hourly floor for a |
| +// deploy that already happened, with nothing in any log to say why. The 2s |
| +// clamp still applies, measured from here: detached from cancellation, not |
| +// from time. |
| func (s *Server) bumpSerial(ctx context.Context) { |
| if s.cfg.Releases == nil { |
| return // no bucket on this console: nothing to announce into |
| } |
| - ctx, cancel := context.WithTimeout(ctx, 2*time.Second) |
| + ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Second) |
| defer cancel() |
| if err := release.BumpSerial(ctx, s.cfg.Releases, time.Now()); err != nil { |
| log.Printf("console: serial bump: %v (boxes fall back to the hourly pass)", err) |
diff --git a/internal/console/serialbump_test.go b/internal/console/serialbump_test.go| index e05c4272c..bbfe715fb 100644 |
| --- a/internal/console/serialbump_test.go |
| +++ b/internal/console/serialbump_test.go |
| @@ -365,6 +365,64 @@ func TestWebProductionBumpsSerial(t *testing.T) { |
| } |
| } |
| +// TestWebRestoreBumpsSerial: restore is a box-consumed write like every other |
| +// door here — pulling the trash marker re-enables instance minting for the |
| +// app, and minting from records is leg 1 of the fast pass's reconcile. An |
| +// un-announced restore leaves the app un-mintable until the hourly floor, |
| +// which is exactly the wait the delete side already announces its way out of. |
| +func TestWebRestoreBumpsSerial(t *testing.T) { |
| + fx, c, csrf := trashPageFixture(t) |
| + |
| + before := serial(t, fx) |
| + status, _, body := postForm(t, c, fx.u("/trash/restore"), |
| + url.Values{"csrf": {csrf}, "app": {"kass"}}) |
| + if status != http.StatusSeeOther { |
| + t.Fatalf("restore = %d, want 303:\n%s", status, body) |
| + } |
| + if got := serial(t, fx); got != before+1 { |
| + t.Fatalf("serial after a restore = %d, want %d", got, before+1) |
| + } |
| +} |
| + |
| +// cancelAwareStore is a bucket that honours context cancellation, the way the |
| +// AWS SDK does and the DirStore (local disk, no ctx checks at all) does not — |
| +// without it, a canceled-context test here would pass for the wrong reason. |
| +type cancelAwareStore struct{ release.Store } |
| + |
| +func (c cancelAwareStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + if err := ctx.Err(); err != nil { |
| + return nil, "", err |
| + } |
| + return c.Store.Get(ctx, key) |
| +} |
| + |
| +func (c cancelAwareStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) { |
| + if err := ctx.Err(); err != nil { |
| + return "", err |
| + } |
| + return c.Store.Put(ctx, key, body, ifMatch) |
| +} |
| + |
| +// TestBumpSerialOutlivesTheRequest: the announcement is detached from the |
| +// request context (context.WithoutCancel). By the time a bump runs, the write |
| +// it announces has already landed and cannot be taken back — so a client that |
| +// hangs up in that window (a killed CI runner, a closed laptop, a proxy |
| +// timeout) must not cancel the announcement for a change that is permanently |
| +// in the bucket. Inheriting the cancellation loses it silently: the fleet |
| +// waits out the hourly floor for a deploy that already happened. |
| +func TestBumpSerialOutlivesTheRequest(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + fx.srv.cfg.Releases = cancelAwareStore{fx.rel} |
| + |
| + ctx, cancel := context.WithCancel(context.Background()) |
| + cancel() // the client is already gone |
| + fx.srv.bumpSerial(ctx) |
| + |
| + if got := serial(t, fx); got != 1 { |
| + t.Fatalf("serial after a bump on a canceled request context = %d, want 1", got) |
| + } |
| +} |
| + |
| // TestBumpSerialSurvivesABrokenStore: the whole contract in one test — a bump |
| // against a store that cannot be written returns normally and logs. Every call |
| // site depends on this: a user-facing write must never fail, or stall, because |