Skip to content

Commit 4c9d30a

Browse files
raymondproguyclaude
andcommitted
docs: record Tier 3 as done and log Stage 2
Marks Tier 3 done in NEXT.md, adds its section to CURRENT-STATE.md and logs the session in PROGRESS.md. The entry says plainly what a green suite does not cover here: no Postgres, so migrations 009-011 have never been applied; the worker's SKIP LOCKED claim is untested; the in-memory double cannot reproduce two workers racing. Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent cfbf29f commit 4c9d30a

3 files changed

Lines changed: 307 additions & 3 deletions

File tree

docs/development/CURRENT-STATE.md

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ not a router change; Apple is the one that does not fit that shape and
1313
has its own `httpapi/apple.go` — see `NEXT.md` Tier 1).
1414

1515
Tier 2 added one admin endpoint on top of those, the first in this repo
16-
— see below.
16+
— see below. Tier 3 added three more admin endpoints and this repo's
17+
first three tables of its own, plus the config that lights up Argon2id,
18+
cloud logging and email templates — see below. Every admin endpoint in
19+
this repo is either read-only or an explicit operator action on a named
20+
key; nothing on that surface applies a suggestion by itself.
1721

1822
Tier 1 also added the second-factor surface: TOTP enroll/confirm/
1923
disable, passkey registration/list/delete, magic-link request/complete,
@@ -217,7 +221,94 @@ cryden copies.
217221
`internal/smoketest`'s sessions check also stops accepting "200 with
218222
anything in it".
219223

