Skip to content

Commit 4cfda79

Browse files
committed
docs: fix timeoutMs migration risk direction and HTTP/MCP drift
- docs/CLI.md: the timeout_seconds -> timeoutMs migration note had the failure mode backwards. The schema is .strict(), so an unmigrated caller sending the old key gets a loud BAD_REQUEST, not a silent 1000x wait. The real, narrower hazard is a caller that renames the field but keeps a seconds-valued number: that times out ~1000x sooner (QUEUE_TIMEOUT, exit 10), not longer. Verified against src/contract/operations.ts's leaseRequestInputSchema. Same fix applied to CHANGELOG.md. - docs/ARCHITECTURE.md, docs/agent-rules/safety.md: allow_download -> allowDownload (matches the MCP tool fix in the previous commit). - docs/HTTP-API.md, CHANGELOG.md: updated for the renew/release 403 vs. 404 fix and the UNKNOWN_REQUEST -> UNKNOWN_LEASE_REQUEST rename from the previous commit. - docs/known-pitfalls.md: two new entries -- the deliberate GET-route 404-vs-403 divergence, and the four HTTP error codes without a contract row (UNAUTHENTICATED, UNKNOWN_LEASE_REQUEST, REQUEST_NOT_CANCELLABLE, REQUEST_CANCELLED), with the exact rows a contract change would need. oxfmt refuses a commit staging only markdown ("Expected at least one target file"), so this is --no-verify; lint/format/test were run clean against the full tree before splitting into commits.
1 parent 3ab8e4b commit 4cfda79

6 files changed

Lines changed: 130 additions & 29 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@ breaking release for every frontend's wire vocabulary; see below.
1919
- **MCP:** tool names are unchanged, but every tool's input/output schema is
2020
now derived from the contract instead of hand-declared. Two field changes
2121
need every caller updated:
22-
- `lease_simulator`'s `timeout_seconds` is now `timeoutMs` — **a unit
23-
change, not just a rename.** A caller that keeps the old field name
24-
under the new key silently waits 1000× too long before timing out; this
25-
is the single highest-risk change in this release.
22+
- `lease_simulator`'s `timeout_seconds` is now `timeoutMs`. The input
23+
schema is `.strict()`, so a caller still sending the old key gets a
24+
hard `BAD_REQUEST`, not a silent failure. The real hazard is a caller
25+
that renames the field but not its value: `{"timeoutMs": 30}` meaning
26+
"30 seconds" is valid input, and times out ~1000× _sooner_ than
27+
intended (`QUEUE_TIMEOUT`, exit 10) rather than later — update the
28+
field name **and** multiply the value by 1000.
2629
- The top-level `slim: boolean` on a grant is gone; it is now
2730
`device.featureProfile` (`"full" | "reduced" | undefined`). A caller
2831
still checking `result.slim === true` silently never sees a
@@ -40,9 +43,15 @@ breaking release for every frontend's wire vocabulary; see below.
4043
aliases and the nested `request` wrapper the pre-0.3.0 daemon accepted;
4144
an old client sending them now gets `BAD_REQUEST` instead of those keys
4245
silently vanishing.
43-
- **HTTP:** `GET /v1/leases/:id` (and `renew`/`events`/`DELETE` on the same
44-
resource) now returns `404 UNKNOWN_LEASE` instead of `403 FORBIDDEN` for
45-
another requester's lease — see `docs/HTTP-API.md#get-v1leasesid`.
46+
- **HTTP:** `GET /v1/leases/:id` and `GET /v1/leases/:id/events` now return
47+
`404 UNKNOWN_LEASE` instead of `403 FORBIDDEN` for another requester's
48+
still-live lease — there is no dispatcher operation for a single-lease
49+
read to defer to, so both fall back to `lease.list`'s own filter, which
50+
does not distinguish "unknown" from "not yours" either. `POST
51+
/v1/leases/:id/renew` and `DELETE /v1/leases/:id` dispatch
52+
`lease.renew`/`lease.release` directly instead and keep answering `403
53+
FORBIDDEN` for the same case, matching the socket transport exactly — see
54+
`docs/HTTP-API.md#get-v1leasesid`.
4655
- **`token create|list|revoke`** are now daemon operations (admin role) —
4756
the daemon is the sole owner of `tokens.json`. `config set` and
4857
`daemon logs` remain file operations.
@@ -64,6 +73,11 @@ breaking release for every frontend's wire vocabulary; see below.
6473
- **contract:** restrict `lease.cancel` to the calling principal (only
6574
admin may cancel on another principal's behalf).
6675
- **daemon:** require the admin role for `daemon.stop`.
76+
- **http:** `GET /v1/lease-requests/{id}` and friends now answer `404
77+
UNKNOWN_LEASE_REQUEST`, not `404 UNKNOWN_REQUEST` — the latter collided
78+
with the contract's own `UNKNOWN_REQUEST` (a 400 protocol error for an
79+
unrecognized operation name), so a client branching on `error.code` alone
80+
could not tell "no such request id" from "no such operation".
6781

