amadan
console: ship budget is 2 a minute, and a refusal says how long (#262)
Agents iterating ship -> verify -> ship were wedging on the ship budget. The number was 10 per token, but the shape was the problem: a fixed-window counter over an HOUR, so spending the ten in five minutes killed the token for the remaining fifty-five, answered "too many uploads — wait a while and try again". A script cannot tell that from an outage, so what it reported was that shipping had stopped working. cliShipWindow becomes a minute at cliShipBurst 2 (svcShipBurst 10, keeping its multiple over the person budget). Three consequences, all wanted: - the hourly ceiling rises 10 -> 120 per person, 60 -> 600 per service. What bounds the storage those buy is the retention sweep, not this counter; the counter's job is to make a stolen token useless for flooding, which 2-a-minute still does. - the INSTANTANEOUS rate gets tighter, not looser: the old budget let ten uploads go back to back, this one lets two. - the worst wait a refused caller can be asked for is 60 seconds — and it is now told the number, in Retry-After and in the message the CLI already prints verbatim (apiErrorFromBody). The semaphore's "another ship is in progress" gets a Retry-After too, for the same reason. rateLimiter.resetIn is the read-only half that reports it; allowShip returns the wait alongside ok so the number can only come from the limiter that actually refused. resetIn deliberately does not sweep — allow just did, and a second walk under that mutex is the stall allow's own comment avoids. Tests: liftShipBudget takes the limiter out of the way of the nine ship tests that are about something else (immutability, minting, the capability vocabulary) — and of the concurrency test, where the limiter sits IN FRONT of the semaphore and would otherwise have made its 429 the wrong 429. The budget itself stays pinned by TestPersonShipBudgetStaysKeyedByToken, TestServiceShipBudgetIsOwnAndKeyedByName, and the new TestShipRefusalNamesTheWait, which asserts Retry-After against cliShipWindow so a move back to an hour fails a test. The CLI acceptance test logs in a second terminal for its third ship, which is what ship's token-keying is for.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
7 files changed,
+249
−35
auth_e2e_test.go+15 −0docs/superpowers/specs/2026-08-03-cli-console-api-design.md+27 −4internal/console/apicli.go+24 −8internal/console/apicli_test.go+44 −5internal/console/authcli.go+76 −15internal/console/authcli_test.go+49 −0internal/console/svcprincipal.go+14 −3
diff --git a/auth_e2e_test.go b/auth_e2e_test.go| index 01e79e472..31565511c 100644 |
| --- a/auth_e2e_test.go |
| +++ b/auth_e2e_test.go |
| @@ -566,6 +566,21 @@ func TestE2ECLIThroughConsole(t *testing.T) { |
| } |
| // --- a second ship+promote, then roll back ------------------------------ |
| + // |
| + // A SECOND terminal, because this is the test's third ship and the |
| + // console's ship budget is cliShipBurst — 2 per token per minute |
| + // (internal/console/authcli.go). A third upload on the same token is a |
| + // 429, and runShip log.Fatals on it, which takes the whole package down |
| + // with no named failure. Logging in again is exactly what a person with |
| + // two machines does, and ship's budget is deliberately keyed on the |
| + // TOKEN rather than the address so that it works: see |
| + // TestPersonShipBudgetStaysKeyedByToken. |
| + token2, expires2 := e2eDeviceLogin(t, ts.URL, cst, addr, "e2e@second-terminal") |
| + credFile.put(credential{Console: ts.URL, Address: addr, Token: token2, ExpiresAt: expires2}) |
| + if err := credFile.save(); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| payload2 := bytes.Repeat([]byte{0x43}, 1024) |
| bin2 := filepath.Join(dir, "carlos-def5678") |
| if err := os.WriteFile(bin2, payload2, 0o755); err != nil { |
diff --git a/docs/superpowers/specs/2026-08-03-cli-console-api-design.md b/docs/superpowers/specs/2026-08-03-cli-console-api-design.md| index 562649f35..2ebf00338 100644 |
| --- a/docs/superpowers/specs/2026-08-03-cli-console-api-design.md |
| +++ b/docs/superpowers/specs/2026-08-03-cli-console-api-design.md |
| @@ -101,10 +101,33 @@ refusals (bad channel, bad version, unknown release), then the rung gate: |
| different secret — account membership — which the 404 above already |
| covers.) |
| -Rate limits: reuse the `cliLimits` family. Uploads get their own bucket-of- |
| -tokens (small: ships are rare) keyed by token hash, not IP — a shared NAT |
| -must not let one developer starve another, and a token is the natural |
| -principal here. |
| +Rate limits: reuse the `cliLimits` family. Uploads get their own budget keyed |
| +by token hash, not IP — a shared NAT must not let one developer starve |
| +another, and a token is the natural principal here. |
| + |
| +**Amended 2026-08-23 — ships are not rare.** The original budget was 10 an |
| +hour per token, on the premise above that "ships are rare". That premise held |
| +for a person cutting a release by hand and does not hold for the caller doing |
| +most of the shipping now: an agent iterating ship → verify → ship. Worse, a |
| +fixed-window counter turns the hour into the refusal itself — spend the ten in |
| +five minutes and the token is dead for the remaining fifty-five, answered |
| +"wait a while", which a script cannot distinguish from an outage. |
| + |
| +The window is now a MINUTE (`cliShipWindow`), at 2 per token |
| +(`cliShipBurst`) and 10 per service principal (`svcShipBurst`). Three |
| +consequences, all deliberate: |
| + |
| +- The hourly ceiling rises 10 → 120 (person) and 60 → 600 (service). What |
| + bounds the storage those buy is the release retention sweep, not this |
| + counter — the counter's job is to make a stolen token useless for |
| + flooding, and 2-a-minute does that. |
| +- The instantaneous rate gets TIGHTER, not looser: the old budget permitted |
| + ten uploads back to back; this one permits two. |
| +- The worst wait a refused caller can be asked for is 60 seconds, and it is |
| + told the number — `Retry-After`, plus the same figure in the message body, |
| + which the CLI already surfaces verbatim (`apiErrorFromBody`). A refusal a |
| + caller can sleep on is the point of the change; the raised ceiling is the |
| + smaller half of it. |
| ## §4 Upload handling |
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index e1eb461fe..f937912e1 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -1413,15 +1413,26 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| } |
| p, _ := principalFrom(r.Context()) |
| // allowShip picks the budget: a person stays keyed on the TOKEN, not the |
| - // address — ships are rare and heavy (bytes on the wire, an object |
| - // written per artifact), so a token is the natural principal for an |
| - // upload, unlike a write budget. Keying by address instead would make a |
| - // laptop and a CI runner sharing one human's identity share one |
| - // shipping budget between them, which is the limit doing less than it |
| - // should, not more. A service gets its own budget, keyed on its NAME |
| + // address — a ship is heavy (bytes on the wire, an object written per |
| + // artifact), so a token is the natural principal for an upload, unlike |
| + // a write budget. (The other half of that reasoning, "ships are rare", |
| + // no longer holds and is no longer load-bearing here: see |
| + // cliShipWindow.) Keying by address instead would make a laptop and a |
| + // CI runner sharing one human's identity share one shipping budget |
| + // between them, which is the limit doing less than it should, not |
| + // more. A service gets its own budget, keyed on its NAME |
| // (svcprincipal.go), separate from the operator who minted it. |
| - if !s.allowShip(p, r, time.Now()) { |
| - jsonError(w, http.StatusTooManyRequests, "too many uploads — wait a while and try again") |
| + // |
| + // The refusal names the wait and stamps Retry-After. "Wait a while", |
| + // which is what this said while the window was an hour, is the answer |
| + // that wedges the caller who meets this budget most: an agent shipping |
| + // in a loop cannot tell a two-second pause from an hour-long lockout, |
| + // so it reports that shipping is broken. cliShipWindow's move to a |
| + // minute is what makes the number worth printing at all. |
| + if ok, wait := s.allowShip(p, r, time.Now()); !ok { |
| + retryAfter(w, wait) |
| + jsonError(w, http.StatusTooManyRequests, fmt.Sprintf( |
| + "too many uploads — try again in %ds", max(int(wait/time.Second), 1))) |
| return |
| } |
| // The rate limiter bounds requests over TIME; it says nothing about how |
| @@ -1435,6 +1446,11 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| case s.shipSem <- struct{}{}: |
| defer func() { <-s.shipSem }() |
| default: |
| + // Seconds, not the budget window: this refusal clears as soon as an |
| + // upload in flight finishes, which is a different and much shorter |
| + // wait than the rate limiter's. Stamped for the same reason — |
| + // "shortly" is not a number a script can sleep on. |
| + retryAfter(w, 2*time.Second) |
| jsonError(w, http.StatusTooManyRequests, "another ship is in progress — try again shortly") |
| return |
| } |
diff --git a/internal/console/apicli_test.go b/internal/console/apicli_test.go| index 18a1fe3b2..3965900c0 100644 |
| --- a/internal/console/apicli_test.go |
| +++ b/internal/console/apicli_test.go |
| @@ -926,12 +926,41 @@ func randomBytes(t *testing.T, n int) []byte { |
| return b |
| } |
| +// liftShipBudget takes the ship rate limiter out of a test's way. |
| +// |
| +// cliShipBurst is 2 a minute (authcli.go) — deliberately small, because the |
| +// caller it bounds is a release upload. A test that ships three or more |
| +// times is almost always testing something else entirely (immutability, |
| +// minting, the capability vocabulary, the concurrency semaphore) and would |
| +// otherwise be refused 429 partway through by a gate it is not about. Worse |
| +// for the semaphore test: the rate limiter sits IN FRONT of the semaphore, |
| +// so without this the 429 it asserts on would be the wrong 429 and the test |
| +// would prove nothing. |
| +// |
| +// The budget itself is pinned elsewhere and stays pinned: |
| +// TestPersonShipBudgetStaysKeyedByToken and |
| +// TestServiceShipBudgetIsOwnAndKeyedByName (authcli_test.go) run against the |
| +// real constants and must never call this. |
| +func liftShipBudget(fx *consoleFixture) { |
| + fx.srv.cli.ship = newRateLimiter(1<<20, time.Minute) |
| + fx.srv.cli.svcShip = newRateLimiter(1<<20, time.Minute) |
| +} |
| + |
| // postShip POSTs a multipart/form-data ship request built from parts, in |
| // the order given, and decodes whatever JSON comes back (a refusal that |
| // never reaches the handler's own JSON responses — e.g. a body cut short by |
| // MaxBytesReader — decodes to an empty map, which the caller only inspects |
| // via the status code). |
| func postShip(t *testing.T, u, bearer string, parts []shipPart) (int, map[string]any) { |
| + t.Helper() |
| + status, _, out := postShipRaw(t, u, bearer, parts) |
| + return status, out |
| +} |
| + |
| +// postShipRaw is postShip with the response HEADERS kept — the refusal that |
| +// needs them is the 429, whose Retry-After is the whole point of the answer |
| +// for a non-human caller and is invisible in the JSON body. |
| +func postShipRaw(t *testing.T, u, bearer string, parts []shipPart) (int, http.Header, map[string]any) { |
| t.Helper() |
| var buf bytes.Buffer |
| mw := multipart.NewWriter(&buf) |
| @@ -966,7 +995,7 @@ func postShip(t *testing.T, u, bearer string, parts []shipPart) (int, map[string |
| } |
| out := map[string]any{} |
| _ = json.Unmarshal(raw, &out) |
| - return res.StatusCode, out |
| + return res.StatusCode, res.Header, out |
| } |
| // TestAPIShipHappy: a fresh binary release ships through the multipart |
| @@ -1074,6 +1103,7 @@ func TestAPIShipRefusals(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-refusals") |
| + liftShipBudget(fx) |
| payload := randomBytes(t, 128) |
| cases := []struct { |
| @@ -1208,6 +1238,7 @@ func TestAPIShipTraversalRefusals(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-traversal") |
| + liftShipBudget(fx) |
| payload := randomBytes(t, 64) |
| cases := []struct { |
| @@ -1251,6 +1282,7 @@ func TestAPIShipImmutableConflict(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-immutable") |
| + liftShipBudget(fx) |
| payload := randomBytes(t, 256) |
| status, out := postShip(t, fx.ts.URL+"/api/cli/apps/bab/titogo/releases", token, |
| @@ -1292,6 +1324,7 @@ func TestAPIShipConcurrencyLimit(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-concurrency") |
| + liftShipBudget(fx) |
| // A hook (same shape as cliNotified): signalled once a request has |
| // actually acquired shipSem, so this test waits on a real fact instead |
| @@ -1462,11 +1495,13 @@ func TestAPIShipRefusesABadCapabilityBlock(t *testing.T) { |
| {"db name with a separator", "cap10009", `{"v":1,"data":{"replicate":"platform","dbs":["a/b.db"]}}`, "a/b.db"}, |
| {"explicit null", "cap10010", `null`, "null"}, |
| } |
| - // One token, and the table is kept at or under cliShipBurst (10 ships per |
| - // token per hour) on purpose — an eleventh case would be answered 429 and |
| - // would pin nothing. The non-object shapes live in their own test below, |
| - // with their own token, for exactly that reason. |
| + // One token, with the ship budget lifted: this table is about the |
| + // capability vocabulary, and every case has to reach the decoder to pin |
| + // anything. It used to be capped at cliShipBurst cases instead, which |
| + // silently made the table's length a function of a rate limit that has |
| + // since moved. |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-badcaps") |
| + liftShipBudget(fx) |
| for _, c := range cases { |
| t.Run(c.name, func(t *testing.T) { |
| status, out := postShip(t, fx.ts.URL+"/api/cli/apps/bab/titogo/releases", token, |
| @@ -1505,6 +1540,7 @@ func TestAPIShipRefusesANonObjectCapabilityBlock(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-nonobject") |
| + liftShipBudget(fx) |
| payload := randomBytes(t, 128) |
| for _, c := range []struct{ name, version, block string }{ |
| @@ -1550,6 +1586,7 @@ func TestAPIShipRefusesARecapabilitiedReship(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-recaps") |
| + liftShipBudget(fx) |
| payload := randomBytes(t, 256) |
| u := fx.ts.URL + "/api/cli/apps/bab/keymail/releases" |
| @@ -1738,6 +1775,7 @@ func TestAPIShipMintsFromTarget(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-mint") |
| + liftShipBudget(fx) |
| // 1. No target set: an empty version is refused, naming the fix. |
| status, out := postShip(t, fx.ts.URL+"/api/cli/apps/bab/km/releases", token, |
| @@ -1795,6 +1833,7 @@ func TestAPIShipLabelLimits(t *testing.T) { |
| t.Fatal(err) |
| } |
| token := cliToken(t, fx, "paul@keymail.dev", "code-ship-labellimits") |
| + liftShipBudget(fx) |
| cases := []struct { |
| name string |
diff --git a/internal/console/authcli.go b/internal/console/authcli.go| index 7343e2964..e38a454c0 100644 |
| --- a/internal/console/authcli.go |
| +++ b/internal/console/authcli.go |
| @@ -67,17 +67,27 @@ const ( |
| // cliPollInterval is the wait the console asks the CLI to keep, seconds. |
| cliPollInterval = 2 |
| - // cliShipWindow is ship's own budget window — an hour, not the minute |
| - // every other CLI limiter uses: a ship is a deliberate release cut, not |
| - // a poll or a login guess, so the window matches how often a human (or |
| - // a CI job) actually does this. |
| - cliShipWindow = time.Hour |
| + // cliShipWindow is the minute window every other CLI limiter uses. It |
| + // was an HOUR until 2026-08-23, on the premise that "a ship is a |
| + // deliberate release cut, not a poll" — a premise a fixed-window |
| + // counter turns into the worst refusal on this surface. Spend the hour |
| + // budget in five minutes and the token is dead for the remaining |
| + // fifty-five, with nothing in the answer to say so; the caller that |
| + // actually does this is an agent iterating ship-test-ship, and what it |
| + // learns is that shipping stopped working. A minute window bounds the |
| + // same burst (see cliShipBurst) and bounds the WAIT too: the worst a |
| + // refused caller can be asked for is one minute, which is a sleep, not |
| + // an outage. |
| + cliShipWindow = time.Minute |
| // cliShipBurst bounds ship uploads per TOKEN (see handleAPIShip) within |
| - // that hour. Ships are rare: a build-and-ship is a deliberate act run a |
| - // handful of times a day at most, so ten in an hour is roomy for |
| - // legitimate use while making a stolen token's usefulness for flooding |
| - // storage negligible. |
| - cliShipBurst = 10 |
| + // that minute. Two, deliberately: a build-and-ship takes longer than |
| + // half a minute to produce anything worth shipping, so two a minute is |
| + // past what an honest iterating caller can generate — while a stolen |
| + // token bursting flat out is held to a TIGHTER instantaneous rate than |
| + // the old ten-in-an-hour allowed (which permitted ten back to back). |
| + // The hourly ceiling rises 10 -> 120; the release retention sweep, not |
| + // this counter, is what bounds what those 120 cost in storage. |
| + cliShipBurst = 2 |
| // cliInstanceBurst bounds the instance verbs (apiinstances.go) per |
| // TOKEN within the ordinary minute window. Keyed by token hash, not by |
| @@ -128,11 +138,13 @@ const ( |
| // for a CI fleet's shared credential running many promotes, rollbacks |
| // and restarts back to back. |
| svcWriteBurst = 120 // per minute, the cliRateWindow |
| - // svcShipBurst: one ship roughly every minute sustained for a full |
| - // hour — six times cliShipBurst's per-token ceiling, for a busy CI |
| - // fleet cutting many releases in a burst rather than one human's |
| - // occasional deploy. |
| - svcShipBurst = 60 // per hour, the cliShipWindow |
| + // svcShipBurst: ten ships a minute sustained — five times |
| + // cliShipBurst's per-token ceiling, for a busy CI fleet cutting many |
| + // releases at once rather than one human's occasional deploy. It kept |
| + // its multiple over the person budget when cliShipWindow went from an |
| + // hour to a minute; the number moved (60 -> 10) only because the window |
| + // under it did. |
| + svcShipBurst = 10 // per minute, the cliShipWindow |
| // wildFailBurst bounds FAILED wildcard enables per principal per |
| // minute — PEEKED before the DNS round trip and CONSUMED only after it |
| @@ -257,6 +269,55 @@ func (l *rateLimiter) tripped(key string, now time.Time) bool { |
| return w.n >= l.max |
| } |
| +// resetIn reports how much of key's CURRENT window is left, so a refusal can |
| +// say WHEN instead of "wait a while". It is meaningful only immediately |
| +// after allow returned false for the same key: a key with no window, or one |
| +// whose window has already run out, has nothing to wait for and answers 0. |
| +// |
| +// Read-only like tripped, and — unlike both allow and tripped — it does NOT |
| +// sweep. The caller it exists for has just been through allow, which swept a |
| +// moment ago; a second walk of the map under the same mutex, on the path |
| +// that is by definition being hit hardest, is the stall allow's own comment |
| +// is at pains to avoid. |
| +// |
| +// Rounded UP to a whole second, and never below one: Retry-After's unit is |
| +// seconds, and a truthful "0" or "0.4" is an invitation to retry straight |
| +// back into the same refusal. |
| +func (l *rateLimiter) resetIn(key string, now time.Time) time.Duration { |
| + l.mu.Lock() |
| + defer l.mu.Unlock() |
| + w := l.hits[key] |
| + if w == nil { |
| + return 0 |
| + } |
| + left := w.start.Add(l.window).Sub(now) |
| + if left <= 0 { |
| + return 0 |
| + } |
| + secs := left / time.Second |
| + if left%time.Second != 0 { |
| + secs++ |
| + } |
| + return secs * time.Second |
| +} |
| + |
| +// retryAfter stamps the one part of a 429 a caller can act on without |
| +// reading prose. The message beside it says the same thing in words for a |
| +// human; this is for the script, the CI step and the agent, all of which |
| +// otherwise have to choose between retrying immediately (into the same |
| +// refusal) and giving up. Called BEFORE jsonError, which writes the status |
| +// line and so closes the header map. |
| +// |
| +// A zero or negative wait is stamped as one second rather than omitted: the |
| +// caller reaching this has been refused, and "retry whenever" is the answer |
| +// that produced the spin. |
| +func retryAfter(w http.ResponseWriter, d time.Duration) { |
| + if d < time.Second { |
| + d = time.Second |
| + } |
| + w.Header().Set("Retry-After", fmt.Sprintf("%d", d/time.Second)) |
| +} |
| + |
| // cliLimits is the set of limiters the CLI endpoints share, hung off the |
| // Server so a test gets a fresh set per fixture. |
| type cliLimits struct { |
diff --git a/internal/console/authcli_test.go b/internal/console/authcli_test.go| index 9a46329f5..12c0546e9 100644 |
| --- a/internal/console/authcli_test.go |
| +++ b/internal/console/authcli_test.go |
| @@ -7,6 +7,7 @@ import ( |
| "io" |
| "net/http" |
| "net/url" |
| + "strconv" |
| "strings" |
| "testing" |
| "time" |
| @@ -711,6 +712,54 @@ func TestServiceShipBudgetIsOwnAndKeyedByName(t *testing.T) { |
| } |
| } |
| +// TestShipRefusalNamesTheWait: a refused ship has to say WHEN — in a header |
| +// a script can read and in prose a human can. |
| +// |
| +// Until 2026-08-23 it said "wait a while" over an HOUR-long fixed window, |
| +// which is the worst pair of answers on this surface: the caller that meets |
| +// this budget most is one shipping in a loop, and it could not tell a short |
| +// pause from a fifty-five-minute lockout, so what it concluded was that |
| +// shipping had stopped working. Naming the wait is only worth doing because |
| +// the window is now a minute; the two changes are one change. |
| +// |
| +// The Retry-After ceiling is asserted as cliShipWindow, not as the literal |
| +// 60: that is the one line here that fails if the window goes back to an |
| +// hour, which is the regression this test is really for. |
| +func TestShipRefusalNamesTheWait(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + now := time.Now() |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, "titogo", "paul@keymail.dev", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + token := cliToken(t, fx, "paul@keymail.dev", "code-ship-retryafter") |
| + for i := 0; i < cliShipBurst; i++ { |
| + v := fmt.Sprintf("w%03d", i) |
| + status, out := postShip(t, fx.ts.URL+"/api/cli/apps/bab/titogo/releases", token, |
| + []shipPart{metaPart(t, v, "binary"), {name: "linux-arm64", body: []byte("x")}}) |
| + if status != http.StatusCreated { |
| + t.Fatalf("ship %d = %d %v", i, status, out) |
| + } |
| + } |
| + status, hdr, out := postShipRaw(t, fx.ts.URL+"/api/cli/apps/bab/titogo/releases", token, |
| + []shipPart{metaPart(t, "over", "binary"), {name: "linux-arm64", body: []byte("x")}}) |
| + if status != http.StatusTooManyRequests { |
| + t.Fatalf("ship over budget = %d %v, want 429", status, out) |
| + } |
| + secs, err := strconv.Atoi(hdr.Get("Retry-After")) |
| + if err != nil { |
| + t.Fatalf("Retry-After = %q, want a whole number of seconds", hdr.Get("Retry-After")) |
| + } |
| + // Never 0 (an invitation to spin straight back into the refusal) and |
| + // never longer than the window it is waiting out. |
| + if secs < 1 || time.Duration(secs)*time.Second > cliShipWindow { |
| + t.Fatalf("Retry-After = %ds, want 1..%d — a wait a caller can sleep on rather than give up on", |
| + secs, cliShipWindow/time.Second) |
| + } |
| + if msg := str(t, out, "error"); !strings.Contains(msg, fmt.Sprintf("%ds", secs)) { |
| + t.Fatalf("refusal %q does not name the %ds wait its own Retry-After promises — the human and the script must be told the same thing", msg, secs) |
| + } |
| +} |
| + |
| // TestPersonShipBudgetStaysKeyedByToken pins the one branch the brief says |
| // must NOT move: a person's ship budget stays keyed on the TOKEN HASH, not |
| // the address (handleAPIShip's own comment) — a second token for the same |
diff --git a/internal/console/svcprincipal.go b/internal/console/svcprincipal.go| index 43dc06f1f..7d55d8f4f 100644 |
| --- a/internal/console/svcprincipal.go |
| +++ b/internal/console/svcprincipal.go |
| @@ -385,11 +385,22 @@ func (s *Server) allowWrite(p Principal, now time.Time) bool { |
| // one shipping budget between them otherwise, which is the limit doing less |
| // than it should. That reasoning is unchanged by this task; only the |
| // service branch is new. |
| -func (s *Server) allowShip(p Principal, r *http.Request, now time.Time) bool { |
| +// |
| +// It is also the one picker that reports HOW LONG a refusal lasts, because |
| +// ship is the one budget a legitimate caller meets in a loop (an agent |
| +// iterating ship-test-ship). The limiter and its key are resolved together |
| +// and the wait is read from THAT limiter — a caller that picked the wait |
| +// from the other one would be told a number belonging to a budget it does |
| +// not spend. The wait is zero, and meaningless, when ok is true. |
| +func (s *Server) allowShip(p Principal, r *http.Request, now time.Time) (ok bool, wait time.Duration) { |
| + lim, key := s.cli.svcShip, principalKey(p) |
| if p.Kind == PrincipalUser { |
| - return s.cli.ship.allow(hashToken(bearer(r)), now) |
| + lim, key = s.cli.ship, hashToken(bearer(r)) |
| + } |
| + if lim.allow(key, now) { |
| + return true, 0 |
| } |
| - return s.cli.svcShip.allow(principalKey(p), now) |
| + return false, lim.resetIn(key, now) |
| } |
| // allowWildFail and wildFailTripped are wildFail's own picker pair — the |