220-
## Tier 3 through 5
224+
## Tier 3 — config plus real endpoints: DONE
225+
226+
The first tier that adds things cryden has no concept of. Built in two
227+
stages on `feat/tier3-config-and-endpoints`; `go build`, `go vet`,
228+
`gofmt -l` and `go test ./...` are clean, and `PROGRESS.md` records what
229+
that does and does not cover.
230+
231+
**Config that lights up an engine feature** (cryden already implements
232+
all of it; this repo supplies values):
233+
234+
- **Argon2id** (`PASSWORD_HASHER=argon2id` plus the five `ARGON2ID_*`
235+
knobs). `Argon2idParams` starts from `security.DefaultArgon2idParams`
236+
and each env var overrides only the field it names — cryden's
237+
`NewArgon2idHasher` uses a partially-filled struct as a real custom
238+
configuration rather than as "defaults plus overrides", so building
239+
one from only the vars that happened to be set would silently drop
240+
the rest to zero. Switching hasher is safe at any time and needs no
241+
migration: existing bcrypt hashes keep verifying and are rewritten one
242+
successful login at a time.
243+
- **Cloud logging** (`CLOUD_LOGGING`, `LOG_LEVEL`,
244+
`CLOUD_LOG_REDACTION`, `CLOUD_LOG_HASH_KEY`) — see the shipped-events
245+
log below.
246+
- **Email templates** (`EMAIL_TEMPLATE_DIR`, new `templates/` package):
247+
cryden deliberately owns no message copy, so this is entirely this
248+
repo's. `verification.txt` and `magic_link.txt` rendered with
249+
`text/template` (`{{.To}}`, `{{.Token}}`, `{{.URL}}`). Unset keeps the
250+
console senders' built-in lines byte for byte; a set-but-broken
251+
directory is a startup failure.
252+
- **API keys** (`API_KEY_PREFIX`, `POST`/`GET /v1/api-keys`,
253+
`DELETE /v1/api-keys/{keyID}`, all `RequireAuth`): cryden scopes every
254+
one to the calling user by deriving the user ID from the verified
255+
token, so a key belonging to another account, a key that does not
256+
exist and an already-revoked key all answer the same `404
257+
api_key_not_found`. The raw key is returned once and never again —
258+
cryden stores only its SHA-256 hash. **No endpoint authenticates
259+
*with* an API key yet**; that is a separate change.
260+
261+
**Endpoints over this repo's own tables:**
262+
263+
- **`GET /v1/admin/security/hash-migration`** (`RequireAdmin`,
264+
read-only): `store.UserStore.Count` against two `CountByType` calls on
265+
`EventPasswordHashUpgraded` — all-time and windowed — so an operator
266+
can watch a bcrypt-to-Argon2id migration drain. `upgraded_events`
267+
counts events, not users, so the field that actually answers "is this
268+
draining" is `upgraded_events_in_window`, and `estimated_remaining` is
269+
named as an estimate on purpose.
270+
- **Per-user metadata** (`usermeta/`, `migrations/009`): this repo's own
271+
table, because cryden's `store.User` deliberately has no metadata
272+
concept. Its purpose is JWT claim mapping — `main.go` sets
273+
`AccessTokenClaims` to `usermeta.ClaimsProvider(...)`, which merges
274+
the operator `role` with every stored key, so a metadata change lands
275+
on that user's next login or refresh and never retroactively. Admin
276+
`GET`/`PUT`/`DELETE /v1/admin/users/{userID}/metadata[/{key}]`, per key
277+
rather than whole-map so two operators cannot lose each other's work.
278+
Key validation and the reserved-claim rule live in the store, not the
279+
handler, so they hold for any writer. The prefix merge costs **two
280+
queries on every login and every refresh**.
281+
- **Webhook delivery log** (`webhook/`, `migrations/010`): `WEBHOOK_URL`
282+
is the on/off switch. cryden calls `notify.WebhookSender` on the login
283+
request path, so `SendWebhook` writes one `pending` row and returns;
284+
a background worker makes the call. **The row is the queue** — a
285+
channel would lose everything on restart. Exponential backoff 30s
286+
doubling to 30m up to `WEBHOOK_MAX_ATTEMPTS`, then `failed` and left
287+
readable. The body is built at enqueue and stored, so a retry sends
288+
identical bytes and the log can answer "what did we send" for a retry
289+
as well as a first attempt. `GET /v1/admin/webhooks/deliveries`
290+
(admin, read-only, no retry button). `id` is a `BIGSERIAL` surrogate
291+
rather than the event id, which cryden may leave **empty**.
292+
- **Shipped-events log** (`shiplog/`, `migrations/011`): this repo ships
293+
no vendor SDK, so "shipped" means recorded in `shipped_log_events`,
294+
read back by `GET /v1/admin/logging/recent`. `main.go` composes
295+
exactly the shape cryden's `logger` doc prescribes — redacting
296+
*inside* the `MultiLogger` fan-out, so stdout keeps the IP that makes
297+
an incident debuggable and only the copy leaving loses it. `level=`
298+
means "at or above", the same direction the `LevelFilter` reads.
299+
`LOG_LEVEL` (default `info`) keeps the volume sane.
300+
301+
The three new tables (`009``011`) have **never been applied to a real
302+
database** — there is no Postgres in this sandbox. The webhook worker's
303+
claim and backoff behaviour is tested against `httptest` and an
304+
in-memory double, not against Postgres `FOR UPDATE SKIP LOCKED`, and
305+
that double cannot reproduce two workers racing. `PROGRESS.md` says all
306+
of this plainly.
307+
308+
## Tier 4 and 5
221309

222310
Not started. See `NEXT.md` for the full, ordered, specced-in-detail
223-
queue.
311+
queue. Tier 4 is all behind `RequireAdmin` and stays read-only by
312+
construction, with the decision already made that an AI suggestion
313+
**pre-fills** a settings form and never auto-applies.
314+

docs/development/NEXT.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ Tier 1 is done — see the status note under Tier 1 and `PROGRESS.md`'s
1414
Tier 2 is done — see the status note under Tier 2 and `PROGRESS.md`'s
1515
2026-09-15 entry. No engine bump this tier, so there were no new cryden
1616
migrations to copy.
17+
Tier 3 is done, in two stages on `feat/tier3-config-and-endpoints` — see
18+
the status note under Tier 3 and `PROGRESS.md`'s 2026-09-15 entries.
1719

1820
---
1921

@@ -215,6 +217,29 @@ Two details were decided rather than assumed, and are recorded in
215217

216218
## Tier 3 — config plus real endpoints
217219