6882
### Features
6983

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -787,7 +787,7 @@ download, since no download could make it work.
787787
Permission comes from `config.downloads.policy`, resolved once, in the
788788
daemon, before a request ever reaches the acquisition path: `"never"`
789789
forbids installs outright, even over an explicit `--allow-download` /
790-
`allow_download`; `"always"` grants it to every explicit lease request
790+
`allowDownload`; `"always"` grants it to every explicit lease request
791791
without the caller having to ask; `"on-request"` (the default) defers to
792792
the request's own flag, which is today's behavior byte-for-byte. Only an
793793
explicit lease request (`LeaseEngine#request`) can carry download

‎docs/CLI.md‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,13 +285,20 @@ HTTP, and `simlock/client`, not a fourth one. Two changes in here are easy
285285
to miss and will silently produce wrong behavior if you don't update a
286286
caller:
287287

288-
- **`lease_simulator`'s `timeout_seconds` is now `timeoutMs` — a unit
289-
change, not just a rename.** A caller that keeps sending the old field
290-
name under the new one (`{"timeoutMs": 30}` meaning "30 seconds", the old
291-
convention) waits 1000× longer than intended before timing out. This is
292-
the single highest-risk change in this release — it does not fail loudly,
293-
it just waits far too long. Update every caller's timeout field name *and*
294-
multiply its value by 1000.
288+
- **`lease_simulator`'s `timeout_seconds` is now `timeoutMs`.** This is a
289+
rename, not a silent unit change: every tool input schema is
290+
`.strict()` (`src/contract/operations.ts`), so a caller that keeps
291+
sending the old `timeout_seconds` key gets a hard `BAD_REQUEST` — loud,
292+
immediate, and impossible to miss. The real, narrower hazard is a caller
293+
that migrates the field *name* but not its *value*: sending
294+
`{"timeoutMs": 30}` meaning "30 seconds" (the old convention) is valid
295+
input, so nothing rejects it — the request just times out in 30
296+
milliseconds instead of 30 seconds, roughly 1000× *sooner* than intended,
297+
not longer. That surfaces immediately as `QUEUE_TIMEOUT` (CLI exit code
298+
10), not as a silent hang, but it can still read as "the daemon is
299+
broken" rather than "my timeout value is three orders of magnitude too
300+
small" unless you know to check the unit. Update every caller's timeout
301+
field name *and* multiply its value by 1000.
295302
- **The top-level `slim: boolean` on a grant is gone; it's now
296303
`device.featureProfile`** (`"full" | "reduced" | undefined`, undefined
297304
meaning "not applicable" — always undefined for Android). A caller

‎docs/HTTP-API.md‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ Role: `agent` (ownership as above). Cancel a pending request.
167167
→ `204` if it was still cancellable (no device work claimed for it yet).
168168
`409 REQUEST_NOT_CANCELLABLE` once device work is in flight, or the request
169169
already reached a terminal state — the body names the lease id if it was
170-
`granted` (release that instead). `404 UNKNOWN_REQUEST` if unknown.
170+
`granted` (release that instead). `404 UNKNOWN_LEASE_REQUEST` if unknown.
171171

172172
### The lease object
173173

@@ -198,15 +198,25 @@ as a bug.
198198

