mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-19 17:25:19 +03:00
8c80c025be072f081a81ec27cd82db570a1de189
411 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Wanjohi
|
8c80c025be |
feat(deploy): drop the IaC layer, and make both apps runnable as containers (#325)
Moving the issuer's state into Postgres (#324) removed the last thing
tying either app to one hosting provider. What was left was a deployment
tool describing resources that no longer existed — so this drops it in
favour of `wrangler`, which is what actually deploys a Worker, and adds
a second way to run each app that involves no provider at all.
## What changes
**Gone:** `alchemy.run.ts`, `docs/alchemy.md`, the `alchemy` and
`effect` root dependencies, and the two type imports that reached out of
`apps/api` into the infrastructure file.
**In its place**, per app:
| | |
|---|---|
| `wrangler.jsonc` | one environment per stage, custom-domain routes,
Hyperdrive, the `AUTH` service binding |
| `Dockerfile` + `server.ts` | the same handler behind a listening
socket |
The handler is the same one either way. What differs is only where its
settings come from, and two of them gained a second spelling so that
nothing has to branch on the runtime: Postgres arrives as a pooled
binding or as `DATABASE_URL`, and the route to the issuer is a service
binding or `AUTH_INTERNAL_URL`.
`docker-compose.yml` now brings up Postgres and both apps together,
which is both what a self-hoster runs and the shape this takes when it
stops being a set of Workers.
## `AUTH_INTERNAL_URL`, which is new
A service binding was quietly doing two jobs: routing to the issuer, and
letting the `iss` claim stay the issuer's public name. Nothing else can
do both with one setting — the public name is often not routable from
inside a deployment — so the name and the route are two settings now.
`AUTH_ISSUER_URL` is still compared literally against every token, and
is unchanged.
## DNS
Moves out of code and into [`docs/dns.md`](docs/dns.md): every hostname,
what it is for, and what answers it today. Six records that change
roughly never did not need a tool, and a table outlives whatever is
serving the names — which is the point, because some of them will stop
being Workers. `wrangler` keeps owning only the part that must stay in
step with a deploy, since a route and its hostname are one fact.
The one rule the table enforces is **one label deep on `nestri.io`**. A
certificate for `*.nestri.io` covers one level and not two, so the
sandbox names are hyphenated rather than nested —
`api-sandbox.nestri.io` can become an ordinary proxied origin later
without a certificate having to be ordered for it first. Production
hostnames are unchanged.
## Also
`EMAIL_DEV_LOG` moves from committed configuration into
`apps/auth/.dev.vars`, which `wrangler deploy` cannot upload. Printing a
live sign-in code to a log should not be one forgotten override away
from a stage somebody else can reach.
## Before this deploys
1. The Hyperdrive ids in both `wrangler.jsonc` files are placeholders.
`wrangler hyperdrive list` has the real ones. Local development works
without them.
2. The Worker names change, so the first deploy creates new Workers.
Confirm the custom domains answer, *then* remove the old Workers and
routes — that order, or the names resolve to nothing in between.
3. Set the secrets listed in [`docs/deploy.md`](docs/deploy.md). The
issuer refuses to sign anyone in without its three mail settings.
## Checks
- 316 tests pass, 0 fail, against a freshly migrated database.
- All four wrangler environments bundle with no warnings.
- Both images build, run, and report `healthy`; the API container
reaches the issuer container and rejects a bad token as 401 rather than
500.
- `AUTH_INTERNAL_URL` verified end to end: discovery and JWKS resolve
through the internal route while `iss` stays the public name.
- `oxlint` clean apart from one pre-existing unused import in
`packages/auth`.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR replaces the Alchemy deployment layer with direct Wrangler
configuration and container-based execution for both control-plane
applications.
- Adds Bun HTTP servers, production Dockerfiles, health checks, and a
Compose deployment for Postgres, auth, and API.
- Supports database and issuer routing through either Cloudflare
bindings or ordinary environment variables.
- Adds stage-specific Worker routes, service bindings, Hyperdrive
configuration, and deployment documentation.
- Moves DNS ownership and deployment guidance into dedicated
documentation.
- Removes unused Alchemy, Effect, and Steam API-key configuration.
<h3>Confidence Score: 5/5</h3>
The current changes appear safe to merge, with no established new
defects or outstanding previous findings.
The container and Worker configurations are internally consistent,
required privileged credentials no longer have Compose defaults, and
plaintext service ports are loopback-bound. The three previous threads
were manually resolved without explanation and therefore are not
outstanding.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| docker-compose.yml | Defines the self-hosted stack with required
credentials, loopback-bound ports, mail pass-through, health
dependencies, and internal issuer routing. |
| apps/api/app/middleware/auth.ts | Adds issuer access through either a
Worker service binding or AUTH_INTERNAL_URL while preserving the public
issuer used for token validation. |
| apps/api/app/server.ts | Exposes the existing API handler through
Bun’s HTTP server with a process-compatible execution context. |
| apps/auth/src/server.ts | Exposes the existing authentication handler
through Bun’s HTTP server. |
| apps/api/wrangler.jsonc | Configures API development, sandbox, and
production Workers with routes, Hyperdrive, issuer settings, and auth
service bindings. |
| apps/auth/wrangler.jsonc | Configures auth development, sandbox, and
production Workers with custom domains and Hyperdrive. |
| packages/core/src/env.ts | Supports database and issuer routing
through environment variables and removes an unused Steam API-key
setting. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Client[Clients] --> Proxy[Custom domain or TLS proxy]
Proxy --> API[API handler]
Proxy --> Auth[Auth issuer handler]
API -->|AUTH service binding| Auth
API -->|AUTH_INTERNAL_URL in containers| Auth
API --> DB[(Postgres)]
Auth --> DB
Wrangler[Cloudflare Wrangler runtime] --> API
Wrangler --> Auth
Compose[Docker Compose runtime] --> API
Compose --> Auth
Compose --> DB
```
<sub>Reviews (2): Last reviewed commit: ["fix(deploy): require every
credential,
a..."](
|
||
|
Wanjohi
|
f30a1432f8 |
fix(deploy): require every credential, and give sandbox its own domain
Three things review caught, and one shape correction. **No credential has a default any more.** The compose file shipped `ADMIN_SHARED_SECRET` falling back to a value written in this repository — and that header bypasses token verification entirely, so anyone reading the file could act as an operator against any deployment that had not overridden it. A default is worth less than it looks here: the deployment that never set the variable is exactly the one where the default is public. Every credential now comes from `.env`, and compose refuses to start naming the variable it wanted. That also takes the last literal password out of a tracked file. **The origin ports are on loopback.** Both services speak plain HTTP and mark no cookie `Secure`, because both expect to sit behind something that terminates TLS. Published on every interface they were a way to reach the issuer around that proxy, with sign-in codes and tokens in clear text. **Mail settings are passed through rather than fixed.** The issuer was pinned to printing sign-in codes to its log, and the three delivery settings never reached it — so the documented way to configure mail could not work, and every code and recipient went to the container log instead. Printing codes is now asked for in `.env` like everything else, and with nothing configured the issuer refuses to send rather than logging. **Sandbox becomes a domain rather than a prefix.** `api.sandbox.nestri.io` and `auth.sandbox.nestri.io`, because sandbox holds whatever is not production and that set grows. One certificate for `*.sandbox.nestri.io` then covers all of it, including unpredictable per-pull-request names, and cannot be presented for production's own domain — which the zone-wide wildcard the previous shape leaned on could. Also drops `STEAM_API_KEY`. It was declared in two type definitions and read by nothing: linking an account makes no outbound call that needs it. |
||
|
Wanjohi
|
51ababc900 |
feat(deploy): drop the IaC layer, and make both apps runnable as containers
Moving the issuer's state into Postgres removed the last thing that tied either app to one hosting provider. What was left was a deployment tool describing resources that no longer existed — so this replaces it with `wrangler`, which is what actually deploys a Worker, and adds a second way to run each app that involves no provider at all. Each app now has a `wrangler.jsonc` with an environment per stage, and a `Dockerfile` beside it. The handler is the same one in both cases; what differs is only where its settings come from. Two of them gained a second spelling so that nothing has to branch on the runtime: Postgres arrives as a pooled binding or as `DATABASE_URL`, and the route to the issuer is a service binding or `AUTH_INTERNAL_URL`. That last one is new, and it is a split the binding was already making without saying so. `AUTH_ISSUER_URL` has to be the issuer's public name, because it is compared literally against every token's `iss` claim — but the public name is often not routable from inside a deployment. So the name and the route are two settings now rather than one that cannot be both. DNS moves out of code and into `docs/dns.md`, which lists every hostname and what it is for. Six records that change roughly never did not need a tool, and the table outlives whatever is answering the names — which is the point, since some of them will stop being Workers. The sandbox hostnames are hyphenated rather than nested for the same reason: a certificate covering `*.nestri.io` covers one label and not two, so `api-sandbox.nestri.io` can become an ordinary origin later without a certificate having to be ordered for it first. Also drops `EMAIL_DEV_LOG` from committed configuration into `.dev.vars`, which `wrangler deploy` cannot upload. Printing a live sign-in code to a log should not be one forgotten override away from production. |
||
|
Wanjohi
|
3d0dcf3e46 |
feat(auth): move issuer state to Postgres (#324)
Moves the issuer's state out of Cloudflare KV and into Postgres, and
splits it by whether a record actually needs the guarantees a key-value
store cannot give.
## Why
The issuer kept everything behind one `get`/`set`/`remove`/`scan`
interface — which is what a library that has to run on any provider's
cache can offer. Three of the things kept there could not be served by
it:
- **Authorization codes** must be redeemable once. `get` → validate →
`remove` are separate steps, so two exchanges of one code arriving
together both pass every check, and each answer is a complete session.
- **Refresh tokens** must be spendable once. Reuse detection works by
recording *when* a token was first spent, so the check and the record
have to be the same operation. Split apart, two refreshes both look like
the first — and the reuse that reveals a stolen token is never recorded,
because recording it is the write the second caller overwrites.
- **Signing keys** don't race, but they're the one record whose loss
ends every session at once, and a cache is a place things may be evicted
from.
This is the same argument `device_grant` already made, applied to the
records that had it too.
## What's here
| table | why it exists |
|---|---|
| `authorization_code` | redeeming is one `delete … returning` |
| `refresh_token` | spending is one `update … where time_used is null
returning *`; `subject` indexed, so signing out everywhere is a query
rather than a prefix scan |
| `auth_key` | retired by setting `expired_at`, never deleted, so tokens
stay verifiable through a rotation |
| `auth_kv` | everything left: the rate-limit counters |
`auth_kv` stays deliberately generic. Those counters are written far
more often than read, meaningless within the hour, and allowed to be
approximate — a lost increment costs one guess out of ten. It's the one
place an unmigrated `jsonb` blob is the right answer rather than a
shortcut.
Both credential tables store a **hash and never the credential**, as
`device_grant` does. An authorization code travels in a query string, so
it passes through browser history, referrer headers and any log along
the redirect; a refresh token resumes a session outright.
`packages/auth` gains `keyStore`, `codeStore` and `refreshStore` as
optional issuer inputs, each defaulting to a storage-backed shim so the
library behaves exactly as before when they aren't passed — same shape
as the existing `deviceStore`.
## Portability
`AuthStorage` was the only stateful Cloudflare-proprietary primitive in
the control plane. It's gone, so the remaining CF surface is Hyperdrive
(a pooler over a plain `DATABASE_URL`), Workers and DNS. The self-host
story stays one service.
## Two behaviour changes worth reviewing
1. **A code that fails a check is now spent.** `consume` happens before
the redirect-URI, client and PKCE checks, because the operation that
decides which caller gets the code has to be the one that removes it.
RFC 6749 §4.1.2 asks for this, but it is a change: a code presented
wrongly no longer gets a second try.
2. **`legacySigningKeys` is removed.** It read a pre-ES256 `oauth:key`
prefix and stamped every key it returned with a hardcoded expiry of
2025-01-02 — so everything it produced has been expired for over a year,
and it only ever read from the store being left behind.
## ⚠️ Deploying this signs everyone out
The signing keys and refresh tokens live in the KV namespace this
removes. The issuer will start with a fresh key set, so every existing
access token stops verifying and every refresh token is gone. Worth
timing deliberately, or copying the key rows across first if that isn't
acceptable.
## Testing
- `packages/auth` — 66 pass (the issuer flows run through the
storage-backed shims, so the rewrite is behaviour-preserving on the
default path)
- `packages/core` — 130 pass, including 24 new ones
The concurrency claims are tested rather than asserted: five overlapping
`claim`s of one refresh token yield exactly one `fresh` and four
`reused`; five overlapping `consume`s of one code yield exactly one
non-null. Also covered: `LIKE` wildcard escaping in `scan` (keys are
built from email addresses and caller addresses, so `%` and `_` are not
hypothetical), and that `scan(['a'])` no longer reaches into `['ab']`.
Migration `0011` applies cleanly from empty.
## Not in scope
Pre-existing `tsc` errors — JSX config for `packages/auth/src/ui/*.tsx`,
`subject.ts`, `oauth2.ts`, `session.test.ts` — are untouched and left
for their own PR.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR moves issuer state from Cloudflare KV into dedicated PostgreSQL
stores and adds atomic operations for authorization-code redemption,
refresh-token claims, and signing-key bootstrap.
- Authorization codes are hashed and consumed with `DELETE ...
RETURNING`.
- Refresh tokens are hashed and claimed with a conditional atomic
update.
- Signing keys are persisted in PostgreSQL with one live key permitted
per kind.
- Remaining approximate rate-limit state uses a generic
PostgreSQL-backed adapter.
- The changes since the previous review preserve each stored key’s
algorithm and make concurrent key bootstrap converge on one live key.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge; both findings from the previous review are
resolved and no new actionable failure remains.
Concurrent key bootstrap now converges through a partial unique index
and a post-insert reread, while imported keys retain their stored
algorithms. Both previous threads were manually resolved, and the
current code confirms their underlying issues are fixed.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| packages/auth/src/issuer.ts | Routes authorization codes, refresh
tokens, signing keys, and invalidation through specialized stores. |
| packages/auth/src/keys.ts | Preserves stored key algorithms and
rereads the store after bootstrap so concurrent issuers converge. |
| packages/core/src/auth/signing-key.ts | Persists key material and
safely ignores a concurrent insertion that already established the live
key. |
| packages/core/src/auth/authorization-code.ts | Implements atomic,
single-use authorization-code consumption. |
| packages/core/src/auth/refresh-token.ts | Implements atomic
refresh-token claims and subject-wide revocation. |
| packages/core/migrations/0011_auth_state_in_postgres.sql | Adds
PostgreSQL tables and constraints for issuer state, including one live
key per kind. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Issuer[OAuth issuer] --> Codes[Authorization code store]
Issuer --> Refresh[Refresh token store]
Issuer --> Keys[Signing key store]
Issuer --> KV[Generic auth state]
Codes -->|DELETE RETURNING| PG[(PostgreSQL)]
Refresh -->|Conditional claim| PG
Keys -->|Partial unique live-kind index| PG
KV -->|Counters and rate limits| PG
```
<sub>Reviews (2): Last reviewed commit: ["fix(auth): keep one live key
per kind,
a..."](
|
||
|
Wanjohi
|
f64f037574 |
fix(auth): keep one live key per kind, and report a key's own algorithm
Two problems found in review, both in the key store. Nothing stopped a kind from having two live keys, and the bootstrap path walks straight into it: two workers starting against an empty table both find no key and both insert one. From then on each signs and encrypts with its own. That is not the harmless split the comment here claimed — the issuer reaches for a single key rather than the published set when it decrypts a session cookie and when it verifies an access token, so a cookie written by one worker is unreadable to the other and a token minted by one is rejected by the other. It stays silent until someone cannot sign in. A partial unique index over the kind, where the key has not been retired, makes the second insert a dropped write instead. Both workers then read the table again and use the key that won, which is all that matters. The conflict clause stops naming a target: both indexes on the table mean the same thing at this call site, that the row already exists in some form. Creating a key is now attempted once rather than retried, because a store declining the write is an expected answer and spinning on it would hang the request instead of failing it. Separately, a key pair reported the algorithm the issuer currently uses rather than the one stored on the key it was built from, so a retained key would advertise the wrong algorithm in a token header and in the JWKS after a rotation — which defeats keeping it. The material was already being imported with the stored value; only what was handed back disagreed. Retiring a key and creating its replacement now have to happen together, so that a kind never has two live keys and never has none. |
||
|
Wanjohi
|
f25c9af545 |
feat(auth): keep issuer state in Postgres
The issuer kept everything behind one get/set/remove/scan interface, which is what a library that must run on any provider's cache can offer. Three of the things kept there could not actually be served by it. An authorization code must be redeemable once and a refresh token spendable once, and through get and set the check and the write are separate steps — so two requests arriving together both read an unspent record, and both mint a session. In the refresh case that also means the reuse which reveals a stolen token is never recorded, because recording it is the write that the second caller overwrites. Each now has a table and an interface of its own: redeeming is one `delete ... returning`, spending is one `update ... where time_used is null returning *`, so exactly one caller is ever told it went first. This is the same argument the device grant already made, applied to the two records that had it too. Signing keys move for a different reason. Nothing races for them; they are the one record whose loss ends every session at once, and a cache is a place things may be evicted from. They are retired by setting a column rather than deleted, so the tokens they signed stay verifiable until they expire. Both credential tables store a hash and never the credential, as the device grant does. An authorization code travels in a query string and so passes through history, referrer headers and any log along the redirect; a refresh token resumes a session outright. What is left in the generic store is the rate-limit counters — written far more often than read, meaningless within the hour, and allowed to be approximate, since a lost increment costs one guess out of ten. Those move to Postgres too, so the only key-value binding this deploys with is gone and the control plane's state is one database. That was the point: nothing here now depends on a primitive a self-hoster cannot run. The generic scan also gained the separator on its prefix, so scanning `a` cannot return what is under `ab` — subjects and email addresses are both prefixes of longer subjects and email addresses. Deploying this signs everyone out. The signing keys and refresh tokens are in a store that is being left behind, so the issuer starts with a fresh key set and every existing token stops verifying. |
||
|
Wanjohi
|
349305d0cc |
feat(api): hold a run to the attempt that claimed it (#323)
Closes the seam week 2 merged without. The agent sends a claim token on
every
write and this side rejected the field, so **every state report and
every ticket
publish answered 400** — neither end's tests could see it, because each
was
written against the document rather than against the other end.
## What lands
**Both agent bodies take `claimToken`.** That alone is what unblocks the
wire.
**The row remembers which attempt holds it.** Taking a claim requires
there to be
no holder; every write after it requires the caller to *be* the holder.
The guard
is in the `where` clause and not only in the read above it, so two
attempts that
both read an unheld row still leave with one winner.
**A report from an attempt that does not hold the run is 409 whatever
the state
is** — including the state the run is already in. That row is the whole
point: an
agent retrying after a lost response holds the token and is told nothing
broke;
an agent that lost the race does not and is told to stop. Both answers
are
decided by the request rather than by when it arrived.
**The ticket is held to the claim too**, for a worse reason than a
double start.
The client re-reads the address rather than caching it, so a ticket
published by
a losing attempt produces a client that connects, successfully, to a
machine
running nothing. A box started twice is waste and it is visible.
**The holder is never cleared**, including on terminal states, so a
settled claim
cannot be replayed and a finished run still records which attempt ran
it. It is
**not** in what goes out — holding one permits writing to a run, and the
owner
reading their own session is not the holder. There is a test asserting
the whole
response shape, so a column added later has to be added there before it
ships.
## One thing the contract asked for that cannot exist
The spec distinguishes *"claim, row already has a holder → 409"* from
*"report,
token is not the holder → 409"*. **Those are the same case.** Taking the
claim
and leaving `requested` are one write, so a rival never observes a
`requested`
row with a holder — it observes a `starting` row it does not hold. The
answer is
409 either way and nothing is lost, but the branch is unreachable and I
have not
written code pretending otherwise. Worth folding into the document.
## Failing first
The new tests against unmodified source:
```
Expected: 200 / Received: 400 the claim moves the row
Expected: 409 / Received: 400 the same state from a second attempt
Expected: 403 / Received: 400 a different host reporting anything
51 pass, 22 fail
```
Every 400 is the strict validator refusing `claimToken` — the live
break,
reproduced. After:
```
284 pass, 0 fail, 777 expect() calls
```
against a `dev` baseline of **271 pass, 0 fail, 747 expect()** that I
measured
before starting. 13 new tests.
## What this does not verify
- **No agent has ever sent one of these requests.** Both ends are still
held by
tests written against a document. This PR makes the shapes agree by
reading
both, which is the thing rule 1 exists to avoid needing — it is a
repair, not
evidence that the wire works. Only a live round trip settles it.
- **Two attempts racing now happens in the tests, but only in one
process.**
Two tests claim concurrently: one races whole `transition` calls, the
other
fires the guarded updates directly so nothing but the `where` clause can
refuse the second. Both fail with two winners if the check is moved out
of
the write. What they do not reach is two *processes* against a shared
database, which is the real shape — and with one host it cannot happen
in
the field either.
- **Neither guard in that `where` clause is pinned on its own.** The
state
predicate and the holder predicate each cover the other, so removing
either
one alone leaves every test passing. That redundancy is deliberate, but
it
means these tests hold the pair and not the parts.
- **An agent that restarts loses its token**, and is then locked out of
a run it
is still hosting. The host side persists it to disk, which narrows this
a lot,
but nothing on this side can recover from a lost holder and nothing
reaps the
run that results.
- **`min(22)` is not an entropy check.** A caller can present 22
identical
characters and be believed. Nothing on this side can verify randomness.
- The `notHolder` branch of `publishTicket` is reachable only when a run
is past
`requested`; the invariant guard on the claim path is unreachable by
design and
is marked as such rather than tested.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR binds each session run to the agent attempt that claimed it.
- Accepts `claimToken` in state-report and ticket-publish API payloads.
- Atomically records the token during the `requested → starting`
transition.
- Rejects later state or ticket writes from attempts that do not hold
the claim.
- Keeps the token out of serialized session responses.
- Adds route, core, and concurrent PostgreSQL coverage for claim
ownership and guarded updates.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge; the prior concern about concurrent claims
is addressed by tests that execute competing guarded updates against
PostgreSQL.
No actionable new failures or repository-rule violations remain, and the
added concurrent coverage verifies that exactly one attempt can claim a
run while only its token can perform subsequent writes.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| packages/core/src/session/index.ts | Adds atomic claim ownership and
enforces it on subsequent state transitions and ticket publication. |
| apps/api/app/routes/session.ts | Extends strict request schemas with
claim tokens and maps holder conflicts to HTTP 409. |
| packages/core/src/session/session.test.ts | Covers claim persistence,
competing attempts, guarded concurrent updates, ticket ownership, and
response non-disclosure. |
| apps/api/test/session.test.ts | Verifies the claim-token wire contract
and route-level conflict behavior. |
| packages/core/src/examples.ts | Adds a correctly shaped claim-token
example for generated API documentation. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant A as Attempt A
participant B as Attempt B
participant API
participant DB as Session row
par Competing claims
A->>API: report starting + token A
API->>DB: UPDATE WHERE requested AND token IS NULL
and
B->>API: report starting + token B
API->>DB: UPDATE WHERE requested AND token IS NULL
end
DB-->>API: Exactly one guarded update succeeds
API-->>A: moved or conflict
API-->>B: moved or conflict
Note over DB: Winning token remains the holder
A->>API: later state/ticket write + token A
API->>DB: "UPDATE WHERE claimToken = token A"
B->>API: later state/ticket write + token B
API-->>B: 409 Another attempt holds this run
```
<sub>Reviews (2): Last reviewed commit: ["test(core): claim two attempts
at once,
..."](
|
||
|
Wanjohi
|
647e5c5264 |
test(core): claim two attempts at once, not one after the other
The mutual exclusion was asserted only through sequential calls, where the winner had already committed before the rival began. That never reaches the case the design is for: both attempts reading the run as unclaimed before either writes. Two tests, because the first can pass for the wrong reason. The concurrent transitions depend on how the transactions interleave; the paired updates skip the read entirely, so nothing but the predicate in the where clause can refuse the second. Both fail with two winners if the check is moved out of the write and left in the read above it. |
||
|
Wanjohi
|
54d5c81edb |
feat(api): hold a run to the attempt that claimed it
The agent side sends a claim token on every write; this side rejected the field outright, so every state report and every ticket publish answered 400. Both bodies now take it. Underneath that, nothing compared a holder. A run was reachable by any caller on the right machine, and a box names exactly one machine — so two attempts polling the same job presented identical credentials and were told apart only by which one's select landed first. That is timing, not a rule, and no caller could be told which case it was in. The row now remembers which attempt holds it. Taking a claim requires there to be no holder; every write after it requires the caller to be the holder. The same state reported by a different attempt is a lost race and not a retry, and is refused whatever the state is - which is the only thing that separates the two 200s from the 409s. The ticket is held to the claim too, for a worse reason than a double start: the client re-reads the address rather than keeping the first, so a ticket written by a losing attempt produces a client that connects, successfully, to a machine running nothing. The holder is never cleared, including on a terminal state, so a settled claim cannot be replayed and a finished run still records which attempt ran it. It is not in what goes out - holding one permits writing to a run, and the owner reading their own session is not the holder. |
||
|
KAAL1 (Bingus)
|
2faf7d77db |
feat(nesinit): mount what the descriptor names, and relay the layer it cannot read (#320)
Stacked on #319, which has PID 1, the channel and the trait but mounts
nothing.
Review that one first; this PR is the descriptor half.
- **The shares are mounted.** A tag names an export, the descriptor
names where
it lands, and every share goes on `nosuid` and `nodev` whether or not it
is
writable — a share is data handed to the guest, and no descriptor has a
way
to ask for a setuid binary or a device node in one. Mounting needs
privileges a test does not have, so the arguments and flags are derived
by a
function the tests assert; that is where the read-only decision lives.
- **Progress is two messages, not one.** `mounted` / `mount_failed` stay
apart
from `started` / `start_failed`, because a share that did not appear and
a
command that did not run want different things looked at. A failure
carries
the reason the operating system gave, verbatim, and the path it happened
on.
- **The second layer is relayed and never read.** Envelopes cross a unix
socket
to the workload and come back the same way. `body` is a string rather
than
nested JSON on purpose: a document this component can index into is a
document it can grow to depend on, and then the layer is not opaque any
more
and the boundary it exists to draw is gone.
- **An envelope is never logged** — not the body, not truncated, not at
debug
level. The channel name and a byte count are the whole of what may be
said
about one. `Payload`'s `Debug` is written by hand for the same reason.
- **A write to a channel nobody reads now ends the session** the same
way a
closed read does, and stops the workload. A caller that stopped
listening has
also stopped being able to say stop; that was two outcomes and is one.
## The tests, failing first
Progress reporting, with the mount result dropped (the state this branch
started from):
```
running 11 tests
test session::tests::an_unreadable_line_does_not_end_a_session ... ok
test session::tests::a_stop_is_idempotent_and_does_not_end_the_session ... ok
test session::tests::the_guest_speaks_first_and_says_its_version ... ok
test session::tests::a_closed_channel_stops_the_workload ... ok
test session::tests::the_descriptor_mounts_and_starts_what_it_names ... ok
test session::tests::an_envelope_crosses_the_session_in_both_directions_unread ... ok
test session::tests::a_share_that_will_not_mount_is_refused_before_anything_starts ... ok
test session::tests::an_exit_is_reported_and_the_workload_is_not_started_again ... FAILED
test session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount ... FAILED
test session::tests::a_signalled_workload_is_reported_as_signalled ... FAILED
test session::tests::a_relay_nothing_is_on_does_not_end_a_session ... FAILED
failures:
---- session::tests::an_exit_is_reported_and_the_workload_is_not_started_again stdout ----
thread 'session::tests::an_exit_is_reported_and_the_workload_is_not_started_again' (312969) panicked at apps/nesinit/src/session.rs:248:13:
assertion `left == right` failed
left: Started
right: Mounted
---- session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount stdout ----
thread 'session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount' (312963) panicked at apps/nesinit/src/session.rs:460:9:
assertion `left == right` failed
left: StartFailed { reason: "ENOENT: /usr/bin/workload" }
right: Mounted
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- session::tests::a_signalled_workload_is_reported_as_signalled stdout ----
thread 'session::tests::a_signalled_workload_is_reported_as_signalled' (312966) panicked at apps/nesinit/src/session.rs:248:13:
assertion `left == right` failed
left: Started
right: Mounted
---- session::tests::a_relay_nothing_is_on_does_not_end_a_session stdout ----
thread 'session::tests::a_relay_nothing_is_on_does_not_end_a_session' (312964) panicked at apps/nesinit/src/session.rs:248:13:
assertion `left == right` failed
left: Started
right: Mounted
failures:
session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount
session::tests::a_relay_nothing_is_on_does_not_end_a_session
session::tests::a_signalled_workload_is_reported_as_signalled
session::tests::an_exit_is_reported_and_the_workload_is_not_started_again
```
The read-only flag, ignored:
```
running 9 tests
test shutdown::tests::a_workload_that_leaves_in_time_is_not_killed ... ok
test shutdown::tests::the_workload_stops_before_anything_else_and_the_disks_flush_before_power ... ok
test shutdown::tests::a_workload_that_overstays_its_grace_is_killed_and_shutdown_still_finishes ... ok
test workload::tests::a_failure_names_the_path_it_happened_on ... ok
test workload::tests::a_writable_share_is_still_mounted_without_devices_or_setuid ... ok
test workload::tests::a_read_only_share_is_mounted_read_only ... FAILED
test session::tests::a_closed_channel_stops_the_workload ... ok
test session::tests::an_exit_is_reported_and_the_workload_is_not_started_again ... ok
test session::tests::a_signalled_workload_is_reported_as_signalled ... ok
failures:
---- workload::tests::a_read_only_share_is_mounted_read_only stdout ----
thread 'workload::tests::a_read_only_share_is_mounted_read_only' (311963) panicked at apps/nesinit/src/workload.rs:212:9:
assertion `left == right` failed
left: 0
right: 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
workload::tests::a_read_only_share_is_mounted_read_only
```
The relay quoting a line it could not decode — which is the second way a
body
reaches a log line, and the reason the failure path logs a length and
nothing
else:
```
running 2 tests
test payload::tests::an_envelope_crosses_in_both_directions_untouched ... ok
test payload::tests::nothing_the_relay_logs_contains_a_body ... FAILED
failures:
---- payload::tests::nothing_the_relay_logs_contains_a_body stdout ----
thread 'payload::tests::nothing_the_relay_logs_contains_a_body' (312716) panicked at apps/nesinit/src/payload.rs:202:9:
a body reached a log line:
2026-09-04T21:11:56.619025Z WARN nesinit::payload: ignoring an envelope that would not decode bytes=62 line={"channel":"identity","body":"a-credential-nobody-should-read"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
payload::tests::nothing_the_relay_logs_contains_a_body
test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 17 filtered out; finished in 0.06s
```
And the first way: a derived `Debug` instead of the hand-written one.
```
running 6 tests
test lifecycle::tests::a_mount_failure_keeps_its_reason_verbatim ... ok
test lifecycle::tests::a_signalled_exit_is_not_a_zero_exit ... ok
test lifecycle::tests::a_line_round_trips ... ok
test lifecycle::tests::defaults_cover_what_a_caller_may_leave_out ... ok
test lifecycle::tests::an_envelope_does_not_print_its_body ... FAILED
test lifecycle::tests::an_envelope_body_stays_a_string_in_both_directions ... ok
failures:
---- lifecycle::tests::an_envelope_does_not_print_its_body stdout ----
thread 'lifecycle::tests::an_envelope_does_not_print_its_body' (313791) panicked at crates/nesprotocol/src/lifecycle.rs:290:9:
the body reached a log line: Payload { channel: "identity", body: "a-credential-nobody-should-read" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
lifecycle::tests::an_envelope_does_not_print_its_body
test result: FAILED. 5 passed; 1 failed; 0 ignored; 0 measured; 9 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesprotocol --lib`
```
All green after: 19 unit tests, 3 against real forked children, 15 in
`nesprotocol`.
## What this does not verify
- **Nothing has been mounted.** No `virtiofs` share has been mounted by
this
code, in a VM or anywhere else. What is tested is the source, target and
flag
word handed to the mount call; that the call succeeds against a real
virtio
transport, that the mount point is where a workload then finds its
files, and
that a `uid` mismatch surfaces as the permission error this is written
to
produce, are all unverified.
- **The relay has never carried a real workload's traffic.** Two
processes, a
real unix socket and bytes that come back unchanged is what the test
shows.
Whether the socket path is the right mechanism is openly a guess, and it
is
meant to be replaceable without anything above it moving.
- **"Never logged" is enforced more narrowly than it reads.** The
hand-written
`Debug` and the failure path's length-only line are both tested. The
capture
test cannot reliably see debug-level lines: callsite interest is cached
process-wide, so a line another test in the same binary reached first
never
arrives in the capture. A future `{:?}` on a whole envelope at debug
level
would not necessarily be caught by these tests, only by the `Debug` impl
keeping its shape.
- **`geometry` is parsed and carried, and nothing consumes it.** This
component
does not start the guest's own services yet. `ticket` exists as a
message
with no producer wired to it.
- **Still no VM, still no vsock, still no `uid` drop**, as in #319, and
no
number in this PR is measured.
- **No third-party workload has gone through any of this.** The claim
that a
descriptor plus a set of shares is enough to run something we did not
write
is untested, and our own workload is the weakest possible witness for
it.
## Since review
`d4d473f` — the relay may not stall the session and may not buffer
without end,
plus a descriptor with a nul byte in it is refused by name. Failing
first, in
order:
The session held still behind a workload that was not reading:
```
running 1 test
test session::tests::a_relay_that_is_not_draining_does_not_stall_the_session ... FAILED
failures:
---- session::tests::a_relay_that_is_not_draining_does_not_stall_the_session stdout ----
thread 'session::tests::a_relay_that_is_not_draining_does_not_stall_the_session' (326881) panicked at apps/nesinit/src/session.rs:670:10:
the session stalled on the relay: Elapsed(())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
session::tests::a_relay_that_is_not_draining_does_not_stall_the_session
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out; finished in 5.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
A frame with no end to it:
```
running 1 test
test payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest ... FAILED
failures:
---- payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest stdout ----
thread 'payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest' (327888) panicked at apps/nesinit/src/payload.rs:375:10:
the relay is still assembling a frame that never ends: Elapsed(())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out; finished in 5.01s
error: test failed, to rerun pass `-p nesinit --lib`
```
And a tag that was quietly emptied instead of refused:
```
running 1 test
test workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name ... FAILED
failures:
---- workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name stdout ----
thread 'workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name' (327380) panicked at apps/nesinit/src/workload.rs:274:40:
an empty source would have been mounted: ("", "/mnt/user", 6)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
One behaviour changed rather than only hardened, and it is worth a
reviewer's
eye: **nothing is queued for a workload that is not on the relay.** An
envelope
that arrives with nobody connected is dropped, as is one that arrives
faster
than the workload reads. That follows the layer's own rule — what
crosses it is
re-sent when it changes, so a held copy is a stale copy — but it does
mean a
sender that assumes delivery is wrong to. Nothing here retries, and
nothing
tells the far end that a particular envelope was dropped.
23 unit tests, 5 against real forked children, 15 in `nesprotocol`.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR mounts descriptor-defined virtiofs shares, reports mount and
process-start progress independently, and relays opaque payload
envelopes between the host channel and workload Unix socket. Changes
since the previous review also bound relay frames, prevent relay
backpressure from stalling lifecycle handling, reject descriptor strings
containing NUL bytes, and track whether a reaped PID remains valid.
- Mounts shares at descriptor-selected targets with `nosuid`, `nodev`,
and optional read-only flags.
- Adds bidirectional opaque payload forwarding with bounded,
non-blocking queues and body-safe logging.
- Adds distinct mounted/start lifecycle responses and failure reporting.
- Adds capped newline-delimited relay frames and drops stale or
backpressured envelopes.
- Reworks workload tracking to avoid signaling a PID after its exit has
been delivered.
<h3>Confidence Score: 5/5</h3>
The reviewed changes appear safe to merge, with no accepted new findings
or outstanding previous root-thread findings.
The resolved relay-stall, unbounded-frame, and invalid-NUL findings are
addressed by non-blocking delivery, capped frame assembly, and explicit
descriptor validation. The protocol-version concern was correctly
withdrawn under the coordinated version-2 deployment model. The
remaining PID check-to-signal race duplicates an existing prior comment
and therefore is not reposted or counted as a new finding.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nesinit/src/payload.rs | Adds the bounded, bidirectional
Unix-socket payload relay with non-blocking delivery and body-safe
logging. |
| apps/nesinit/src/session.rs | Integrates payload events with lifecycle
handling and separately reports mount and process-start outcomes. |
| apps/nesinit/src/workload.rs | Implements descriptor-driven virtiofs
mounts and switches process signaling to tracked reaper state. |
| apps/nesinit/src/reap.rs | Adds shared reaped-state tracking so
callers stop treating a delivered PID as the workload. |
| crates/nesprotocol/src/lifecycle.rs | Extends lifecycle messages with
mount progress and opaque payload envelopes while redacting payload
bodies from Debug output. |
| apps/nesinit/src/main.rs | Starts the payload relay before the
workload session and wires bounded relay ports into session handling. |
| apps/nesinit/README.md | Documents mount behavior, payload opacity,
delivery semantics, frame limits, and progress reporting. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant H as Host
participant N as nesinit
participant M as virtiofs mounts
participant W as Workload
H->>N: Boot descriptor
N->>M: Mount descriptor shares
M-->>N: Success or failure
N-->>H: mounted / mount_failed
N->>W: Start command
N-->>H: started / start_failed
H->>N: Payload envelope
N-->>W: Non-blocking Unix-socket relay
W->>N: Payload envelope
N-->>H: Payload envelope
W-->>N: Exit
N-->>H: workload_exited
```
<sub>Reviews (3): Last reviewed commit: ["fix(nesinit): the relay may
not stall
th..."](
|
||
|
Wanjohi
|
217e82a9a8 |
feat(auth): email is the root of an account, and Steam is a connection (#318)
## What landed
**Email is the root of an account.** A `user` is created by verifying an
email
address and nothing else. Every Steam account is now a connection
hanging off a
user that already exists, capped at four.
**And it is the only thing that creates one.** Signing in with a gaming
account
or with an SSH key are both unwired from the issuer. Each could mint a
user,
which makes an account only as recoverable as the thing that made it and
gives
one person as many accounts as they have gaming logins. The providers
still
exist under `packages/auth/src/provider/` and can be wired back;
connecting a
Steam account is unaffected, because that runs through `POST
/steam/link` in
`apps/api` against a user who already exists. A test asserts the two
routes are
not served, so they cannot come back quietly.
**The pin-code provider is wired**, and email delivery refuses rather
than
guesses. **The device authorization grant is served**, and it now ends
at a
question somebody has to answer.
## The review found five real defects. All five are fixed, and so are
six more
The first pass of this branch shipped a device flow that handed out
tokens
without asking anybody, an email path whose pin could be guessed
outright, and
three read-then-write races. Each fix below has a test that fails
without it —
verified by reverting the fix and watching the test go red, not by
assertion.
### Signing in was mistaken for saying yes
`GET /device?user_code=…` started a provider flow and provider success
approved
the grant. So the whole attack was: ask for a device code, mail somebody
the
pre-filled link, keep the device code, poll. They see an ordinary
sign-in
prompt, complete it correctly, and you hold their access **and refresh**
tokens.
They were never asked a question, because there wasn't one.
There is now. Signing in establishes who the browser belongs to; it does
not
establish that the person meant to hand an account to a program running
somewhere else. The flow ends at a page that names the client, shows the
user
code back so it can be compared against what the device is displaying,
and
offers Approve and Deny. Approving is a POST carrying a value placed in
the
cookie alongside it, so another site cannot submit it on their behalf.
Denial moved onto that page too. It was a `GET` anybody could fire with
no
authentication: a link scanner or a chat unfurler would cancel real
sign-ins,
and anyone who learned a user code could grief one.
### A six-digit pin with unlimited guesses and a day to use them
The code travelled in an encrypted cookie held by the caller,
verification
compared against that cookie, and a wrong answer re-rendered the form.
Nobody
has to be the person the code was mailed to — type someone else's
address into
the first screen and the code goes to their mailbox while the cookie
stays with
you. At that point the only thing between a stranger and an account was
a
million requests. The constant-time comparison was guarding a door you
could
keep knocking on.
Guesses are counted on the server now, under a name that rotates with
every
code. The placement is the point: a counter kept beside the code, in the
cookie,
is one the guesser winds back by replaying an older copy. Starting over
is still
allowed and still costs a fresh code sent to the mailbox being aimed at,
where
somebody notices. The cookie's twenty-four hour life is ten minutes.
Resend is
spaced and bounded, because it was otherwise a way to mail a stranger as
fast as
requests go out. Both refusals say the same thing, since which one it
was is a
fact about someone else's mailbox.
The user codes on the other side of the flow are rate limited too —
eight
characters over a twenty-five character alphabet is a large space but a
fixed
one, and the endpoint had no opinion about how often you asked.
### Three read-then-write races
- **A poll could erase an approval.** The grant was read, modified and
written
back whole, so a poll that read a pending record and then wrote its
bookkeeping undid an approval that landed in between, leaving the client
polling a dead grant until it aged out. The same window let an approval
overwrite a denial.
- **The connection cap counted nothing.** `select … for update` over the
connections a user already had locks the rows it finds, and finding none
locks
nothing — there are no gap locks under read committed. Six concurrent
links
against a cap of four produced six; the test asserts that.
- **Concurrent email sign-ins returned a driver error.** Two tabs
finishing the
same sign-in both found no user, and the loser got a raw constraint
violation
instead of the account the winner had just made.
The cap now counts under a lock on the account's own row, which is the
one thing
every caller for that account contends on. The email paths let the
unique index
arbitrate and read back what the winner wrote. Device grants moved out
of the
key-value store into a table, where approving is one conditional update
and
redeeming is one delete that returns what it deleted.
### Three more the review did not raise
- **`client_id` was never checked at either end.** Anyone could mint a
grant
naming any client, and any holder of a leaked device code could redeem
it. It
is validated at issue and has to match at redemption.
- **Tokens were minted at approval** and left in storage until
collected, so the
lifetime reported to the client overstated what was left, and a grant
nobody
collected still left a usable refresh token lying around. They are
minted at
redemption.
- **The device code was stored as written.** It is the credential the
tokens are
handed to, so what is kept is now its hash: enough to recognise it, not
enough
to present it.
### And the environment check that decided none of this mattered
Mail delivery threw only when the environment said `production`, and
logged the
recipient and the live code otherwise. The deployment sets no such
marker — see
`alchemy.run.ts` before this change — so production took the developer
branch,
printed every code to a retained log, and reported success while nobody
received
anything.
That is the cost of a fail-open default: the deployment that forgets its
mail
settings is exactly the one with no marker saying it is real, so it gets
the
lenient branch precisely when it should not. Turned around. Printing a
code is
asked for by name; absence of configuration is a refusal; two settings
out of
three is an error rather than a fallback. Stages anyone else can reach
are
checked at deploy time, so a missing setting stops the deploy with the
name of
the variable it wanted.
## Where device grants live, and why it is a table
Short-lived state that would sit happily in a cache, in Postgres anyway.
The
reason is not durability. Every transition has to happen exactly once
while two
parties touch the same record — a browser somebody is clicking through
and a
program polling every few seconds — and a store that can only read and
write
whole records cannot promise that. The key-value store behind the rest
of the
issuer has no compare-and-swap, so on it the poll/approval race and
single
redemption can be narrowed and never closed.
The issuer cannot reach the database, so the store is an interface
(`packages/auth/src/device.ts`) with two implementations: one in memory
for
tests, one in `packages/core/src/auth/device-grant.ts`. Every method is
a single
operation and no caller reads a grant, decides, and writes it back.
Migration
`0010` adds the table. Rows are swept when a grant is created rather
than on a
schedule, since a grant lives ten minutes and that is the only statement
that
adds one.
## `session.claim_token` is here on another lane's behalf
Migration `0009` adds `session.claim_token`, nullable `text`, no default
and no
backfill. **It is not identity work and carries no identity reason** —
do not go
looking for one. It records which attempt holds a session run; the
endpoint that
reads and writes it arrives separately. It is in this migration only
because a
schema change has one owner at a time. The column is declared in
`session.sql.ts` and the snapshot, so the next `drizzle-kit generate`
will not
try to drop it — `0010` was generated clean, which is the proof.
## The tests
```
$ bun test
268 pass
0 fail
738 expect() calls
Ran 268 tests across 21 files.
```
Baseline on `dev` before any of this, measured on the same database:
**198 pass,
0 fail, 531 expect() calls.**
The tests that matter are the ones that would have caught the defects,
so each
was checked by putting the defect back:
| Reverted | What goes red |
|---|---|
| the confirmation step | signing in through the link leaves the grant
pending |
| the guess counter | four tests, including one that replays an older
cookie |
| the lock on the account row | six connections against a cap of four |
| the unique-violation handling | a driver error where a sentence should
be |
| the field-level poll write | an approval erased by a poll behind it |
| the verification rate limit | four tests |
Concurrency is exercised by running the same call several times at once
against
a real database, because run one at a time all of it passes whether or
not any
of the protection exists.
## The migration, against a database built to be awkward
`packages/core/script/verify-migration-0009.sh` builds a database
containing the
rows that make the statements do work — an account with no address, two
accounts
holding one address in different cases, an account already over the cap,
a
soft-deleted row holding a live row's address — applies everything
before `0009`,
applies it, and checks each case. All nineteen checks still pass. The
negative
control still dies where it should: with the de-duplication statement
neutered,
`create unique index` fails on the first duplicate pair.
## What this still does not verify
**1. No real email has ever been sent.** The mailer is tested against a
stubbed
`fetch`: the URL, the bearer header, the body it builds, and that it
refuses
when unconfigured. The body shape follows the common `{from, to,
subject, text}`
convention and may need a field the first real provider wants. This now
fails
closed and the deploy refuses without settings, so the failure mode is
loud
rather than silent — but it is still untested against a provider.
**2. The migration is verified against a database I built, not the one
that
matters.** I do not know whether production has duplicate addresses, how
many
rows carry case or whitespace, or whether any account is already over
the cap.
The fixtures cover those because they are *possible*. Worth three
`select`s
against production before this is applied.
**3. The verification rate limit is approximate.** The counter is in the
key-value store, so a caller spread across a distributed edge can exceed
the
budget somewhat. The number that decides the question is whether
somebody is
working through the code space, and a handful either way does not change
it.
**4. The single-redemption and conditional-transition properties are
held
against Postgres, and the in-memory store is trusted rather than
proved.** The
memory implementation gets its atomicity from nothing suspending inside
a
method, which is true of it and is not a promise the interface makes. It
is for
tests and local runs.
**5. A person who signed up by email carries an empty connected-account
id in
their token.** The subject schema requires the field and an account with
no
connection has nothing to put there, so it gets `''` — the same value a
server-to-server caller has always carried, and the one consumer already
falls
back to `''`. The honest shape is an optional field, and making it
optional
touches `subjects.ts`, the actor model and the API middleware. Flagging
rather
than doing.
**6. The desktop client cannot actually complete this flow yet.** Its
`DeviceCode` struct has no field for the device code, so it has nothing
to poll
with. That is a different lane's file and nothing here touches it, but
the grant
is not reachable end to end until it does.
**7. The four-account cap and the user-code alphabet are asserted, not
measured.** Four comes from a household's size and a screen's width. The
alphabet excludes look-alikes on the same reasoning. No measurement
decides
either, and no test here pretends one does.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes verified email the root account identity, turns Steam into
an attached account connection, adds provider-neutral email-code
delivery, and implements an RFC 8628 device authorization flow backed by
atomic PostgreSQL grant transitions. It also aligns the related
identity, session, and device-grant migrations and adds concurrency and
flow tests.
- Removes Steam and SSH as direct auth-worker sign-in providers.
- Adds email-code account creation and deployment-time mail
configuration checks.
- Adds explicit device approval, denial, polling throttling, client
binding, and one-time redemption.
- Serializes Steam-link cap enforcement and handles concurrent email
uniqueness conflicts.
- Adds and aligns migrations, snapshots, durable device-grant storage,
and `session.claim_token`.
- One non-blocking resend-limit replay issue remains.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge, with one non-blocking email-delivery abuse
limitation that should be hardened.
The prior device-token theft, polling race, Steam-link concurrency,
email uniqueness, and session-schema findings are fixed in the current
code; the five corresponding threads were manually resolved without
explanatory replies. The remaining new issue permits bypassing the
intended email send cap through replay of an older provider cookie, but
the resend interval still bounds its rate and it does not compromise
account authentication.
**Files Needing Attention:** packages/auth/src/provider/code.ts
<details open><summary><h3>Security Review</h3></summary>
The device flow now requires an explicit, CSRF-protected confirmation
and uses atomic, client-bound redemption. One lower-impact abuse issue
remains: replaying an older code-provider cookie can bypass the intended
email send cap.
</details>
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| packages/auth/src/issuer.ts | Adds an explicitly confirmed RFC 8628
device flow with client-bound, one-time token redemption. |
| packages/auth/src/provider/code.ts | Adds server-side attempt and send
accounting, but replacement flows leave earlier cookie-referenced
counters replayable. |
| packages/core/src/auth/device-grant.ts | Implements durable device
grants using atomic conditional approval, denial, polling updates, and
consumption. |
| packages/core/src/user/identity.ts | Makes email identity creation
conflict-aware and serializes Steam-link cap enforcement on the user
row. |
| apps/auth/src/index.ts | Reconfigures the deployed issuer around email
sign-in and PostgreSQL-backed desktop device authorization. |
| alchemy.run.ts | Requires complete mail configuration for permanent
stages and explicitly enables code logging only for ephemeral
development stages. |
| packages/core/migrations/0010_device_authorization_grant.sql | Adds
the device-grant enum, table, and unique indexes in alignment with the
Drizzle model and snapshot. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant D as Desktop client
participant A as Auth issuer
participant B as Browser
participant E as Email provider
participant DB as PostgreSQL
D->>A: POST /device/authorize
A->>DB: Create pending grant
A-->>D: device_code, user_code, interval
B->>A: Enter user_code
A->>E: Send email verification code
B->>A: Verify email code
A-->>B: Display client and user-code confirmation
B->>A: Approve or deny
A->>DB: Atomic terminal transition
loop Until terminal
D->>A: Poll /token
end
A->>DB: Delete-and-return approved grant
A-->>D: Access and refresh tokens
```
<details><summary>Prompt To Fix All With AI</summary>
`````markdown
### Issue 1
packages/auth/src/provider/code.ts:291-297
**Cookie replay bypasses send cap**
Replaying an earlier encrypted provider cookie bypasses `maxSends`. A resend creates a new flow with an incremented counter but leaves the old flow and its lower counter valid, so the old cookie can call `sendCode` again after each resend interval. This permits repeated unsolicited sign-in emails to an attacker-selected address, although the resend interval still limits their rate.
**How this was verified:** The resend check reads only the flow named by the presented cookie, while creating its replacement neither updates nor removes that old flow.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
`````
</details>
<sub>Reviews (3): Last reviewed commit: ["fix(auth): stop a caller
working
through..."](
|
||
KAAL1
|
a94b323edb |
fix(nesinit): the relay may not stall the session, and may not buffer without end
Three problems in the relay, all of them found in review. Handing an envelope over waited for room. That loop also carries stop, shutdown and the workload's exit, so a workload slow to read its own mail — or one that never connected — could hold the lifecycle layer still behind it. It never waits now: an envelope that will not fit is dropped, which costs nothing, because what crosses this layer is re-sent when it changes. Envelopes were queued for a workload that was not there. The queue filled with copies that would be stale by the time anyone connected, and filling it was what stalled the session. Nothing is held while the socket has nobody on it. A frame had no maximum length. The workload can write for as long as it likes without ever sending a newline, and the process assembling that is the one the kernel has been told not to kill, so the memory it takes comes out of everything else in the guest. Past 64 KiB the connection is dropped and the relay waits for the next one; the failure says how long the frame got and nothing about what was in it. Also, a tag or a mount point with a nul byte in it was quietly turned into an empty string, so an unmountable descriptor arrived later as a mount failure about something else, after the mount point had already been created. It is refused by name now, before anything is created. The relay's tests grew a harness that waits for the connection to be carried before sending anything down it, because dropping what arrives with nobody connected made "connected" something a test has to establish rather than assume. |
||
|
KAAL1
|
7b99f49f62 |
feat(nesinit): mount what the descriptor names, and relay the layer it cannot read
Builds on the previous change, which had PID 1, the channel and the trait but mounted nothing. The shares are mounted now: a tag names an export, the descriptor names where it lands, and every share goes on nosuid and nodev whether or not it is writable — a share is data handed to the guest, and no descriptor has a way to ask for a setuid binary or a device node in one. Mounting needs privileges a test does not have, so the arguments and flags are derived by a function the tests can assert, which is where the read-only decision lives. Progress is reported in two messages rather than one. A share that did not mount and a command that did not run are not the same incident, and each carries the reason the operating system gave and the path it happened on: a permission error on a named directory can be acted on, where "the share did not mount" cannot. The second layer is relayed and never read. Bytes arrive on the channel in an envelope, cross a unix socket to the workload, and come back the same way. The body is a string rather than nested JSON on purpose: a document this component can index into is a document it can grow to depend on, and then the layer is no longer opaque and the boundary it exists to draw is gone. An envelope is never logged — not the body, not truncated, not at debug level — and the channel name with a byte count is the whole of what may be said about one. The type's Debug is written by hand for the same reason, because a derived one puts the body one careless format string away from a log line. A write to a channel nobody is reading now ends the session the same way a closed read does. A caller that has stopped listening has also stopped being able to say stop, which is one situation and was two outcomes. The guest listens on the relay socket and the workload dials in, which is the convention the other guest sockets already use and removes the startup ordering problem: a workload that is not running yet has simply not connected yet. |
||
|
KAAL1 (Bingus)
|
736c0013e9 |
feat(nesinit): PID 1 for a box — reaping, ordered shutdown, and one channel out (#319)
PID 1 inside a box. A microVM has no init unless something is it, and
three of
the jobs belong to nothing else in the guest: reaping whatever the
workload
orphans, turning a signal into an ordered shutdown, and holding the
guest end
of the one channel out.
- `reap.rs` — reaping, plus the subreaper bit and taking init out of the
OOM
killer's reach.
- `shutdown.rs` — the order, behind a trait so it can be asserted
without a VM:
workload first and alone, then everything else, then flush, then power
off.
- `session.rs` — the exchange. The guest speaks first with its protocol
version, is handed one boot descriptor, reports, and stops. Generic over
the
byte stream, so the whole protocol is testable over an in-memory pipe.
- `workload.rs` — one trait the descriptor drops into, a real process
behind
it, and a double.
- `nesprotocol::lifecycle` — the types, behind a feature that is off by
default so the media components keep building without serde.
It reports and does not supervise: when the workload ends, the exit goes
up the
channel and the session is over. Nothing here restarts anything.
**Mounting is not implemented in this PR.** The descriptor's `mounts`
are
refused rather than ignored, and the next PR in the stack implements
them.
### On the OOM killer
`refuse_oom_kill()` writes `-1000` to this process's own
`oom_score_adj`, and
that part is init's job: everything else in the guest exiting is a
message up
the channel, whereas init exiting takes the channel with it, and the far
end
sees a box that stopped answering for no stated reason.
The rest is the image's job and cannot be done from here. Making the
workload
the *preferred* victim means scoring processes this component did not
start, so
the image has to leave the workload's score at or above the default and
must
not lower it for the guest's own services either.
## The tests, failing first
Reaping, with `reap_exited()` returning nothing:
```
running 3 tests
test a_child_that_exits_is_reaped_with_its_code ... FAILED
test a_child_that_is_killed_is_reaped_as_signalled ... FAILED
test an_orphan_is_reaped_by_whoever_inherits_it ... FAILED
failures:
---- a_child_that_exits_is_reaped_with_its_code stdout ----
thread 'a_child_that_exits_is_reaped_with_its_code' (304365) panicked at apps/nesinit/tests/reaping.rs:52:32:
the child was left a zombie
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- a_child_that_is_killed_is_reaped_as_signalled stdout ----
thread 'a_child_that_is_killed_is_reaped_as_signalled' (304366) panicked at apps/nesinit/tests/reaping.rs:66:32:
the child was left a zombie
---- an_orphan_is_reaped_by_whoever_inherits_it stdout ----
thread 'an_orphan_is_reaped_by_whoever_inherits_it' (304367) panicked at apps/nesinit/tests/reaping.rs:98:5:
the child was left a zombie
failures:
a_child_that_exits_is_reaped_with_its_code
a_child_that_is_killed_is_reaped_as_signalled
an_orphan_is_reaped_by_whoever_inherits_it
test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 15.02s
```
Shutdown, with one signal to everything instead of an order:
```
running 3 tests
test shutdown::tests::a_workload_that_leaves_in_time_is_not_killed ... ok
test shutdown::tests::a_workload_that_overstays_its_grace_is_killed_and_shutdown_still_finishes ... FAILED
test shutdown::tests::the_workload_stops_before_anything_else_and_the_disks_flush_before_power ... FAILED
failures:
---- shutdown::tests::a_workload_that_overstays_its_grace_is_killed_and_shutdown_still_finishes stdout ----
thread 'shutdown::tests::a_workload_that_overstays_its_grace_is_killed_and_shutdown_still_finishes' (304568) panicked at apps/nesinit/src/shutdown.rs:105:79:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- shutdown::tests::the_workload_stops_before_anything_else_and_the_disks_flush_before_power stdout ----
thread 'shutdown::tests::the_workload_stops_before_anything_else_and_the_disks_flush_before_power' (304569) panicked at apps/nesinit/src/shutdown.rs:80:9:
assertion `left == right` failed
left: ["signal_rest", "kill_rest", "flush_disks", "power_off"]
right: ["signal_workload", "await_workload", "signal_rest", "kill_rest", "flush_disks", "power_off"]
failures:
shutdown::tests::a_workload_that_overstays_its_grace_is_killed_and_shutdown_still_finishes
shutdown::tests::the_workload_stops_before_anything_else_and_the_disks_flush_before_power
test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 8 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
The handshake, with the version left where it was before the bump:
```
running 1 test
test session::tests::the_guest_speaks_first_and_says_its_version ... FAILED
failures:
---- session::tests::the_guest_speaks_first_and_says_its_version stdout ----
thread 'session::tests::the_guest_speaks_first_and_says_its_version' (304772) panicked at apps/nesinit/src/session.rs:180:9:
assertion `left == right` failed
left: Ready { protocol_version: 1 }
right: Ready { protocol_version: 2 }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
session::tests::the_guest_speaks_first_and_says_its_version
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 10 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
And the property most likely to be quietly regressed later — a
supervisor loop
that restarts what it started, instead of reporting:
```
running 1 test
test session::tests::an_exit_is_reported_and_the_workload_is_not_started_again ... FAILED
failures:
---- session::tests::an_exit_is_reported_and_the_workload_is_not_started_again stdout ----
thread 'session::tests::an_exit_is_reported_and_the_workload_is_not_started_again' (304968) panicked at apps/nesinit/src/session.rs:237:9:
assertion `left == right` failed: an exit is reported, never restarted
left: 2
right: 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
session::tests::an_exit_is_reported_and_the_workload_is_not_started_again
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 10 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
All green after: 11 unit tests, 3 against real forked children, plus 3
new in
`nesprotocol`.
## What this does not verify
- **Nothing has run in a VM, and nothing has run as PID 1.** Orphan
reparenting is exercised with `PR_SET_CHILD_SUBREAPER` in a test
process,
which is the same kernel mechanism but not the same privilege.
- **No real vsock connection has been made.** The protocol is tested
over an
in-memory pipe; the dial itself — the address, the port, a listener that
is
not there — is unexercised, and the deliberate no-retry behaviour has
never
met a refused connection.
- **The real shutdown is untested.** The order is asserted through a
double;
`kill(-1)`, `sync` and the power-off call themselves need a guest, and a
test
process must not make them.
- **The OOM write is unchecked.** It warns and continues where there is
no
procfs, and nothing here confirms the kernel honoured the score.
- **Dropping to `uid`/`gid` before exec is unexercised** — it needs
privileges
a test does not have, so the `pre_exec` path has run in no test.
- **No number here is measured.** Nothing in this PR claims a timing, a
rate
or a count from hardware.
- **A version mismatch is not refused by this end.** The guest announces
its
version in its first line and the far end compares; as the layer stands
there
is nothing for the guest to compare against, so "both ends refuse on
mismatch" is only half-implementable. Worth settling in the channel's
specification before either end grows a second version, and I would
rather
raise it than invent a message for it here.
## Since review
`cb8f37a` — three fixes, all from the review, described in its message.
The one
worth naming here is that the reaper is now the only thing in the
component
that calls `wait`: it hands each exit to whoever asked for that pid, and
registering interest holds the same lock the delivery takes, so an exit
that
happens before its caller is registered is delivered rather than
dropped.
There is a test for exactly that, and it fails without the lock:
```
running 1 test
test an_exit_that_happens_before_the_caller_is_registered_is_not_lost ... FAILED
failures:
---- an_exit_that_happens_before_the_caller_is_registered_is_not_lost stdout ----
thread 'an_exit_that_happens_before_the_caller_is_registered_is_not_lost' (321463) panicked at apps/nesinit/tests/reaping.rs:171:9:
the exit was dropped on the way through
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
an_exit_that_happens_before_the_caller_is_registered_is_not_lost
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 4 filtered out; finished in 5.20s
error: test failed, to rerun pass `-p nesinit --test reaping`
```
Also, and still not verified: the ordered shutdown's real calls now
include
signalling and waiting for one pid rather than every child, and none of
that
has run in a guest either.
`071241f` — a pid stops being the workload's the moment it is reaped, so
a stop
or a kill during shutdown can no longer land on whatever the kernel gave
that
number to next. One window is left, between the reap and the delivery,
and
closing it needs a handle the kernel keeps rather than a number — noted
below
rather than papered over.
- **A pid is still a number here.** Between a reap and the exit being
handed
on, a freed pid is briefly treated as the workload's. Nothing has hit
that
window, and nothing can until a guest recycles pids under load; a
`pidfd`
would remove the class of bug rather than narrow it.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR adds `nesinit`, a guest PID 1 implementation that reaps orphaned
children, exchanges lifecycle messages over vsock, launches one
workload, and performs ordered shutdown.
- Adds a centralized child reaper and waiter registry.
- Adds workload launch, identity changes, exit reporting, and explicit
rejection of unsupported mounts.
- Adds the lifecycle protocol behind an optional `nesprotocol` feature.
- Adds ordered workload-first shutdown and associated tests.
- Since the previous review, tracks whether the watched workload PID is
still live to reduce stale-PID signaling risk.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge based on the accepted findings and the
resolved state of all previous review threads.
No new actionable finding remains after excluding the stale-PID race as
a duplicate of a manually resolved previous thread and confirming that
the stale state left by the shutdown-only wait path is not subsequently
used to signal a workload PID. Previous thread PRRC_kwDOLnCyk87q1Eew was
manually resolved without explanation.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nesinit/src/reap.rs | Adds centralized child reaping, waiter
registration, exit delivery, and shared workload-running state. |
| apps/nesinit/src/workload.rs | Implements workload launch and
signaling through a watched PID whose delivered exit marks it inactive.
|
| apps/nesinit/src/main.rs | Wires the reaper, vsock session, workload
handle, and real ordered-shutdown operations together. |
| apps/nesinit/src/session.rs | Implements the versioned one-descriptor
lifecycle exchange and reports one workload exit without restarting it.
|
| apps/nesinit/src/shutdown.rs | Encodes and tests workload-first
shutdown followed by guest services, disk flush, and power-off. |
| crates/nesprotocol/src/lifecycle.rs | Defines the feature-gated
lifecycle protocol types shared by the guest and host. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[nesinit starts] --> B[Become subreaper and adjust OOM score]
B --> C[Connect to host over vsock]
C --> D[Send protocol version]
D --> E[Receive boot descriptor]
E --> F[Start workload and register PID]
F --> G{Session event}
G -->|Workload exits| H[Reaper delivers exit]
H --> I[Report workload exit]
G -->|Stop or channel closes| J[Signal workload]
G -->|Shutdown or session ends| K[Ordered shutdown]
I --> K
J --> K
K --> L[Stop workload]
L --> M[Stop remaining processes]
M --> N[Sync disks]
N --> O[Power off]
```
<sub>Reviews (3): Last reviewed commit: ["fix(nesinit): a pid stops
being the
work..."](
|
||
|
Wanjohi
|
304bb1f2ef |
fix(auth): count sign-in codes against the mailbox, not the browser
The cap on how many codes a sign-in could ask for was held per attempt, keyed by a value in the caller's own cookie. That bounds nothing. The caller decides how many attempts to start, and starting a fresh one costs them a discarded cookie — so either replaying an older cookie or simply beginning again walked straight around it, and the only thing left spacing the mail out was the interval between sends. The count now sits against the claim, over a window. That is the thing being protected: the mailbox belongs to somebody who did not ask to hear from us, and whoever is pointing at it is not the party to trust with the tally. A resend also left the previous code live, with a budget of guesses of its own. Several resends therefore meant several working codes and several times the chances at them, which made asking for a new code the cheapest way to buy more tries at the old one. A new code now retires the one before it. Reported against the replay path. The replay was real and the same hole was wider than that: starting a new attempt needed no replay at all. |
||
|
KAAL1
|
071241f944 |
fix(nesinit): a pid stops being the workload's the moment it is reaped
A pid is only a name for a process until that process is reaped; after that the kernel may hand the same number to something else. The handle kept the number, so a stop or a kill issued during shutdown — which every session outcome reaches — could land on a process nobody meant, and the one aimed at the workload would have been a SIGKILL. The registry now clears the flag as it delivers the exit, in the same call, and nothing signals a pid whose flag is down. Waiting for the workload on the way out takes the same answer: already reaped is already gone. There is a window left, between the reap and the delivery, and closing it entirely needs a handle the kernel keeps rather than a number. Recorded rather than papered over. |
||
|
Wanjohi
|
fc825f5219 |
fix(auth): stop a caller working through the user code space
A user code is eight characters from a twenty-five character alphabet, which is a large space but a fixed one, and the endpoint that checked them had no opinion about how often you asked. That is the guessing attack RFC 8628 section 5.2 asks implementations to limit, and nothing here did. Wrong codes are now counted per caller address over a rolling window, and the endpoint stops answering once the budget is gone. Getting a code right is not charged for, so somebody who mistypes once and then succeeds is not walking towards a lockout. A caller whose address cannot be established shares one bucket with every other such caller, which makes stripping the headers that say where you are buy a smaller budget rather than an unlimited one. The counter lives in the general-purpose store and is approximate. The number that decides this is whether somebody is working through the code space, and a handful either way does not change that answer. |
||
|
Wanjohi
|
355d1492d9 |
fix(auth): give a sign-in code a budget of guesses and a short life
A six-digit code has a million values, and nothing was counting how many of them a caller tried. The code travelled in an encrypted cookie the caller held, verification compared against that cookie, and a wrong answer simply re-rendered the form. Nobody has to be the person the code was mailed to: type somebody else's address into the first screen and the code goes to their mailbox while the cookie stays with you. At that point the only thing between a stranger and an account is a million requests, and the constant-time comparison protecting the code was guarding a door you could just keep knocking on. Guesses are now counted on the server, under a name that changes with every code. That placement is the point: a counter kept beside the code, in the cookie, is a counter the guesser can wind back by replaying an older copy. Starting over is still allowed and still costs a fresh code sent to the mailbox being aimed at, which is where somebody notices. A correct code spends its record too, so its remaining guesses do not carry into the next one. The cookie also lived for twenty-four hours, which made the pin a password with a million possible values and a day to try them. Ten minutes now, and the code stops being accepted when the clock says so rather than when the cookie happens to go away. Resend had no limit either, so the button was a way to mail a stranger as fast as requests go out. Codes to one address are spaced, and one attempt at signing in can only ask for so many. Both refusals say the same thing on purpose. Which of the two it was is a fact about somebody else's mailbox. |
||
|
Wanjohi
|
36179150a1 |
fix(auth): make a device sign-in an answer somebody gave
Anybody could ask for a device code and be handed a link with the user code already in it. Following that link started a sign-in, and finishing the sign-in approved the grant. So sending somebody the link was enough: they saw an ordinary sign-in prompt, completed it, and whoever kept the device code polled and collected their access and refresh tokens. The victim never saw a question, because there was not one. There is now. Signing in says who the browser belongs to; it does not say the person meant to hand an account to a program somewhere else. Those are two questions and only the second authorizes anything, so the flow ends at a page that names the program, shows the code back so it can be compared with what the device is displaying, and offers Approve and Deny. Approving is a POST carrying a value from the cookie, so another site cannot submit it on somebody's behalf. Denial moved onto the same page: it used to be a GET anyone could fire, which meant a link scanner could cancel a real sign-in and a stranger with a user code could grief one. Three more things that were wrong underneath. The grant was read, modified and written back as a whole record. A poll that read a pending grant and then wrote its bookkeeping erased an approval that landed in between, and the client polled a dead grant until it expired. Grants moved to a table, where approving is one conditional update and redeeming is one delete that returns what it deleted, so neither party can undo the other and two polls cannot both be served. Tokens were minted when the person clicked and left sitting in storage until collected. They are minted at redemption now, so the lifetime the client is told about starts when it receives them, and a grant nobody collects leaves no usable refresh token behind. The client identifier was never checked, at either end. It is validated when the grant is created and has to match when the code is redeemed — without that, a leaked code is redeemable by anyone, and the identifier the token carries is whatever the last caller claimed. The device code is also stored as a hash now, since it is the credential the tokens are handed to. The store is an interface because the issuer cannot reach the database, and because the guarantees are the point: every method is one operation, and no caller reads a grant, decides, and writes it back. |
||
|
Wanjohi
|
15f8d3eb34 |
fix(core): hold the account rules when two requests arrive together
Three rules here are enforced across a lookup and then a write, and each was only as good as whatever stopped the two from interleaving. Nothing did. The connection cap counted with `select ... for update` over the connections a user already had. That locks the rows it finds, and when it finds none it locks nothing — there are no gap locks under read committed — so several first-time links all counted zero and all inserted. Six concurrent links against a cap of four produced six. The count now happens under a lock on the account's own row, which is the one thing every caller for that account is guaranteed to contend on. Creating an account from a verified address looked the address up and then inserted. Two tabs finishing the same sign-in both found nothing, and the loser got the driver's constraint violation instead of the account the winner had just made. The unique index is the thing that actually arbitrates, so the loser now reads back what the winner wrote. Claiming an address on an older account had the same shape and now gives the same sentence a screen would have shown a moment earlier. The tests run each call several times at once against a real database, because run one at a time all three pass whether or not any of this exists. |
||
|
KAAL1
|
cb8f37a0e4 |
fix(nesinit): one thing reaps, and the workload stops alone
Three problems in the shutdown and reaping paths, all of them found in review. Reaping and waiting cannot be two mechanisms. `waitpid(-1, ...)` collects any child, so the reaper and a caller waiting on its own child race for the same status, and whichever loses gets nothing — losing the workload's exit, which is the one thing this component exists to report. The reaper is now the only waiter and hands each exit to whoever asked for that pid. Registering interest holds the same lock the delivery takes, so a child that exits before its caller is registered is still delivered rather than dropped; there is a test that fails without that. Killing the workload killed everything. `kill(-1, SIGKILL)` is every process init may signal, so a workload that overstayed its grace period took the guest's services with it, before the ordered stop the shutdown promises them had even started. It signals the one pid now. The shutdown had no workload to stop. It built a fresh handle with no pid, so the graceful stop was a no-op and the workload only died in the sweep that follows — which is exactly the order this was written to avoid. The handle the session used is now the handle the shutdown uses, and waiting for the workload waits for that pid rather than for any child to leave. |
||
|
Wanjohi
|
2c4e9d9b0b |
fix(auth): refuse to send a sign-in code rather than log one
The rule was "throw when the environment says production, otherwise log the code and carry on". The deployment sets no such marker, so the branch that ran was the developer one: every recipient and every usable sign-in code printed to a retained log, the screen reporting success, and nobody receiving anything. That is what a fail-open default costs. The deployment that forgets its mail settings is exactly the deployment with no marker saying it is a real one, so it takes the lenient branch precisely when it should not. Turned around: printing a live code is asked for by name and anything else is an error, so absence of configuration is a refusal instead of an assumption. Two settings out of three is also an error now, because it means somebody is halfway through wiring a provider up and a quiet fallback would hide the missing half. Stages anyone else can reach are checked at deploy time, so a missing setting stops the deploy with the name of the variable it wanted rather than surfacing later as a person waiting for mail that never comes. |
||
|
Wanjohi
|
affe1e3c73 |
refactor(auth): serve one provider, and make it the email one
Signing in with a gaming account or with an SSH key could both bring a user into existence. That makes an account only as recoverable as the thing that created it, and gives one person as many accounts as they have gaming logins — neither of which is what an account is supposed to be now that verifying an address is what creates one. Both are unwired rather than deleted. The provider implementations stay where they are, because connecting a gaming account is still something this product does; it just does it from the API, against a user who already exists, which is a connection hanging off an identity rather than an identity of its own. The worker test followed: it exercised the two flows that are gone, and now covers the one that is left plus an assertion that the other two are not routed, so they cannot come back quietly. |
||
|
KAAL1
|
a461cbafa5 |
feat(nesinit): PID 1 for a box — reaping, ordered shutdown, and one channel out
A microVM has no init unless something is it, and three of the jobs belong to nothing else in the guest: reaping whatever the workload orphans, turning a signal into an ordered shutdown, and being the guest end of the one channel out. None of it knows what it is running. The guest dials out on a fixed vsock port, says its protocol version first, is handed one boot descriptor — a command line, shares, output geometry, and what an exit means — and carries that out. There is no code path that branches on which workload started, which is the property the component exists to keep. It reports and does not supervise. When the workload ends, the exit goes up the channel and the session is over; `on_exit` says what the exit means, and starting something again is a decision for the end that can see whether restarting is repair or a loop. A signalled workload is reported as signalled with no exit code, because reporting 0 for a killed process makes a kill look like a clean run. Two seams keep this testable without a VM, which is the reason for both of them. Reaping runs against real forked children, with the subreaper bit making a test process inherit orphans the way PID 1 does. The channel is generic over the byte stream, so the exchange is driven over an in-memory pipe — the transport contributes nothing to the protocol beyond ordering and framing. The lifecycle types live in nesprotocol behind a feature, off by default: both ends of the channel read one definition and cannot drift from it silently, while the media components keep building without serde. Mounting shares is not implemented in this build. The descriptor's mounts are refused rather than ignored — a workload started without the shares it was promised fails later, somewhere else, for a reason nobody can see from here. |
||
|
Wanjohi
|
bd163392ca |
fix(core): declare the claim column the migration adds
The migration adds session.claim_token, but neither the schema nor the
snapshot knew about it. Nothing breaks today because the two agree with
each other; it breaks the moment someone declares the field, because
generate then diffs against a snapshot without it and emits
ALTER TABLE "session" ADD COLUMN "claim_token" text;
which fails on every database the migration has already run against.
Declared with no writer yet, so the schema, the snapshot and the
database say the same thing.
|
||
|
Wanjohi
|
1b61d2251e |
fix(core): hold the connection cap on the path a settings screen uses
Connecting a Steam account wrote the row itself, so the limit on how many one person may connect was enforced on the sign-in path and nowhere else — and this is the path the settings screen calls, which makes it the one that would have gone over. It now resolves who is asking and hands over to the single place the rule lives. Two things fall out of that. A Steam account already connected to somebody else is a conflict rather than a silent success returning the other person's row id, and a Steam id of the wrong shape is refused before a lookup. |
||
|
Wanjohi
|
96b0cf8111 |
feat(auth): sign in with an email address
Wires the pin-code provider, which existed and was never reachable, and makes it the only branch that can create an account. Steam now resolves an existing connection instead of minting a user from a persona, and refuses when there is no account behind it — which is an answer the interface renders rather than an implicit signup. Delivery is a small provider-neutral POST rather than a vendor SDK: configure an endpoint, a key and a from address. With none of them set it logs the code outside production so a local sign-in works, and throws in production, because a screen that says "check your email" when nothing was sent leaves someone waiting instead of telling anybody. A person who has only ever signed in by email has no connected account, and the token says so with an empty value — the same one a server-to-server caller has always carried. |
||
|
Wanjohi
|
2a1b7abe9a |
feat(auth): serve the device authorization grant
A program with no browser — the desktop app — had a client for RFC 8628 and nothing to point it at. This serves the other half: a device authorization request that hands back a code, a page a person enters that code on, and a token endpoint that answers the poll. Both of the paths the client already implements are now reachable. Polling faster than the advertised interval gets slow_down, and each warning widens the interval so ignoring one costs more than the last; refusing gets access_denied, so a request nobody started stops instead of being polled until it ages out. The interval is capped, because it only ever grows and a code has to stay pollable for the whole of its life. The codes live in the same storage as the other short-lived grants rather than in a table, since that is what they are. User codes are drawn from an alphabet with no vowels and no look-alike pairs, and are accepted back in whatever case and spacing a person retyped them in. |
||
|
Wanjohi
|
1e81a8f92d |
feat(core): make one address one account, on rows that never had one
Runs against a database where every user was created by a gaming sign-in, so most rows have no email at all and nothing has ever stopped two rows from sharing one. The address is normalized first, duplicates are separated before the unique index exists — the older row keeps the address, the newer one is asked for a new one and loses nothing else — and the index is partial so that accounts with no address do not collide with each other. Verified against a database built to contain the awkward rows rather than against an empty schema, by the script alongside it: an account with no address, one with both, one with two connections, a duplicated address in two different cases, an account already over the connection cap, and a deleted row holding an address a live row also holds. Removing the de-duplication makes the index creation fail, which is how we know the fixtures are load-bearing. Also adds a nullable column recording which attempt holds a session run. It is not part of the change above and carries no reason of its own; the endpoint that reads and writes it arrives separately, and it is here because a schema change has one owner at a time. |
||
|
Wanjohi
|
da65cca4f2 |
feat(core): an account is an email address, and Steam is a connection
Signing in with Steam used to create the account. That made a second Steam account a second person, and it made losing a Steam account lose everything attached to it — the boxes, the team, the billing history. Invert it. A user comes into existence by verifying an email address and nothing else; a Steam account hangs off a user that already exists, capped at four. Signing in with Steam resolves an account and refuses when there is none, so the accounts made before this keep working — they already have the connection this looks for — while nothing new is created behind a persona. The cap lives here rather than in the schema because a unique index cannot count the rows sharing a foreign key. The email column gains a partial unique index instead, which is the constraint that can be expressed, and the address is trimmed and lower-cased at the edge so two spellings are not two accounts. |
||
|
Wanjohi
|
4eff67a11a | fix(core): point the placement marker at the decision that covers placement | ||
|
Wanjohi
|
ecebc528ae |
feat(api): the session endpoint, and a claim that only one caller can win (#317)
## What this is
`POST /session`, `GET /session/:id`, and the three endpoints the host
agent
calls: `GET /machine/jobs`, `POST /session/:id/state`, `POST
/session/:id/ticket`.
Core had `Session` and `Box` and no HTTP surface at all; this adds the
surface
and the two things core was missing to support it safely.
### The access rule
An agent may only see or touch a run whose box is placed on its own
hardware,
and that is a `where` clause on all three agent endpoints — not a check
sitting
next to the query, and not the agent asking politely for its own work.
Host
credentials are long-lived secrets on hardware in somebody's living
room, so
the blast radius of one leaking is decided by the join and nowhere else.
`Session.listJobsForMachine`, `forMachine`, `compareAndSetState` and
`publishTicket` all scope to the machine inside the statement. The
compare-and-set is scoped there too, and not only by the read above it:
a
caller checking first and the write being scoped are different
properties, and
only one of them survives somebody adding a second caller.
"No such run" and "not your run" are the same 403 with the same body, so
reporting states at ids cannot be used to discover which ids exist.
### The claim
`Session.setState` updated on the id alone. Two agents polling the same
run
both succeeded and both started the same box. There is one host today,
which
is exactly why that would have been built wrong and stayed wrong.
`Session.compareAndSetState` puts the state being moved *out of* into
the
`where` clause, so the database picks the winner and the loser matches
zero
rows. `Session.transition` layers the classification on top and returns
one of
five outcomes, which the route maps:
| Case | Answer |
|---|---|
| same host re-reports a state it already reported | `200`, nothing
changes |
| a different host reports anything | `403` |
| a transition not in the table | `409`, row does not move |
| a legal transition something else won first | `409` |
| it happened | `200` |
`setState` is left exactly as it was — it is the unscoped primitive the
core
tests already pin, and the new path is additive.
### Placement
`POST /session` makes no placement decision. A box names its hardware,
so a
run inherits it by join, and the request body is `.strict()` so that
naming a
machine there is a validation error rather than a field quietly ignored.
The interface went where boxes are actually placed: `Placement.Placer`
in
`packages/core/src/box/placement.ts`, with `Placement.onlyHost` as the
implementation and `Box.createPlaced` as the one caller. A real
scheduler
replaces that file and nothing else.
## The test failing first
Tests were written and run against unmodified code. Verbatim, trimmed to
the
result lines (the full log is stack traces for the same failures):
```
(fail) POST /session > a request creates the job, in the envelope both ends read [199.00ms]
(fail) POST /session > creating a session makes no placement decision [55.00ms]
(fail) POST /session > a box somebody else owns is not there to run [106.00ms]
(fail) POST /session > a box already running refuses a second run rather than picking one [51.00ms]
(fail) POST /session > you can only play as an account you have linked [85.00ms]
(fail) POST /session > a host cannot ask for a session on its owner’s behalf [54.00ms]
(fail) POST /session > requesting a session requires a signed-in person [1.00ms]
(fail) GET /session/:id > the owner reads their own run, ticket and all [47.00ms]
(fail) GET /session/:id > somebody else’s run is not visible, and neither is its absence [97.00ms]
(fail) GET /session/:id > reading a run requires a signed-in person [1.00ms]
(fail) GET /machine/jobs > a host is handed the work for its own boxes, with the kind on the wire [50.00ms]
(fail) GET /machine/jobs > a host never sees work for a box on other hardware [84.00ms]
(fail) GET /machine/jobs > bad credentials are indistinguishable from none [41.00ms]
(fail) GET /machine/jobs > a person cannot poll for jobs [44.00ms]
(fail) POST /session/:id/state > the claim moves the row, and the job stops being offered [41.00ms]
(fail) POST /session/:id/state > the same host re-reporting a state it already reported is fine [54.00ms]
(fail) POST /session/:id/state > a different host reporting anything is refused, and learns nothing [99.00ms]
(fail) POST /session/:id/state > a transition that is not allowed is a conflict, and the row stays put [47.00ms]
(fail) POST /session/:id/state > a stopped run cannot be started again [47.00ms]
(fail) POST /session/:id/state > a duplicate live report does not extend a run somebody is billed for [44.00ms]
(fail) POST /session/:id/state > a state nobody defined is a validation error, not a conflict [49.00ms]
(fail) POST /session/:id/state > a person cannot report a state on their own session [48.00ms]
(fail) POST /session/:id/ticket > a later ticket replaces the first, because it is a better address [44.00ms]
(fail) POST /session/:id/ticket > a different host cannot publish an address for someone else’s run [75.00ms]
(fail) POST /session/:id/ticket > a stopped run has no address to publish [48.00ms]
(fail) POST /session/:id/ticket > a ticket has to say something [47.00ms]
(fail) Session routes in the spec > every path a caller needs is documented [48.00ms]
error: Cannot find module './placement.js' from '/home/…/packages/core/src/box/placement.test.ts'
(fail) Session jobs > a requested session is the job, and it carries its kind [37.00ms]
(fail) Session jobs > a job belongs to the machine its box is placed on and to no other [71.00ms]
(fail) Session jobs > only a requested session is work; a claimed one is not offered again [34.00ms]
(fail) Session claim > the claim is a compare-and-set, so the second attempt finds nothing to move [37.00ms]
(fail) Session claim > a machine that is not the box’s host cannot move the row [72.00ms]
(fail) Session claim > a session that does not exist is refused the same way as one that is not yours [31.00ms]
(fail) Session claim > re-reporting the state you already reported changes nothing [37.00ms]
(fail) Session claim > a transition off the table is refused and the row does not move [36.00ms]
(fail) Session claim > the timestamps survive a duplicate report, which is what billing rests on [33.00ms]
(fail) Session claim > publishing a ticket is scoped to the host too [73.00ms]
(fail) Session claim > a stopped session has no address to publish [36.00ms]
7 pass
39 fail
1 error
Ran 46 tests across 3 files. [2.88s]
```
(46 rather than 49 because the three `Placement` tests never ran — the
module
they import did not exist.)
## And passing after
```
49 pass
0 fail
126 expect() calls
Ran 49 tests across 3 files. [3.72s]
```
Full suite, against a fresh migrated database on `DATABASE_URL` and
`TEST_DATABASE_URL`:
```
181 pass
0 fail
479 expect() calls
Ran 181 tests across 16 files. [7.89s]
```
Baseline before this branch was `138 pass, 0 fail, 371 expect() calls`
across
14 files, so 43 tests and 108 assertions are new and nothing regressed.
`tsc --noEmit -p apps/api` reports the same five pre-existing errors in
`app/utils/hook.ts` and `app/utils/validator.ts` as it does on `dev`,
and none
in the files this branch adds.
## Shared files touched
- `apps/api/app/index.ts` — three lines: one import, and two mounts
(`/session`, plus `SessionApi.machineRoute` at `/machine`).
- `packages/core/src/box/index.ts` — one import and `Box.createPlaced`
added.
Nothing existing changed.
- `packages/core/src/session/session.test.ts` — `scene()` now also
returns
`machineId`; two new `describe` blocks appended.
- `packages/core/src/examples.ts` was **not** touched; the job schema's
examples reuse the existing `Examples.Session`, `Examples.Box` and
`Examples.Game` values.
No migration was created and none was needed.
## Judgement calls
1. **`GET /machine/jobs` lives in `session.ts`, mounted at `/machine`.**
The
path belongs under the prefix a host already uses for everything it asks
about itself, but the handler is this lane's code, so it is a second
Hono
instance (`SessionApi.machineRoute`) rather than an edit to
`machine.ts`.
2. **A job's payload beyond `kind` was not specified anywhere**, so it
is:
`kind`, `sessionId`, `boxId`, `boxTier`, `gameId`, `steamAppId`,
`linkedAccountId`. `steamAppId` is in there because the agent launches
by
store id and not by our internal one, and `boxTier` because the tier
also
sets output geometry. **This is the most likely place for the two
implementations to disagree** — see the disagreement note below.
3. **A run's terminal states refuse a ticket** (`409`). Nothing said
what
publishing an address for a stopped run should do; writing one can only
mislead a client that is still polling.
4. **A state report accepts only `starting`, `live`, `ended`,
`failed`.**
`requested` is written once, at creation, so reporting it is a `400`
rather
than a no-op.
5. **A box already running refuses a second run** with `409`. Not stated
in so
many words, but `Session.activeForBox` documents itself as the query
that
should start refusing rather than picking a winner silently, and this is
the
caller that would otherwise have made it pick.
6. **`Placement.onlyHost` refuses when there is more than one
candidate**
rather than taking the first row. Picking would be a scheduling policy
invented by accident and impossible to find later. It also refuses when
there are none, because `box.machineId` is not nullable and a placer
that
cannot answer must say so.
7. **`POST /session` returns `201`**, and its body is `.strict()`.
8. **Person-facing 404s, agent-facing 403s.** Both are the settled shape
for
their realm and both are asserted identical between "does not exist" and
"not yours".
9. **`POST /session` accepts an explicit `linkedAccountId`**, defaulting
to the
one the caller's session carries. A personal access token carries none,
so
requiring the session to supply it would make that credential unable to
start a run; the account is verified to belong to the caller either way.
## Where the specification was ambiguous or came out wrong
- **The lost-claim 409 is not reachable through the HTTP endpoint as
specified.** A box names exactly one machine, so both racers for a given
run
authenticate as *the same* machine — and the same machine re-reporting a
state it already reported is specified as `200`. Which of the two
answers a
caller gets therefore depends only on whether its read happened before
or
after the winner's write: concurrent gets `409`, a sequential retry gets
`200`. That is a defensible reading and it is what is implemented, but
it
means the two rules are distinguished by timing and not by anything a
caller
can see. If a second agent process per host is ever real, this needs a
claim
token in the request rather than a timing accident.
- **The job payload is unspecified**, which is the one part of the wire
the
agent parses and the one thing a document written before two
implementations
exist was supposed to pin. If the other end's test expects different
field
names, that is this gap and not a bug in either implementation, and it
should
be settled in the specification before either side moves.
- **Nothing said whether a person may report a state.** Implemented as
`403`:
terminal states are the agent's to write, and a person closing their app
is
a different fact from a run that stopped. That reading is also why
cancellation, listed below, has
nowhere to live yet.
## What this does not verify
- **No live round trip.** Every assertion here goes through
`app.request`
against a real Postgres. No real host agent has ever called any of these
five endpoints, and no client has ever read a ticket out of one.
- **The claim is not verified under concurrency.** The compare-and-set
is
pinned by stepping two attempts in sequence and showing the second
matches
zero rows. Two agents actually racing, on two connections, is not tested
and
cannot be from a single-process test.
- **The wire shape is asserted against this lane's reading of the
specification, not against the other end.** That is deliberate — each
end
writes its own test — but it means a shape agreement has *not* been
demonstrated. Both tests passing separately is the evidence, and it does
not
exist yet.
- **No ticket in this repository has ever been a real iroh ticket.**
They are
opaque strings to every assertion here; that the value survives a round
trip
says nothing about whether anything could connect to it.
- **`steamAppId` reaching the agent is untested end to end.** It is
asserted
present and correct in the poll response and nothing consumes it.
- **Nothing reaps a stuck `requested`.** A run whose host never polls
sits
there forever, and no screen distinguishes that from "starting soon".
Left
open deliberately: a timeout here would hide the gap rather than close
it.
- **Cancellation has no path.** A person who closes their app between
`requested` and `starting` leaves work that will be claimed and started
into
an empty room. There is no endpoint for it and this branch did not
invent
one.
- **Nobody writes `ended` when the agent itself dies.** The only writer
of
terminal states is the agent, so a host that loses power leaves a `live`
run
that keeps billing. The fix belongs with whatever decides host liveness
has
authority over a run's state, and that does not exist.
`Machine.isOnline`
exists but nothing joins it to a session.
- **Placement is verified only for the single-host case and the two
refusals.**
No test exercises a real alternative placer beyond an inline stub, and
nothing calls `Box.createPlaced` from an endpoint, because there is no
box
creation endpoint yet.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR adds the HTTP and core-domain session lifecycle, including user
session requests, machine job polling, host-scoped state transitions and
ticket publication, placement support, box-state synchronization, and a
database constraint ensuring one active session per box.
- Adds authenticated user and machine session endpoints with
resource-level ownership and host scoping.
- Uses compare-and-set state transitions so only one caller can claim a
requested session.
- Adds a partial unique index and duplicate-data repair migration to
enforce one active run per box.
- Clears connection tickets when sessions become terminal.
- Adds placement abstractions and extensive API, domain, migration, and
concurrency-oriented tests.
- The latest changes document and test that ownership is checked per
user rather than per selected linked account; they do not resolve the
previously reported multi-account launch failure.
<h3>Confidence Score: 4/5</h3>
The PR is not yet safe to merge because selecting a different linked
account can still launch a game that only another account owns.
The previously reported ownership issue remains unresolved: the current
request path checks the game against the user-wide library and
separately verifies only that the selected linked account belongs to
that user. The new test explicitly accepts this behavior, so a user with
multiple Steam links can request a game through an account that does not
own it, causing the host launch to fail. The three resolved previous
findings are fully addressed or manually resolved and do not remain
outstanding.
**Files Needing Attention:** apps/api/app/routes/session.ts
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/api/app/routes/session.ts | Adds user and host session endpoints
with scoped authorization and validation, but the previously reported
selected-account ownership mismatch remains. |
| packages/core/src/session/index.ts | Adds job queries, host-scoped
compare-and-set transitions, ticket lifecycle handling, box-state
synchronization, and active-session conflict translation. |
| packages/core/src/session/session.sql.ts | Declares the partial unique
index enforcing at most one undeleted, unstopped session per box. |
| packages/core/migrations/0008_session_one_active_run_per_box.sql |
Repairs pre-existing duplicate active sessions, clears their stale
tickets, and creates the active-session unique index. |
| packages/core/src/box/placement.ts | Introduces a placement seam that
selects the sole owner host and refuses ambiguous or unavailable
placement. |
| apps/api/test/session.test.ts | Adds broad endpoint coverage and now
explicitly demonstrates the unresolved per-user rather than
per-linked-account ownership behavior. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant U as User client
participant A as Session API
participant D as Core domain
participant DB as PostgreSQL
participant H as Host agent
U->>A: POST /session
A->>D: Validate box, game, library, and linked account
D->>DB: Insert requested session
DB-->>D: Enforce one active session per box
H->>A: GET /machine/jobs
A->>D: List requested sessions scoped to host
D->>DB: Join session through box.machineId
H->>A: POST /session/:id/state (starting)
A->>D: Compare-and-set state for host
D->>DB: Atomic scoped transition
H->>A: POST /session/:id/ticket
A->>D: Publish ticket while addressable
U->>A: GET /session/:id
A-->>U: Current state and ticket
H->>A: POST /session/:id/state (ended/failed)
D->>DB: Set terminal state and clear ticket
```
<sub>Reviews (4): Last reviewed commit: ["fix(api): say what the library
check
act..."](
|
||
|
Wanjohi
|
7bdca1240f |
fix(api): say what the library check actually proves
A library entry records the person, not the account the games were synced from, and `POST /library/sync` is not even told which account a list came from. So the ownership check added for session requests asks "has somebody this person linked got this game?" and not "does the account about to play own it?" — for the one Steam account most people have those are the same sentence, and for two they are not. Confirmed rather than reasoned about: a person with two Steam links, a game synced at person level, and a request naming the second account is accepted today. The check stays, because it still turns a box that boots, tries to launch and fails minutes later into an immediate refusal, and it never refuses on account grounds that the data cannot support. What changes is the comment, which claimed the stronger property, and a test that pins the gap so it is found deliberately rather than by surprise. Closing it properly means recording the linked account on a library entry: a column, a sync contract that says which account a list belongs to, a uniqueness rule per account rather than per person, and a backfill with no correct answer for rows already written. That is a decision about what a library is, and inferring it here would be the kind of modelling taken by accident that this branch refuses elsewhere. |
||
|
Wanjohi
|
51dabddbd8 |
fix(api): a run drives the box under it, and needs a game and a claim
Four things the session endpoints did not do, or did wrongly. The box had three states and nothing wrote them. A box read `created` while a run on it was `live`, so every screen showing a person what their hardware is doing was reading a column no code had ever moved. A run reaching `live` now makes its box `running`, and a terminal run stops it: `ended` cleanly, `failed` not, carrying the reason the agent gave. Not every run state maps — a box has no `starting` on purpose, because that transition is synchronous from the agent's side and a state nobody sets is a state that lies. Both writes are one transaction, since "this run is live" and "the box under it is running" are one fact in two tables, and a box stuck `running` with nothing on it has nothing to correct it. `POST /session` accepted any game in the catalog. A run launches as a Steam account that has to own the game, so one outside the caller's library is a box that starts, tries to launch and fails minutes later with nothing to point at; it is now refused up front. Told apart from a game that does not exist rather than hidden, because the catalog is public and "you do not own this" is a sentence a person can act on. The library is a synced copy, so this refuses a game bought since the last sync — that is a staleness bug in the sync, not a reason to start runs that cannot work. Publishing a ticket only refused terminal runs, so a host could publish an address for a run it had never claimed. A ticket is the address of something being brought up, so only `starting` and `live` accept one, and the state is in the write rather than only in the check above it. The two refusals stay separate answers because they are different mistakes: one agent skipped a step, the other has nothing left to reach. The migration that adds the one-active-run index stopped older duplicate runs without clearing the ticket they had published, which is the invariant that same migration exists to establish. It clears it now, verified against a box carrying two unstopped runs. Nine tests, each checked against the unfixed code first. |
||
|
Wanjohi
|
0d8630379b |
fix(api): a box gets one run, and a stopped run keeps no address
Two invariants the session endpoint stated but did not hold. A box runs one thing at a time. `POST /session` read `activeForBox` and refused when something was already running, but the read and the insert are two statements with nothing between them: two requests that both saw "nothing is running" each got a row, and the job poll then handed the host the same box to start twice. Demonstrated at 2 rows and 2 jobs from one box. That is the failure the state claim exists to prevent, one step earlier, and it takes the same answer — a partial unique index on the predicate the read asks about, so the database refuses the second insert. `Session.request` turns that refusal into the same 409 in the same words, so a caller cannot tell which of the two caught it. The migration resolves any existing duplicates before creating the index, keeping each box's newest unstopped run because that is the one a person is waiting on, and stopping the rest rather than deleting them. Separately, a run that reached `ended` or `failed` kept the last ticket it published. Publishing a new one is already refused, so the stale address was both the only ticket a client could read for a dead run and the one nothing was allowed to replace — and a client that polls would dial it. Terminal transitions now clear it, in `setState` as well as in the compare-and-set, so the invariant does not depend on which writer stopped the run. Seven tests, each checked against the unfixed code first. The published descriptions for the ticket field and the read endpoint now say that a stopped run has no address. |
||
|
KAAL1 (Bingus)
|
e70f05e245 |
fix(nescope): make HDR reachable — start XWayland, advertise the opaque FourCCs (#314)
Three related fixes. Together they take HDR from unreachable to working
end to end on the XWayland path.
## XWayland was never started
Three lines had been commented out since the initial import: the call
that spawns XWayland, the guard that waits for it, and the `DISPLAY`
handed to the child. Every game therefore launched as a native Wayland
client. Nothing reported it -- the compositor still logged the X display
it was telling clients to point at, which is why it read as working.
That is also why HDR never fired. The colour space is signalled over a
protocol whose Vulkan layer lives in the game process and finds the
compositor through the X11 root window, so the one path able to carry it
was the one path no game was on. `ENABLE_GAMESCOPE_WSI` and `DXVK_HDR`
were already being set, which switched that layer on and then handed it
a
display it could not use.
Restoring the guard also fixes the ordering it was written for: the
launch
now happens after XWayland reports ready rather than ~40ms before it.
## Mesa was dropping every format we advertised alpha-only
Mesa tracks two flags per VkFormat -- one contributed by a format alpha
FourCC, one by its opaque FourCC -- and skips any format carrying only
one:
```c
if (!(disp_fmt->flags & WSI_WL_FMT_ALPHA) ||
!(disp_fmt->flags & WSI_WL_FMT_OPAQUE))
continue;
```
We advertised `ARGB8888` and `XRGB8888`, so `B8G8R8A8` survived and made
the list look like it was working. Everything else was alpha-only and
was
dropped in silence -- `ABGR8888` had been advertised all along while
`R8G8B8A8` never once appeared on a surface. Adding the opaque spellings
takes the surface from 6 formats to 21 and restores the three that carry
HDR.
The comment above that list claimed it was for XWayland DRI3 and that a
game swapchain format was independent of it. It was the opposite: the
list decides what a game can select, and deleting an entry removes that
format from every client.
## Verified against swapchains, not format lists
A client asking for `A2B10G10R10` + `HDR10_ST2084` now gets a swapchain
and the compositor is told colorspace `1000104008`; one asking for
`R16G16B16A16_SFLOAT` + scRGB linear gets `1000104002`. Previously both
were refused at creation -- the WSI layer re-checks the requested format
against the driver own surface list, so the colour space and the pixel
format arrive from two different places and only one was being supplied.
`apps/nescope/scripts/verify-hdr-formats.sh` asks what a client is
offered
from inside a child process, keeping the XCB and Wayland surfaces apart
since a game presents through the XCB one. The default mode guards both
halves of what the compositor controls; `--expect-layer` states the full
target and passes once a WSI layer is present. No new dependencies
(`vulkaninfo` + `python3`).
## Still open
HDR is XWayland-only, documented as a FIXME in `hdr.rs`. A WSI layer
binds
the swapchain factory on its own Wayland connection while a native
client
surface lives on the client one, and object IDs do not cross
connections.
The FIXME records the fix both reference implementations point at, and
the
trap to avoid when we take it: gating format injection on "the
compositor
supports HDR" rather than on being able to signal the surface hands a
client PQ pixels that arrive tagged as SDR, with nothing reporting an
error.
nescope ships no WSI layer of its own; the above was verified with the
stock gamescope one, which drives our protocol unmodified.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes HDR-capable native Wayland presentation reachable, adds
the alpha/opaque DMA-BUF FourCC pairs Mesa requires, makes XWayland an
explicit compatibility mode, and adds an HDR surface-format diagnostic.
- Starts XWayland only with `--xwayland`, waits for readiness before
launching the child, and stops cleanly if startup fails or times out.
- Routes Proton through Wayland unconditionally while retaining
`DXVK_HDR` as an HDR-specific setting.
- Advertises paired alpha and opaque FourCC variants with portable
modifiers.
- Separates XCB and Wayland probe results, selects one hardware adapter,
and distinguishes probe failures from format regressions.
- Documents the limitations of the legacy gamescope WSI path and the
external-layer dependency.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge; no outstanding correctness, security, or
repository-rule issue remains.
All previous findings are resolved in the current code, including the
XWayland failure lifecycle, removal of unsupported vendor modifiers,
corrected HDR documentation, reliable diagnostic failure handling,
per-GPU format selection, and unconditional Proton Wayland routing. The
changes since the previous review preserve diagnostic output handling
without introducing a new failure.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescope/src/main.rs | Adds opt-in XWayland lifecycle handling,
readiness timeout, conditional DISPLAY propagation, and unconditional
Proton Wayland routing; the previous launch-environment finding is
fixed. |
| apps/nescope/src/state.rs | Stops the event loop on reported XWayland
startup failure and advertises portable paired alpha/opaque DMA-BUF
formats without vendor-specific modifiers. |
| apps/nescope/src/hdr.rs | Documents the working native Wayland HDR
path and accurately distinguishes it from the external, deliberately
disabled gamescope WSI route. |
| apps/nescope/scripts/verify-hdr-formats.sh | Adds a diagnostic that
keeps GPU and surface paths separate and now preserves the intended exit
behavior when filtered Vulkan diagnostics contain no matching lines. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Launch[nescope child launch] --> Mode{--xwayland?}
Mode -->|No| Wayland[Native Wayland surface]
Mode -->|Yes| Wait[Start and await XWayland]
Wait -->|Ready| XCB[XCB / XWayland surface]
Wait -->|Error or 10s timeout| Stop[Log failure and stop]
Wayland --> Formats[Paired alpha and opaque FourCCs]
Formats --> HDR[HDR10 and scRGB formats available]
XCB --> SDR[X11 compatibility path without native HDR]
Proton[Proton child] -->|PROTON_ENABLE_WAYLAND=1| Wayland
```
<sub>Reviews (9): Last reviewed commit: ["nescope/scripts: guard the
diagnostic
pi..."](
|
||
|
KAAL1 (Bingus)
|
200bc9c75f |
fix(nescapture): stop defaulting unreadable formats to BGRA (#315)
Stacked on #313, which this depends on.
Found while checking whether the 10-bit HDR path actually works now that
a
client can obtain an HDR swapchain (see #314). **It does** — verified
end to
end rather than from the format list: a client requesting `A2B10G10R10`
+
`HDR10_ST2084` produces
```
pix_fmt=yuv420p10le color_range=pc
color_space=bt2020nc color_transfer=smpte2084 color_primaries=bt2020
```
which is a correctly tagged HDR10 stream, and the first pixel-level HDR
check
here with 10-bit rather than 8-bit input. Three ways it could have gone
wrong
instead.
## Unrecognised formats defaulted to BGRA
`vk_format_to_input_format` returned `BGRA` for anything it did not
know, which
reads a packed 10-bit or FP16 buffer as eight-bit channels. It now
returns
`None`, and the encode loop drops those frames with one log line per
format.
A stalled stream is a complaint. A stream at full frame rate carrying
nonsense
is not, and that is the failure this area keeps producing.
## Bit depth and input format had drifted apart
They were two separate matches on the same `VkFormat`. `A2R10G10B10`
counted as
ten-bit in one and had no entry in the other, so it fell back to
eight-bit
BGRA — the encoder configured for ten bits while the converter read
eight.
Depth now derives from the input format, so that disagreement is
unrepresentable. `A2R10G10B10` stays unmapped deliberately: a WSI layer
offers
it as one of its HDR pairs and the compositor dmabuf list advertises it,
but
the converter has no red-first 10-bit input, so there is nothing correct
to map
it to.
## The CPU fallback could not read either HDR format
It read four bytes per pixel for every format and encoded eight-bit
regardless, so a packed 10-bit buffer became garbage and an FP16 one was
half
an image of misread floats. It now refuses what it cannot read.
## Recorded, not fixed: the colour space we see is not always the one
requested
A FIXME at the point the value is read. A WSI layer rewrites
`imageColorSpace`
to `SRGB_NONLINEAR` before calling down — deliberately, since it carries
the
real colour space to the compositor out of band. We sit below it, so we
read
the rewrite. Measured, all three lines from one run:
```
[Gamescope WSI] ... colorspace: VK_COLOR_SPACE_HDR10_ST2084_EXT
swapchain created — format=A2B10G10R10 colorspace=SRGB_NONLINEAR
(re)init encoder: H265 Yuv420 Ten Bt709 → P010
```
Ten-bit right, BT.709 wrong: PQ samples encoded and tagged as SDR. The
same
client *without* the layer gives `Ten Bt2020` and an smpte2084 stream,
so this
is specific to the layer path — which is the path Proton titles take.
The fix cannot be local; the true colour space only exists in the
compositor,
which does receive it, so it needs a channel from there. Layer ordering
is not
a fix — we do not control it, and the non-layer path still needs the
Vulkan
value. Left out of this PR as a design change rather than a bug fix.
## Verification
- 4 new tests, 12 total, all passing.
- No new clippy warnings (diffed against the base branch).
- 10-bit HDR path: unchanged, still `Ten Bt2020` / smpte2084.
- SDR path: `verify-chain.sh` passes, 835 frames, 8-bit BGRA, brightness
agreement 0.57.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes Vulkan format handling fail safely instead of interpreting
unsupported swapchain buffers as BGRA.
- Maps supported Vulkan formats to explicit converter inputs and derives
bit depth from that mapping.
- Drops unsupported GPU frames with rate-limited logging.
- Rejects unsupported HDR formats in the eight-bit CPU fallback.
- Documents the color-space limitation caused by rewritten WSI metadata.
- Adds tests covering unsupported, eight-bit, 10-bit, and FP16 formats.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge, with no new actionable issues introduced
since the previous review.
The changes since the previous review are empty, the sole previous
finding was manually resolved after Greptile conceded it based on the
stacked PR dependency, and the full PR introduces no confirmed rule
violations or remaining correctness failures.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescapture/src/encode.rs | Replaces unsafe BGRA fallback behavior
with explicit format validation, consistent bit-depth derivation,
guarded CPU fallback, and focused tests. |
| apps/nescapture/src/swapchain.rs | Documents the known WSI color-space
rewrite limitation at the point where swapchain metadata is recorded. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Captured Vulkan frame] --> B{Known converter input?}
B -->|No| C[Log format change and drop frame]
B -->|Yes| D[Derive input format and bit depth]
D --> E{DMA-BUF path available?}
E -->|Yes| F[GPU color conversion and encoding]
E -->|No| G{Eight-bit RGBA or BGRA?}
G -->|Yes| H[CPU conversion and encoding]
G -->|No| I[Return recoverable error]
```
<sub>Reviews (3): Last reviewed commit: ["docs(nescapture): the
colour-space note
..."](
|
||
|
KAAL1 (Bingus)
|
3389e6065f |
fix(nescapture): tag encoded streams full-range to match the samples written (#313)
## The bug
`nescapture` sets the colour converter full-range unconditionally, but
the video usability information carried pixelforge's **default
limited-range flag**. A compliant decoder then expanded 16–235 out of
samples that already covered 0–255 — darkening midtones and clipping
both ends.
pixelforge keeps two separate flags for this, one on the converter and
one on the colour description, and its own documentation says they must
agree. Only the first was being set.
The two lines are about forty apart, each is correct on its own, and the
comment above the second states the right intent while the call below it
does the opposite:
```rust
// GPU framebuffer captures are always full-range — use BT.709 full-range
// so the decoder doesn't apply limited-range expansion.
enc_cfg = enc_cfg.with_color_description(ColorDescription::bt709());
// ^ this constructor is limited-range
```
## Evidence
Measured on a Radeon RX 9060 XT, comparing the encoded result against
the compositor's own readback of the same frames:
| ground truth = `51` | before | after |
|---|---|---|
| flat background, decoded | **`38`** | `49–51` |
| mean luma, capture path vs readback | **10.41 apart** | **0.55 apart**
|
| luma histogram intersection | **0.090** | **0.913** |
| declared `color_range` | `tv` | `pc` |
**The encoded luma is byte-identical before and after** — `Y = 51.00`,
standard deviation `0.00` on both runs. Only the tag changed, which is
what identifies this as a signalling bug rather than a conversion one,
and why nothing short of a comparison against ground truth could see it:
the stream was valid, the frame rate was right, the picture was
recognisable, and every liveness check passed.
The HDR arm (`bt2020_pq`) carried the same defect and is fixed the same
way, but **has not been run** — no 10-bit verification here.
## `scripts/verify-chain.sh`
Runs a Vulkan workload under `nescope` with the layer active and
compares the encoded output against `nescope-shot`'s readback of the
same frames. Two paths that share almost no code see the same content,
so disagreement localises the fault; a single path cannot tell a correct
frame from a plausible-looking wrong one.
**Confirmed it fails when this change is reverted** — both the tag check
and the brightness-agreement check fire.
One note on its thresholds, since it is easy to get backwards: the
not-blank check is a low absolute floor plus a comparison against the
readback's own structure, rather than a fixed number. A fixed number was
tried first and was wrong in the worst way — the **broken** build scored
20.49 on it and the **fixed** build 17.74, because the range defect
stretched contrast and that reads as more detail. How much structure a
correct frame carries depends on what the workload drew, so the only
stable reference is ground truth measured in the same run.
## Not covered
`vkcube` rather than a real workload; 720p, H.264, 8-bit; one card, one
driver. XWayland, HUD detection and real swapchain formats are
untouched.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
The PR aligns encoded-stream color metadata with the full-range samples
produced by nescapture and updates the CPU fallback to BT.709 full-range
conversion.
- Updates pixelforge and configures matching converter color space,
range, and SDR reference white.
- Corrects Vulkan color-space mapping and adds regression tests for SDR,
HDR, and CPU fallback behavior.
- Adds SDR capture-chain and HDR comparison verification scripts.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge.
No blocking failure remains.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescapture/src/encode.rs | Aligns GPU and CPU conversion output
with encoded color metadata and adds focused regression coverage. |
| apps/nescapture/scripts/verify-chain.sh | Adds an end-to-end SDR
verifier using a static corner patch to avoid the previously reported
temporal mismatch. |
| apps/nescapture/scripts/verify-hdr.sh | Adds an HDR comparison harness
for inspecting conversion behavior across builds. |
| apps/nescapture/Cargo.toml | Advances pixelforge to the revision
providing the required color-conversion configuration. |
| Cargo.lock | Records the pixelforge update and resulting transitive
dependency refresh. |
<sub>Reviews (5): Last reviewed commit: ["test(nescapture): check the
HDR
conversi..."](
|
||
|
Wanjohi
|
bbe729e5c7 |
feat(api): the session endpoint, and a claim that only one caller can win
A run of a box had core support and no HTTP surface. This adds both halves of it: a person asks for a run and reads it back, and the host agent the box is placed on is handed the work and reports what happened. The access rule is the point. An agent may only see or touch a run whose box is placed on its own hardware, and that is a `where` clause on every one of the three agent endpoints rather than a check next to them — host credentials are long-lived secrets sitting on hardware in somebody's home, so what one leaking can reach has to be decided by the query. "No such run" and "not your run" are the same refusal, so ids cannot be discovered by reporting states at them. `Session.setState` updated on the id alone, which means two agents polling the same work both succeed and both start the same box. There is one host today, which is exactly why that would have been built wrong and stayed wrong. The state a run is moving out of is now part of the `where` clause, so the database picks the winner; the loser gets a conflict rather than a silent no-op. Three cases that look alike are kept apart: re-reporting a state you already reported changes nothing and is not an error, a transition that does not exist is refused with the run left where it was, and another host reporting anything is forbidden. Asking for a run makes no decision about where it happens — a box already names its hardware, so the run inherits it by join. Placement therefore gets an interface at box creation, where the decision actually is, with the single-host case as its implementation and a deliberate refusal when there is more than one candidate and no policy to choose with. Tests cover the wire shape from both sides, the query scoping, the claim, and the timestamp idempotence a run's billing rests on. |
||
|
Wanjohi
|
aaa1bbd0f4 |
fix(nesdoctor): the closing prose still said "launch times"
Missed when the histogram was corrected: the tool spent a paragraph asking people to send the JSON and described it as holding "installed titles with sizes and launch times", which is the same claim the histogram itself no longer makes. Steam stores when a title was last played, not each time it was launched. Found by running the installer end to end after the release — the shipped 0.3.0 binary prints it. Fixed here for the next one; it is prose in the closing paragraph, not a number anybody acted on. Same wording in the --json flag's doc comment, in the submit URL's comment, and in the consent prompt that asks to read the library at all. That last one matters most of the four: it is what somebody reads before saying yes. |
||
|
Wanjohi
|
d8e5c19181 |
release(nesdoctor): point the installers at v0.3.0
Flipped after publishing, so there was never a window where the live installer named a tag that did not exist. Verified before committing: all five assets under the new tag return 200, and the installer was run end to end — it fetched the published binary, checked it against SHA256SUMS, and the thing it ran reported 0.3.0. |
||
|
Wanjohi
|
29a826d9f9 |
release(nesdoctor): 0.3.0
Minor rather than patch, because the output changed meaning and not just its numbers. `up=` was overstated by about a fifth and is now measured over the window the bytes were actually counted in, so a figure from 0.2.x and one from this release are not comparable. And the hour-of-day histogram was labelled launches when Steam only stores one timestamp per title, so the summary line now carries which sample a peak came from — `n=28/all` where it used to say `n=28`, which is a shape people paste. New wire keys hours30, n30, nspan, playh and peaksrc. hours, n and peak keep their names and meaning so the corpus stays continuous; a submission without peaksrc is whole-library by construction.nesdoctor-v0.3.0 |
||
|
Wanjohi
|
cf56aaf04c |
docs: this repo is public, so say what things are, not who decided them
Comments and served API descriptions here had grown references that only make sense to someone with our internal notes: relative paths that escape this tree, filenames and titles of documents nobody outside can open, quoted prose from them, and the name of a component that has no public surface — once in an OpenAPI description, which is published output rather than source. None of it was load-bearing. Every case restates as what the code actually requires, and every rewrite came out shorter: "in the words the host agent reports" for a component name, "republished as addresses are discovered" for a quoted phrase, "a size tier sets vCPU, RAM and the output geometry" for a sentence that had been carrying a path. Internal reasoning is now cited exactly one way, ref(d-NNNN) in a source comment, with the rule that the sentence must still stand if the marker is deleted. CLAUDE.md leads with it, because the previous version of this mistake was made by people who knew the repo was public and it still took ten occurrences to notice, so "be careful" is not a mechanism. Commit messages get the stricter rule and carry no references at all: a comment can be fixed by the next commit and a published message cannot be fixed at all. Git hooks now enforce both halves. The check caught a real one while being written: the CLAUDE.md table spelled out the paths it was prohibiting, which discloses them to exactly the reader it protects against. 138 tests, 0 fail. |
||
|
Wanjohi
|
c3682136f1 |
test(api): hold the heartbeat wire contract from the host's side
`lastSeen` and `intervalSeconds` are the two field names neslet's plane.rs reads out of the reply, and a rename on either side yields a host that beats, parses nothing and reports success. These tests are what make that a contract rather than a coincidence. Also asserts the property the middleware comment claims and nothing checked: wrong machine credentials and no credentials produce *identical* responses, because bad credentials fall through to `public` rather than erroring so that probing cannot reveal which machine ids exist. Comparing the two bodies is the only way that stays true. Two of these tests started out asserting 401 and were wrong, not the code — `machineOnly` sees a public actor either way and forbids. 138 tests, 0 fail. |
||
|
Wanjohi
|
4315510de8 |
feat(api): a host can say it is alive, and is told how often to
Second half of G1's "neslet registers against api.nestri.io and heartbeats".
Registration already worked; there was no heartbeat endpoint at all — grep for
it across apps/api and packages/core returned nothing, and neslet's own
main.rs says the same from its side.
POST /machine/heartbeat, machine credentials only. Two decisions worth stating
because neither is obvious from the diff:
**It returns the interval.** The auth middleware already touches lastSeen on
every authenticated machine request, so an endpoint that only did that would
add an endpoint and no capability. What a host cannot know on its own is how
often the control plane wants to hear from it, so the response carries the
cadence. A fleet whose interval can only change by shipping a new agent is a
fleet whose interval never changes.
**It takes no body.** neslet has a HostSummary ready to send, and week 2 owns
box state reporting. Accepting fields nothing acts on yet would mean a wire
shape we would have to keep, chosen before the thing that consumes it exists.
Online-ness is derived from lastSeen rather than stored: a host that stops
beating goes offline through the passage of time, which is the one mechanism
that cannot itself fail. Three missed beats, not one — a single missed beat is
a lost packet, and treating that as offline would make placement flap.
Also: the machine actor's teamID stops being optional. It was `...(teamId ? {}
: {})` in the middleware, a branch for a state that cannot exist now that
machine.team_id is notNull.
134 tests, 0 fail.
|
||
|
Wanjohi
|
6c1d407985 |
feat(core): a box is a row, a session is the billing unit
Migration 1 of 0048, and the first of the seven weeks — nothing about a live
feed works without these two tables, so it is not a cleanup during them.
box a VM someone owns: an id that is also its DNS label, an editable
label, an owning user, the machine it sits on, a tier and a state.
Owned by a person and placed on a team's hardware, which are two
different relationships, hence both userId and machineId.
session one run of one box by one linked Steam account, and what costs
money. Separate from box because the ticket changes after bind as
addresses are discovered — the vsock contract calls it "a stream,
not one value" — so it is a column a client polls, not a value it
is handed once.
Box states are neslet's own three and no more. `starting` and `stopping` are
the obvious additions and both are omitted because nothing would ever write
them; a failed box is `stopped` with stopClean false, which is how neslet
models it too.
The generated migration would have failed on live rows in three ways, so it
is hand-written and tested against a database seeded at the old schema:
- machine.team_id becomes notNull, and *every existing row is null* because
the old registration path passed null. Personal teams are backfilled for
machine owners first, reusing a team they already own rather than minting
a second, with the owner membership row repaired where missing.
- game_download.host_id becomes a foreign key. It held free-form strings,
so unattributable rows are deleted before the cast — the only destructive
statement here, and a considered loss: it is a progress report neslet
re-derives from disk.
- Team.createPersonal was written and documented in packages/core/CLAUDE.md
as part of the login flow and never actually called, so no user has a
team. ensurePersonal is idempotent and now runs on every login, which is
what backfills accounts the migration does not reach.
Verified on a seeded legacy database: three null-team machines backfilled, an
existing team reused rather than duplicated, a blank display name handled, and
both unattributable download rows dropped while the attributable one survived.
Also fixes two things this work ran into rather than caused:
- Database.client() built a new postgres pool on every call, and use()
called it twice per invocation — pools of ten connections held for a 30s
idle timeout. Invisible in a Worker where requests are short; the suite
crossed 100 connections and Postgres said "sorry, too many clients
already" in whichever file ran last, which reads as a flaky test rather
than a leak. Now one pool per connection string.
- download.test.ts asserted against `hst_…` host ids, which is exactly the
unattributable row the new foreign key exists to refuse.
There is no "no team" any more: PATCH /machine/:id took teamId null to mean
"mine alone" and now requires a team, because the personal team is the one to
name. Its test is updated to the new contract rather than deleted.
113 → 128 tests, 0 fail.
|
||
|
Wanjohi
|
1bfdfcf3cf |
fix(nesdoctor): "launch records" were never launches
Steam keeps one LastPlayed per title, so the hour-of-day histogram holds one sample per *title* — at the hour it was last closed, over the whole life of the library. It was labelled and reported as a launch histogram, and the module header claimed a library of eighty games is "eighty samples of what hour this person launches a game at — a real distribution". It is not. The bias has a direction, and everything pushes the same way: a title played once years ago weighs exactly as much as a daily driver, a daily driver contributes one sample ever, and an afternoon spent installing and trying a dozen games stamps a dozen titles with that afternoon's hour. So the metric over-weights trying and under-weights playing. On the production host it reads n=331 with 329 titles no longer installed; on this dev machine, 28 titles spanning 570 days. Corrected rather than disclaimed, because the direction is knowable: - Fields say what they hold — last_played_hours, titles_sampled, and no "launch" anywhere. The display says "when you last played each game — 28 titles, local time, reaching back 19 months". - A second histogram over titles played in the last 30 days, which is one sample per title still in use, and the peak window prefers it when it has the samples to claim a shape. - Which histogram the peak came from is stated in the output and on the wire (peaksrc=30d|all), and the summary line carries the sample count the window was actually computed from, so a narrow peak drawn from nine titles cannot borrow the authority of three hundred. - New keys hours30, n30, nspan, playh. hours/n/peak keep their names and meaning so the corpus stays continuous; submissions without peaksrc are whole-library by construction. Playtime is read and reported but deliberately not used as a weight: it is a lifetime total against a single timestamp, so weighting by it would multiply one arbitrary hour by five hundred. Five tests, including the wrapping midnight window, which is the case a non-wrapping scan gets wrong and exactly the evening peak 0017 is about. |
||
|
Wanjohi
|
3dba825f17 |
fix(nesdoctor): up= was overstated by about a fifth
The throughput window and the byte count disagreed. Bytes were counted from the moment the upload threads started, the 1.5 s queue-fill ramp included; the divisor was that same span with 1.5 s subtracted from it. So a numerator covering ~8.3 s was divided by ~6.8 s, and every up= figure nesdoctor has ever published is high by ~22%. Snapshot the counter and the clock together after the ramp, and measure both from there. Excluding the ramp is also the better measurement: TCP slow-start lives in it, so it is not the steady state a session gets. Found by running speedtest on the same line in the same afternoon — 284 Mbps against our 502 — which is the only way it could have been found. The code was self-consistent and the number it printed was plausible, so no amount of re-reading would have shown it. A boundary effect remains and is documented in the code rather than papered over: bytes arrive one completed 8 MiB POST at a time, so up= keeps a few per cent of upward slack. Submissions collected to date stay useful as a floor and as a bufferbloat corpus. They are not usable as throughput. |
||
|
Wanjohi
|
90bd93f47f |
docs(nesdoctor): Windows will block it, and here is why and what to press
Reported from a real machine on release day. The README asks strangers to run a binary, so it should say what actually happens when they try. The why matters more than the workaround: SmartScreen objects to the file being unsigned and having no download history, not to anything the program does. And history attaches to the file hash, so a project releasing four times in an afternoon never accumulates any -- waiting is not a strategy. Offers the source build as the version that requires no trust, and says that stopping is a reasonable choice. Reproducible CI builds and published checksums prove provenance without moving SmartScreen an inch, and pretending otherwise would be the kind of overclaim this tool cannot afford. |
||
|
Wanjohi
|
f0227201db |
release(nesdoctor): point the installers at v0.2.2
Flipped after publishing, so there was never a window where the live installer named a tag that did not exist. |