220+
> **Status: every sub-item below is built, in two stages on
221+
> `feat/tier3-config-and-endpoints`.** Stage 1: Argon2id, cloud-logger
222+
> and email-template config, API keys, hash-migration. Stage 2:
223+
> `user_metadata` with JWT claim mapping, webhooks with a delivery log
224+
> and background worker, cloud-logging shipped-events log. `go build`,
225+
> `go vet`, `gofmt -l` and `go test ./...` are all clean on this branch.
226+
> What is still owed, and said plainly rather than implied: the
227+
> migrations `009``011` have **never been applied to a real database**
228+
> (no Postgres in this sandbox), the webhook worker's claim and backoff
229+
> behaviour is tested against `httptest` and an in-memory double
230+
> **rather than against Postgres `FOR UPDATE SKIP LOCKED`**, the
231+
> in-memory double cannot reproduce two workers racing (one mutex), and
232+
> this repo still has **no graceful shutdown** — owed before the
233+
> shipped-events sink could move off the request goroutine. `PROGRESS.md`
234+
> has all of it.
235+
>
236+
> Two deliberate deviations from the spec below, both argued in
237+
> `PROGRESS.md`: `webhook_deliveries` uses a `BIGSERIAL` surrogate
238+
> primary key rather than the event id (which cryden may leave **empty**,
239+
> by design, when its `crypto/rand` generator fails), and the shipped
240+
> copy is recorded in this repo's own table rather than sent to a vendor,
241+
> because this repo ships no vendor SDK.
242+
218243
- **Argon2id, cloud loggers, custom email templates**: config only.
219244
Custom email templates specifically need **no engine change at
220245
all** — cryden deliberately has no template config (there's a test

docs/development/PROGRESS.md

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,3 +490,191 @@ Next: Stage 2, once someone can run a build. The check-in the user
490490
asked for is the point at which this entry was written; Stage 1's code
491491
should be built, vetted, formatted and tested before Stage 2 starts on
492492
top of it.
493+
494+
## 2026-09-15 — Tier 3, Stage 2 (metadata, webhooks, shipped events)
495+
496+
All four commands were run on `feat/tier3-config-and-endpoints` before
497+
each commit, and all four were clean:
498+
499+
```
500+
go build ./... ok
501+
go vet ./... ok
502+
gofmt -l . (no output)
503+
go test -count=1 ./... config 0.031s httpapi 13.308s shiplog 0.006s
504+
templates 0.008s usermeta 0.008s webhook 0.029s
505+
```
506+
507+
The Stage 1 entry ended by saying Stage 2 should not start until
508+
someone could run a build. The toolchain here recovered, so it did.
509+
510+
Four commits, one logical step each:
511+
512+
- `d43a73d``usermeta/`, `migrations/009`, the three admin routes and
513+
the claims-provider merge in `main.go`.
514+
- `b3292c9``webhook/` (store, sender, worker), `migrations/010`, the
515+
deliveries endpoint, the `WEBHOOK_*` config and `.env.example` block.
516+
- `bd2c990``shiplog/` (store, logger), `migrations/011`, the logging
517+
endpoint, and the `MultiLogger` composition in `main.go`.
518+
- `cfbf29f``README.md` and `openapi/spec.yaml` (1.3), then the three
519+
docs files.
520+
521+
### What was built, and the decisions worth re-reading
522+
523+
- **A repo-owned store is an interface, a Postgres implementation and an
524+
in-memory double, in one package.** `usermeta`, `webhook` and
525+
`shiplog` each follow cryden's own `store/interfaces.go` +
526+
`store/memory` + `store/postgres` split. That is what makes these
527+
endpoints testable with no Postgres — which matters more than usual
528+
here, because there is none in this sandbox.
529+
- **The webhook delivery row is the queue, not a channel.** cryden calls
530+
`SendWebhook` synchronously on the login request path, so `SendWebhook`
531+
writes one `pending` row and returns; the capacity-1 channel is only a
532+
nudge, and a full channel drops the hint rather than blocking. A
533+
channel would lose everything on restart, and "was that lockout
534+
announced" is unanswerable for an event that vanished before a row
535+
was written.
536+
- **The body is built once, at enqueue, and stored.** Retries resend
537+
identical bytes, so the delivery log answers "what did we send" for a
538+
retry as well as a first attempt, and the signature covers the same
539+
bytes the log shows.
540+
- **`webhook_deliveries.id` is a `BIGSERIAL` surrogate, not the event
541+
id.** The spec said `id UUID PK`. But `notify.WebhookEvent.ID` **may
542+
be empty** — cryden generates it with `crypto/rand` and on generator
543+
failure deliberately delivers without one — and a delivery log whose
544+
primary key can be blank loses exactly the rows an operator most wants.
545+
The engine's id is recorded beside it as `event_id`. This is a
546+
deviation from the plan and is why it is written down.
547+
- **"Shipped" means recorded in this repo's own table.** There is no
548+
vendor SDK here, so `shipped_log_events` holds the same bytes a hosted
549+
aggregator would have received, which is what makes it a stand-in for
550+
one rather than a second, different log beside it. Swapping in a real
551+
client is a change to one line of `main.go`.
552+
- **The shipped-events sink writes synchronously, and that is a
553+
deliberate ceiling.** An asynchronous sink needs a buffer, a flush
554+
policy and a shutdown path, and this repo has no graceful shutdown
555+
anywhere yet. A buffer that is never flushed on exit is a log that
556+
silently drops its last records before a crash — for a log, the
557+
failure that matters most. `LOG_LEVEL` (default `info`) is what keeps
558+
the volume sane meanwhile.
559+
- **`level=` on the logging endpoint means "at or above".** The same
560+
direction `logger.LevelFilter` reads the word, so one word means one
561+
thing within one feature. The in-memory double filters **by name
562+
against the same set the SQL passes**, so it is faithful by
563+
construction even for out-of-range levels, where `Level.String()`
564+
clamps.
565+
- **An unrecognized stored level name is filed at `LevelError`**, not
566+
dropped. Failing a whole listing over one hand-written row would be a
567+
log an operator cannot read because of a typo in a row they were
568+
trying to inspect.
569+
- **`sink` is recorded even though only one value is written today**
570+
a row read out of a table shared with a second sink stays
571+
attributable.
572+
- **Metadata key validation lives in `usermeta`, not `httpapi`.** The
573+
reserved-claim rule is a data invariant, so it holds for any writer.
574+
`reserved_claim_names` is reported by `GET` so a console can grey
575+
those out rather than let an operator discover the rule by rejection.
576+
Writes are per key, never a whole-map `PUT`, so two operators editing
577+
different fields cannot lose each other's work.
578+
- **`PUT`'s body decodes `value` into a `json.RawMessage`, not an
579+
`any`.** `{"value": null}` and a missing `value` are different things,
580+
and decoding into `any` collapses both to nil.
581+
- **A malformed `userID` is a `404`, not a `500`.** Handed straight to
582+
Postgres, `"not-a-uuid"` is a driver error — "invalid input syntax for
583+
type uuid" — which `mapError` turns into a 500 an operator reads as a
584+
bug in the API rather than as a stale bookmark.
585+
586+
### Bugs found, and how
587+
588+
Three real ones, none of which a type check would have caught:
589+
590+
- **`WEBHOOK_MAX_ATTEMPTS` set without `WEBHOOK_URL` did not fail
591+
startup.** The orphaned-setting check used `os.LookupEnv`, but the
592+
config tests' own `loadForTest` uses `t.Setenv(name, "")` — which
593+
*sets* the variable to empty — so six existing tests failed. The
594+
package's documented convention is that empty counts as unset
595+
everywhere, so the check became `os.Getenv(...) != ""`. Found by
596+
running the suite, which is the only thing that would have.
597+
- **`openapi/spec.yaml` had never parsed as YAML.** `APIKey.id`'s
598+
description — `What DELETE /api-keys/{keyID} takes.` — sat unquoted
599+
inside a flow mapping, so the `{` opened a nested mapping and a parser
600+
stops there. Nothing had ever run the file through one; it was caught
601+
only because the 1.3 additions were validated. Fixed by quoting that
602+
one scalar, with a comment saying why. It is a syntax fix, not a
603+
contract change — no path, field or status code moved, so 1.3's
604+
"additive only" note stands.
605+
- **`webhook.Sender` as a typed nil.** A nil `*webhook.Sender` assigned
606+
to cryden's `notify.WebhookSender` field is non-nil to cryden and
607+
would silently turn on `DefaultWebhookEvents` for a deployment with
608+
`WEBHOOK_URL` unset. `main.go` assigns the field inside the `if
609+
webhookStore != nil` block for exactly that reason, and the store is
610+
declared as the interface rather than the concrete type. The same
611+
class of trap `logger.NewMultiLogger` documents for untyped nils.
612+
613+
Two test bugs, both found by a red test and both the test's fault:
614+
`TestWebhookDeliveriesFiltersByStatus` resolved rows with `ClaimDue`,
615+
which sweeps *every* due row, so its second row came back `in_flight`
616+
rather than `pending` — the test now resolves before seeding; and
617+
`TestParseLevelFilesAnUnknownNameAtTheMostSevereEnd` asserted that
618+
`"INFO"` and `"warning "` were unknown, when `logger.ParseLevel` is
619+
case-insensitive, trims, and accepts the `warning` alias. The premise
620+
was wrong, not the code.
621+
622+
One naming collision, the same class as Stage 1's `Deliveries`:
623+
`shiplog.Logger` could not have both a `Log` method (the
624+
`logger.ContextLogger` interface dictates the name) and a `Log` field,
625+
so the field is `Errors`.
626+
627+
### Verification: what this does NOT cover
628+
629+
Said plainly, per `CODEX.md`, rather than implied by a green suite:
630+
631+
- **There is no Postgres and no network in this sandbox.**
632+
`migrations/009`, `010` and `011` have **never been applied to a real
633+
database** — not once, in any environment. They are a copy of a
634+
design, not a verified schema. Everything downstream of them is
635+
tested through the in-memory doubles.
636+
- **The webhook worker's claim and backoff behaviour is not tested
637+
against Postgres.** `ClaimDue`'s single `UPDATE … WHERE id IN (SELECT
638+
… FOR UPDATE SKIP LOCKED)` statement has not been run. What is tested
639+
is the worker's behaviour against `httptest` and `MemoryStore`.
640+
- **The in-memory double cannot reproduce two workers racing.** It is
641+
one mutex, so it proves the worker handles a claimed row correctly and
642+
proves nothing about contention. `SKIP LOCKED` is the reason raising
643+
the worker count later is safe, and that reason is unverified here.
644+
- **`internal/smoketest` still has never been run** against a database,
645+
unchanged from every previous tier's note.
646+
- **WebAuthn still needs a real browser authenticator, and Apple a live
647+
round trip.** Unchanged.
648+
- **The `usermeta` claims path is tested for storage and for the merge,
649+
but the "reaches a freshly issued token" assertion runs on cryden's
650+
in-memory user store**, not on Postgres' `user_metadata` table.
651+
- **`shiplog`'s Postgres `List` has not been run against the JSONB
652+
column it reads.** The `lib/pq` bytea trap (a `[]byte` param is sent
653+
as bytea hex, which a JSONB column rejects, so params go as
654+
`string(raw)`) is handled by reading rather than by a passing test —
655+
the `Insert` path that would exercise it needs a database.
656+
657+
Newly owed by this tier, alongside the three tables: **the graceful
658+
shutdown the Stage 1 entry already flagged.** The webhook worker takes a
659+
`context.Context` and gets `context.Background()`; the shipped-events
660+
sink writes synchronously precisely because there is nowhere to flush a
661+
buffer on exit. Both become cheap once shutdown exists and neither was
662+
smuggled in behind the other.
663+
664+
### Noticed while working, not fixed
665+
666+
- **`openapi/spec.yaml` still predates Tier 1** — unchanged from the
667+
Tier 2 and Stage 1 notes. Stage 2 added only its own schemas, paths and
668+
the 1.3 version bump; the gap is still there.
669+
- **`README.md`'s "Design notes" now carries the repo-wide read-only
670+
rule as prose.** It is in `CLAUDE.md` as a rule; a reviewer reading
671+
only the README previously had no way to know why there is no retry
672+
button.
673+
- **The delivery log and the shipped-events log both answer `404
674+
not_configured` when their store is nil**, which is a wiring fact. A
675+
client cannot currently tell that apart from "the resource genuinely
676+
does not exist" — the same shape every other unconfigured feature in
677+
this API already uses, so it is consistent rather than new.
678+
679+
Tier 3 is complete. Next is Tier 4, which stays read-only by
680+
construction with the pre-fill-never-auto-apply decision already made.

0 commit comments

Comments
 (0)