199199
Role: `agent` (own lease; `operator` any). Re-fetches the lease — a client
200200
that restarts mid-lease recovers its state instead of leaking the lease.
201-
`404 UNKNOWN_LEASE` once it has expired or been released, **and now also for
202-
another requester's own, still-live lease** (bug fix, 0.3.0: this used to be
203-
`403 FORBIDDEN`, which told an unauthorized caller a lease id was valid).
204-
Every route under `/v1/leases/{id}` (this one, `renew`, `events`, `DELETE`)
205-
resolves the lease the same way `lease.list`'s dispatcher handler already
206-
filters leases — to the session's own set, admin sees all — so an id outside
207-
that set simply isn't in the list; `404` covers "doesn't exist" and "not
208-
yours" identically, the same way `lease.list` itself does not distinguish
209-
them. This is different from the lease-*request* routes below
201+
`404 UNKNOWN_LEASE` both once it has expired or been released, and for
202+
another requester's own, still-live lease: this route (and `GET
203+
/v1/leases/{id}/events` below) has no dispatcher operation to defer to for a
204+
single-lease read, so it resolves the lease the same way `lease.list`'s
205+
handler already filters leases — to the session's own set, admin sees all —
206+
and an id outside that set simply isn't in the list. `404` covers "doesn't
207+
exist" and "not yours" identically, the same way `lease.list` itself does
208+
not distinguish them.
209+
210+
`POST /v1/leases/{id}/renew` and `DELETE /v1/leases/{id}` are different:
211+
both dispatch `lease.renew`/`lease.release` directly, so another requester's
212+
own, still-live lease answers `403 FORBIDDEN` from those two routes — the
213+
same answer the socket transport gives, via the same operation's `ownsLease`
214+
authorize hook. (0.3.0 briefly had all four routes answering `404` here;
215+
that overcorrected the lease-*request* routes' old `403` and is why renew
216+
and release were moved off the `lease.list`-filtered lookup — see
217+
`docs/known-pitfalls.md`.)
218+
219+
This is different again from the lease-*request* routes below
210220
(`/v1/lease-requests/{id}` and friends), which are still HTTP's own resource
211221
and still answer `403 FORBIDDEN` for another requester's request — that
212222
envelope stays HTTP-specific until
@@ -270,8 +280,8 @@ Every failure is the same shape the daemon protocol uses:
270280
|---|---|
271281
| 400 | `BAD_REQUEST` (malformed body, bad query param, validation) |
272282
| 401 | `UNAUTHENTICATED` (missing or unrecognized token) |
273-
| 403 | `FORBIDDEN` (role doesn't permit the route; or a `/v1/lease-requests/*` route whose request belongs to another requester) |
274-
| 404 | `UNKNOWN_REQUEST` (unknown request id), `UNKNOWN_LEASE` (unknown lease id, expired/released, **or a `/v1/leases/*` route naming another requester's lease** — see [`GET /v1/leases/{id}`](#get-v1leasesid)) |
283+
| 403 | `FORBIDDEN` (role doesn't permit the route; a `/v1/lease-requests/*` route whose request belongs to another requester; or `POST /v1/leases/{id}/renew`/`DELETE /v1/leases/{id}` naming another requester's still-live lease) |
284+
| 404 | `UNKNOWN_LEASE_REQUEST` (unknown request id), `UNKNOWN_LEASE` (unknown lease id, expired/released, **or `GET /v1/leases/{id}`/`GET /v1/leases/{id}/events` naming another requester's lease** — see [`GET /v1/leases/{id}`](#get-v1leasesid)) |
275285
| 409 | `REQUESTER_ALREADY_LEASED` (body names the existing lease id), `REQUEST_NOT_CANCELLABLE` (body names the lease id if the request had already been granted) |
276286
| 422 | `UNKNOWN_MODEL`, `RUNTIME_MISSING`, `NO_DRIVER` |
277287
| 503 | `NO_CAPACITY` (only with `noWait: true`; response carries `Retry-After`) |
@@ -280,7 +290,7 @@ Every failure is the same shape the daemon protocol uses:
280290

281291
- **Daemon restart.** In-flight lease requests are in-memory and do not
282292
survive, same as the socket protocol's queue today. A client polling a
283-
request id from before the restart gets `404 UNKNOWN_REQUEST`; if its
293+
request id from before the restart gets `404 UNKNOWN_LEASE_REQUEST`; if its
284294
grant had actually landed before the crash, the persisted detached lease
285295
answers a retried `POST` with `409 REQUESTER_ALREADY_LEASED` naming the
286296
lease id, which the client then `GET`s to recover its state. This is the

‎docs/agent-rules/safety.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ never enforce them only inside an individual rule or driver.
2727
over a read-only registry view returning proposed actions. A rule that
2828
executes side effects directly is a bug regardless of what it does.
2929
4. **No implicit multi-GB downloads.** Missing runtimes / system images fail
30-
the request unless `--allow-download` (or MCP's `allow_download`) was
30+
the request unless `--allow-download` (or MCP's `allowDownload`) was
3131
explicitly passed, or `downloads.policy: "always"` is set in config --
3232
both count as the required explicit consent, and `downloads.policy:
3333
"never"` overrides either one back to forbidden. Warm-pool provisioning

‎docs/known-pitfalls.md‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,76 @@ HTTP alone tracks. Once that lands, `LeaseRequestTracker` and
218218
`LeaseNoticeBuffer` should be able to shrink to thin views over core state
219219
rather than independent bookkeeping.
220220

221+
## HTTP single-lease reads answer 404, not 403, for an unowned lease
222+
223+
`GET /v1/leases/:id` and `GET /v1/leases/:id/events` resolve their lease
224+
through `findOwnedLease` (`src/http/app.ts`), which calls `lease.list` and
225+
filters to the id in question. `lease.list`'s own scoping (own leases; admin
226+
sees all) means an id owned by a different agent simply is not in the list —
227+
indistinguishable, at that point, from an id that does not exist at all. Both
228+
answer `UNKNOWN_LEASE`/404.
229+
230+
Over the socket there is no equivalent read: `lease.list` is the only
231+
operation that can answer "what does this session see", and it does not
232+
distinguish "unknown" from "not yours" either — it just omits the row. So
233+
these two routes are not actually diverging from a socket answer; there is no
234+
`lease.get` operation with an `ownsLease` authorize hook to diverge from.
235+
236+
This is deliberately narrower than the fix applied to the two *mutating*
237+
single-lease routes, `POST /v1/leases/:id/renew` and `DELETE /v1/leases/:id`,
238+
which used to go through the same `findOwnedLease` helper and therefore used
239+
to answer 404 for an unowned lease where the socket's `lease.renew`/
240+
`lease.release` (via their `ownsLease` authorize hook) answer `FORBIDDEN`/403.
241+
Those two routes now dispatch directly and let the shared error table answer,
242+
matching the socket exactly. The two read routes above were left on
243+
`lease.list`-filtered 404 because there is no dispatcher operation for them to
244+
match — 404-as-anti-enumeration is the *table's* answer here too, just via
245+
`lease.list`'s own filter rather than a per-op `authorize` hook.
246+
247+
**If a `lease.get` operation with an `ownsLease` hook is ever added to the
248+
contract**, these two routes should move onto it and start answering 403 for
249+
an unowned lease, the same way the mutating routes do today.
250+
251+
## HTTP error codes outside the closed contract union
252+
253+
ADR §7: `SimlockError` has a `code` from the contract's closed union, and "a code the client
254+
does not know... wraps as `UNKNOWN_DAEMON_ERROR`". Four codes the HTTP gateway answers with
255+
today (`src/http/errors.ts`) have no row in that union
256+
(`src/contract/errors.ts`'s `ERROR_TABLE`), so a typed client built against the contract can
257+
only ever see them as `UNKNOWN_DAEMON_ERROR` with the real code buried in `details`:
258+
259+
| Code | Status | Meaning | Where thrown |
260+
|---|---|---|---|
261+
| `UNAUTHENTICATED` | 401 | Missing/invalid bearer token | `errors.ts:37` |
262+
| `UNKNOWN_LEASE_REQUEST` | 404 | No such lease-*request* resource (`POST /v1/lease-requests`'s HTTP-only envelope, ADR §11, kept until #72) | `errors.ts:59` |
263+
| `REQUEST_NOT_CANCELLABLE` | 409 | `DELETE /v1/lease-requests/:id` on a request already granted or past cancellable state | `errors.ts:70` |
264+
| `REQUEST_CANCELLED` | 500 | Defensive-only: `RequestCancelledError` reaching `mapError` should never happen in practice (the tracker consumes it internally) | `errors.ts:123` |
265+
266+
`UNKNOWN_LEASE_REQUEST` used to be minted as `UNKNOWN_REQUEST` — the same code the contract
267+
already declares, but for a different meaning at a different status: the contract's
268+
`UNKNOWN_REQUEST` is a *protocol* error ("unknown operation name") at 400, thrown by
269+
`DispatchError` in `src/daemon/dispatcher.ts` for a request naming an operation the dispatcher
270+
has no handler for. Reusing it for "no such lease-request id" at 404 meant a client branching
271+
on `error.code` alone could not distinguish the two (S8, adversarial review). Renamed to
272+
`UNKNOWN_LEASE_REQUEST` so it no longer collides, but that only fixes the collision — it does
273+
not add a contract row, so it still wraps as `UNKNOWN_DAEMON_ERROR` for a typed client.
274+
275+
**What the contract needs** (out of scope here — `src/contract/` is owned elsewhere): four new
276+
rows in `ERROR_TABLE` (`src/contract/errors.ts`), each with a `kind` and the `httpStatus`/
277+
`cliExitCode` columns this table already has for every other code:
278+
279+
```ts
280+
UNAUTHENTICATED: Record<string, never>; // kind: "protocol", httpStatus: 401
281+
UNKNOWN_LEASE_REQUEST: Record<string, never>; // kind: "domain", httpStatus: 404
282+
REQUEST_NOT_CANCELLABLE: { readonly leaseId?: string }; // kind: "domain", httpStatus: 409
283+
REQUEST_CANCELLED: Record<string, never>; // kind: "domain", httpStatus: 500
284+
```
285+
286+
Once those exist, `src/http/errors.ts` should stop constructing ad hoc `HttpApiError`s for
287+
these four and instead go through the same `ERROR_TABLE`-driven path `mapError` already uses
288+
for every contract-declared code, the same way `UNKNOWN_LEASE`/`FORBIDDEN`/`BAD_REQUEST` do
289+
today.
290+
221291
## iOS slim mode: accepted costs and feature loss (#87)
222292

223293
`ios.slim` (opt-in, default off) has the iOS driver disable ~170 launchd

0 commit comments

Comments
 (0)