| index 3e3c08730..fc75635b2 100644 |
| --- a/docs/superpowers/plans/2026-10-24-service-runs.md |
| +++ b/docs/superpowers/plans/2026-10-24-service-runs.md |
| @@ -12,8 +12,9 @@ second run. |
| `Principal`); |
| - the run doors admitting that principal with its narrower checks |
| (`apiruns.go`, `apirunsfollow.go`); |
| -- an idempotency key, stored create-only beside the run objects, that answers |
| - retries from its own state. |
| +- an idempotency key, recorded in the instance record by the same |
| + compare-and-swap that admits the run, so admission and dedupe are one |
| + atomic write. |
| |
| **Tech Stack:** Go. The console's SQLite store (`internal/console/authstore.go`). |
| The deployment bucket through `release.Store` (`Put(ctx, key, body, ifMatch)`: |
| @@ -35,18 +36,19 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| role reaches the run doors, and `Admin` does not. |
| - **Command prefixes:** |
| - at most 32 per credential; |
| - - each word 1–128 bytes, with no empty word; |
| - - matched word for word, exactly: no globbing, no case folding, no substring. |
| -- **`Idempotency-Key`:** 1–128 bytes of `[A-Za-z0-9._:-]`. It is honoured for 7 |
| - days from the create. |
| -- **Timings:** |
| - - create handler deadline: 2 minutes; |
| - - pending-key abandon threshold: 5 minutes; |
| - - key sweep age: 7 days. |
| + - each prefix has at least one word; |
| + - each word is 1–128 bytes; |
| + - matched word for word, exactly: no globbing, no case folding, no substring; |
| + - a zero-word prefix never matches. |
| +- **`Idempotency-Key`:** 1–128 bytes of `[A-Za-z0-9._:-]`. The record's |
| + `run_keys` keeps entries for 24 hours, and the newest 128 at most. |
| - **`on_behalf_of`:** an email of at most 254 bytes, set only by a `run` |
| service. A person who sends it gets a 400. |
| -- **Every refusal on a run door carries a stable `code`** in its JSON body, |
| - alongside today's `error` message. |
| +- **Every refusal written by a run HANDLER carries a stable `code`** in its |
| + JSON body, alongside today's `error` message. The shared gates keep their |
| + bodies exactly as today, deliberately identical across doors: |
| + `cliNotSignedIn`, the role-mismatch 403 in `cliRoute`, and the canonical |
| + membership `http.NotFound` in `apiAccountApp`. |
| - **A new `carlos` flag isn't done until `docs/site/cli.md` names it** |
| (AGENTS.md). |
| - **Branch rules:** the branch lands through amadan (`amadan branch describe`, |
| @@ -55,7 +57,7 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| |
| ## Review Focus |
| |
| -1. **A retry of a known key while the first run is still open.** Today the |
| +1. **A retry of an admitted key while the first run is still open.** Today the |
| "already has a run open" refusal (`apiruns.go:201`) fires before the body is |
| read. A keyed retry must get its run back, not that refusal. The pin is |
| `TestRunKeyedRetryWhileOpenReturnsTheRun` in Task 4. |
| @@ -70,7 +72,7 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| `TestCommandGranted` table. |
| 5. **A keyed create whose meta declares a sha256 that doesn't match the |
| uploaded bytes.** It gets a 400 `file_hash_mismatch`, and nothing is |
| - written, key included. The pin is `TestRunKeyedHashMismatchWritesNothing` |
| + written. The pin is `TestRunKeyedHashMismatchWritesNothing` |
| in Task 4. |
| |
| --- |
| @@ -165,8 +167,11 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| - `{"Import","classic"}` is false; |
| - `{}` under anything is false; |
| - with two prefixes, matching the second is true. |
| - - `TestCheckCommandPrefixes`: an empty list gives `ErrNoCommands`. An empty |
| - word, a 129-byte word and 33 prefixes are each refused. 32 prefixes of |
| + - a zero-word prefix (`[[]]`, and `[nil]`) matches nothing. |
| + - `TestCheckCommandPrefixes`: an empty list gives `ErrNoCommands`. A |
| + zero-word prefix, an empty word, a 129-byte word and 33 prefixes are each |
| + refused. `CLIPrincipal` also refuses a stored `run` row whose commands hold |
| + a zero-word prefix (`TestRunCredentialRefusedWithAnEmptyPrefix`). 32 prefixes of |
| 128-byte words are accepted. |
| - `TestRunCredentialCarriesItsCommands`: mint a `Run` principal with |
| `[["import","classic"]]`, and `CLIPrincipal` returns those `Commands` and a |
| @@ -243,11 +248,14 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| `serviceToken`, and mint `Run` with `[["import","classic"]]`): |
| - `TestRunRoleMatrix`: on all five routes, a `Run` service passes the role |
| gate. `Publish`, `Deploy`, `Instance`, `Operate`, `Service` and `Admin` |
| - get the "closed to service credentials" 403. Today's |
| - `apiruns_test.go:189` expectation for `Instance` still holds. |
| + each get `cliRoute`'s role-mismatch 403 (`authcli.go:467`), not the |
| + closed-door message, because the set is no longer empty. Update the tests |
| + that pin the closed-door registrations and the route-role table |
| + (`svcroutes_test.go:226` and the fixed door count there). Keep explicit |
| + coverage that Admin is refused on the run doors. |
| - `TestRunServiceStartsAGrantedCommand`: argv `import classic --slug x` |
| - gets a 201 or 200 (as today), and request.json has `ByService`, |
| - `ByCredential` and `OnBehalfOf`. |
| + gets a 202, as create gives today (`apiruns.go:317`), and request.json has |
| + `ByService`, `ByCredential` and `OnBehalfOf`. |
| - `TestRunServiceRefusedAnUngrantedCommand`: argv `db shell` gets 403 |
| `command_not_granted`, and no run objects exist. |
| - `TestRunServiceWrongAppRefused`: a `Run` credential for app `other` on |
| @@ -265,8 +273,10 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| - `TestRunPersonMaySetNoOnBehalfOf`: a person's meta with `on_behalf_of` |
| gets 400 `on_behalf_of_invalid`. |
| - `TestRunOnBehalfOfShape`: not an email, or 255 bytes, gets 400. |
| - - `TestRunRefusalsCarryCodes`: every refusal the run tests provoke has a |
| - `code` that is in `runRefusalCodes`. |
| + - `TestRunRefusalsCarryCodes`: every refusal a run HANDLER writes has a |
| + `code` that is in `runRefusalCodes`. The shared-gate refusals (401, the |
| + role 403, the membership 404) are byte-identical to today, and |
| + `apiruns_test.go:195`'s canonical-404 test is unchanged. |
| - `TestServiceRunRendersAsTheServices`: the follow and history rendering of |
| a service's run contains `run by service ci for paul@example.com (as |
| stated by the service)` and never presents the email as the actor. |
| @@ -284,109 +294,112 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| |
| --- |
| |
| -### Task 4: Idempotent create |
| +### Task 4: Idempotent create, settled by the record |
| |
| **Files:** |
| +- Modify: `internal/provision/provision.go`. `Record` gains |
| + `RunKeys []RunKey \`json:"run_keys,omitempty"\``. |
| +- Modify: `internal/provision/run.go`. |
| + - `RunAdmission` gains `Key string` (the scoped key hash) and `Digest string`. |
| + - `RunAdmit` handles the key inside the same mutation. |
| + - Every record mutation that replaces or clears `ActiveRun` (the next |
| + admission, ack, expire, the sweep's clear) keeps `RunKeys`. |
| - Create: `internal/runs/keys.go` |
| +- Modify: `internal/runs/runs.go`. `MetaFile` gains `SHA256 string` |
| + (`omitempty`). Convert the positional `MetaFile{...}` literals in existing |
| + tests (`internal/runs/runs_test.go:74-89` and any others the compiler |
| + finds) to named fields. |
| - Modify: `internal/console/apiruns.go` |
| -- Modify: the run sweep (`internal/console/runsweep.go`), adding a key sweep |
| -- Test: `internal/runs/keys_test.go`, `internal/console/apiruns_key_test.go` |
| +- Test: `internal/runs/keys_test.go`, `internal/provision/run_keys_test.go`, |
| + `internal/console/apiruns_key_test.go` |
| |
| **Interfaces:** |
| - Produces in `internal/runs`: |
| - `func ValidKey(k string) bool` |
| - - `func KeyObject(acct, app, scope, host, key string) string`. It returns |
| - `RunsPrefix(acct, app) + "keys/" + hex(sha256(scope + "\x00" + host + "\x00" + key)) + ".json"`. |
| - `scope` is `"svc:" + CredentialID` for a service and `"user:" + address` |
| - for a person. |
| - - `type KeyState string`, with `KeyPending`, `KeyAdmitted`, `KeyRefused` |
| - and `KeyAbandoned`. |
| - - `type Key struct { Schema int; State KeyState; RunID string; Digest string; Code string; Status int; CreatedAt, UpdatedAt time.Time }` |
| + - `func ScopedKey(scope, key string) string`. It returns |
| + `hex(sha256(scope + "\x00" + key))`. `scope` is `"svc:" + CredentialID` |
| + for a service and `"user:" + address` for a person. |
| - `func MetaDigest(m Meta) string`: sha256 over a canonical encoding of argv, |
| timeout_s, on_behalf_of and every file's name, size and sha256. |
| -- `MetaFile` gains `SHA256 string` (`omitempty`). A keyed create requires it |
| - on every file. The spooled bytes are verified against it, and a mismatch |
| - gives 400 `file_hash_mismatch` before any write. |
| -- **Create's order changes, but only for a request with an `Idempotency-Key`.** |
| - An unkeyed request keeps today's exact order, so Global Constraint 1 holds. |
| - For a keyed create: |
| + - `const KeyTTL = 24 * time.Hour`, `const MaxRunKeys = 128`. |
| +- Produces in `internal/provision`: |
| + - `type RunKey struct { Key, RunID, Digest string; At time.Time }` |
| + - `type RunKeyReplay struct { RunID string }`, which implements `error`, |
| + returned by `RunAdmit` when the key is already there with the same digest; |
| + - `var ErrRunKeyReused`, for when the key is already there with a different |
| + digest. |
| +- `RunAdmit`'s mutation, in this order inside the one CAS: |
| + 1. prune `RunKeys` to entries younger than `KeyTTL`; |
| + 2. if `p.Key != ""` and an entry has that key: return `ErrRunKeyReused` if |
| + its digest differs, else `&RunKeyReplay{RunID}`. Both run BEFORE today's |
| + open-run, moving and import refusals, so a replay is answered even while |
| + its own run is still open; |
| + 3. today's admission; |
| + 4. on success, when `p.Key != ""`, append `{Key, RunID, Digest, now}` and |
| + trim to the newest `MaxRunKeys`. |
| +- **Create's order, but only for a keyed request** (an unkeyed request keeps |
| + today's exact order): |
| 1. principal, membership, host-claim and rate budget; |
| 2. read the record (a 404 stays a 404); |
| 3. parse the meta; |
| - 4. check the key (below); |
| - 5. today's class, move, open-run and import refusals, then the command check |
| - (Task 3); |
| - 6. spool and verify the files; |
| - 7. mint the id; |
| - 8. write the `pending` key create-only; |
| - 9. write the objects; |
| - 10. admit; |
| - 11. CAS the key to `admitted` or `refused`, recording the code and status. |
| -- **The key check**, at step 4: |
| - - **No key object:** continue. |
| - - **Digest differs:** 409 `idempotency_key_reused`, and write nothing. |
| - - **`admitted`:** 200 with the create response for `RunID` (its id and the |
| - record's state if the record still holds it, else the terminal state from |
| - result.json, else `done`). Stop reading the body. |
| - - **`refused`:** repeat the stored `Status` and `Code`. |
| - - **`pending`, younger than 5 minutes:** 409 `in_progress`, with |
| - `Retry-After: 5`. |
| - - **`pending`, 5 minutes or older:** re-read the record. If its `ActiveRun.ID` |
| - equals `RunID`, CAS the key to `admitted` and answer as `admitted`. |
| - Otherwise CAS it to `abandoned` and proceed as fresh. |
| - - **`abandoned`:** proceed as fresh, overwriting the key by CAS on its etag |
| - at step 8. |
| -- The create handler's context gets a 2-minute deadline that covers the whole |
| - keyed path. The admission's own 30-second write context sits inside it. |
| -- `GET .../instances/{host}/runs?key=<k>` is a new route on the same |
| - `Exactly(Run)` set plus people. It returns |
| - `{"state": ..., "run_id": ...}` or a 404, scoped by the caller's `scope`. |
| -- **The key sweep** deletes key objects whose `CreatedAt` is older than 7 |
| - days. It runs in the same loop as the run sweep, and it logs a count and |
| - never deletes anything younger. |
| + 4. **look up the key in that record read.** On the same digest, answer |
| + `202` with the run (`"replayed": true`) and stop reading the body. On a |
| + different digest, `409` `idempotency_key_reused`; |
| + 5. today's class, move, open-run and import refusals; |
| + 6. the command check (Task 3); |
| + 7. spool the files and verify each against its declared sha256 (`400` |
| + `file_hash_mismatch`, nothing written); |
| + 8. mint the id, write the objects, and admit with `Key` and `Digest`; |
| + 9. if admission returns `*RunKeyReplay` (a concurrent twin won), delete |
| + this request's objects and answer `202` with the winner's run, replayed; |
| + `ErrRunKeyReused` answers `409`; |
| + 10. today's lost-admission settling (`apiruns.go:279-291`) is unchanged, and |
| + is now also correct for keys, because the key commits with the |
| + admission or not at all. |
| +- `GET .../instances/{host}/runs?key=<k>`: a new route on the run set |
| + (`Exactly(Run)`, plus people). It returns `{"run_id": ...}` from the |
| + record's `RunKeys` for the caller's scoped key, or 404. |
| |
| - [ ] **Step 1: Write the failing tests** |
| - `TestValidKey`: a table of good keys, plus an empty key, a 129-byte key and |
| one containing `/` or a space. |
| - - `TestKeyObjectSeparatesCallersAndHosts`: the same key under two scopes, or |
| - two hosts, gives two different objects. |
| + - `TestScopedKeySeparatesCallers`. |
| - `TestMetaDigestCoversEveryField`: changing argv, timeout, on_behalf_of, a |
| file name, a size or a sha256 each changes the digest. |
| - - `TestRunKeyedCreateThenRetryReturnsTheSameRun`: create with key K, retry |
| - the identical request, get the same `id`, and the box saw one request |
| - (one request.json). |
| - - `TestRunKeyedRetryWhileOpenReturnsTheRun`: the run is still `queued` or |
| - `picked`, and the retry gives a 200 with that id, not `run_open`. |
| - - `TestRunKeyedRetryAfterAckStillReturnsTheRun`: ack, sweep the run's |
| - objects, retry, and still get the same id with no new run. |
| - - `TestRunKeyReusedWithADifferentRequest`: 409 `idempotency_key_reused`, |
| - with nothing written. |
| - - `TestRunConcurrentIdenticalKeyedCreatesMakeOneRun`: two goroutines create |
| - with the same key. One run results, and the other answer is either 409 |
| - `in_progress` or the same id. |
| - - `TestRunAbandonedPendingKeySettles`: a planted `pending` key, 6 minutes |
| - old (the test clock), whose run the record holds, settles to `admitted`. |
| - The same, with the record not holding it, settles to `abandoned` and a |
| - fresh run is created. |
| - - `TestRunRefusedKeyRepeatsItsRefusal`: a keyed create refused for |
| - `command_not_granted` is refused the same way on retry. |
| - - `TestRunKeyOfAnotherCallerIsInvisible`: the person's key K, used by the |
| - service, behaves as a fresh key, and `?key=K` from the service is a 404. |
| + - `TestRunAdmitRecordsAndReplaysAKey`, at the provision level: |
| + - admit with K and D, then admit again with K and D, which gives |
| + `RunKeyReplay` with the first id; |
| + - K with D2 gives `ErrRunKeyReused`; |
| + - after the run is acked or cleared, K and D still replay; |
| + - after a second, unkeyed run replaces it, K and D still replay. |
| + - `TestRunKeysPrunedByAgeAndCount`: entries older than 24h are dropped, and |
| + at most 128 are kept, newest first. |
| + - `TestRunKeyedCreateThenRetryReturnsTheSameRun`: same id, one request.json, |
| + and the retry carries `replayed: true`. |
| + - `TestRunKeyedRetryWhileOpenReturnsTheRun` |
| + - `TestRunKeyedRetryAfterAckStillReturnsTheRun` |
| + - `TestRunKeyReusedWithADifferentRequest`: 409, with nothing written. |
| + - `TestRunConcurrentIdenticalKeyedCreatesMakeOneRun`: two goroutines with |
| + the same key and request. Exactly one admission happens, both answers name |
| + the same id, and the loser's objects are deleted. |
| + - `TestRunKeyOfAnotherCallerIsInvisible`: the person's K used by the service |
| + is fresh, and `?key=K` from the service is a 404. |
| - `TestRunKeyedHashMismatchWritesNothing` |
| - `TestRunKnownKeyAnsweredWithoutReadingFiles`: retry with a body whose file |
| - part errors on read. The answer is still a 200 with the id. |
| - - `TestRunKeyLookup`: `?key=` returns the state and run id. |
| - - `TestRunKeySweepRemovesOnlyOldKeys`: an 8-day-old key is gone, and a 6-day |
| - one stays. |
| + part errors on read, and still get a 202 with the id. |
| + - `TestRunRefusedKeyIsReevaluated`: a keyed create refused with `run_open` |
| + starts the run once the other run is acked. |
| + - `TestRunKeyLookup` |
| - `TestRunUnkeyedOrderUnchanged`: an unkeyed create against an open run is |
| - still refused before the body is read (today's order). |
| + still refused before the body is read. |
| - [ ] **Step 2: Run them and watch them fail.** |
| - `go test ./internal/runs/ ./internal/console/ -run 'Key|Keyed|Digest'` fails. |
| + `go test ./internal/runs/ ./internal/provision/ ./internal/console/ -run 'Key|Keyed|Digest'` |
| + fails. |
| - [ ] **Step 3: Implement.** |
| - [ ] **Step 4: Watch them pass**, plus every pre-existing test under |
| - `go test ./internal/console/ -run Run`. |
| -- [ ] **Step 5: Commit**: `console: a keyed run create answers a retry from the |
| - key's own state and never runs twice (PRO-6214)` |
| + `go test ./internal/provision/ ./internal/console/ -run Run`. |
| +- [ ] **Step 5: Commit**: `console: a keyed run create is admitted once, |
| + settled by the instance record, and a retry gets the same run (PRO-6214)` |
| |
| --- |
| |
| @@ -395,7 +408,9 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| **Files:** |
| - Modify: `run.go`. `carlos run` gains `--idempotency-key <key>`, sends it as |
| the header, and sends each file's sha256 in the meta when a key is given. |
| - It prints the run id before streaming, as it does today. |
| + It accepts the `202` replay (`"replayed": true`) the same way as a fresh |
| + `202` (`run.go:223`), saying "already started: run <id>" before following |
| + it. It prints the run id before streaming, as it does today. |
| - Modify: `docs/site/cli.md`, covering `carlos run --idempotency-key` and |
| `carlos services create --role run --commands`. |
| - Modify: the services and run docs pages under `docs/site/`, wherever the |
| @@ -415,6 +430,9 @@ amends `docs/superpowers/specs/2026-10-08-carlos-run-design.md` § 2 ruling 1. |
| - [ ] **Step 1: Write the failing tests** |
| - `TestRunIdempotencyKeyFlag`: the request carries `Idempotency-Key`, and the |
| meta's files carry `sha256`. |
| + - `TestRunCLIRetryFollowsTheReplayedRun`: against a test console, a second |
| + `carlos run` with the same key prints "already started", follows the same |
| + id, and starts no second run. |
| - An invalid key is refused by the CLI before any network call. |
| - `TestRunRefusalCodesAreDocumented`: every entry in `runRefusalCodes` |
| appears in the run API doc page. |
amadan