AGENTS.md@internal/storage · git:20260928.6e4aa51 · 2026-09-28 · sha256 49ff880662b56630

AGENTS.md@internal/storage git:20260928.6e4aa51A

Immutable. This exact content never changes and is served at /api/v1/blob/49ff880662b56630.

# Storage Agent Instructions

These instructions apply to `internal/storage/` and may be refined by nearer
scoped `AGENTS.md` files. Defer to [`internal/AGENTS.md`](../AGENTS.md) (format
before push, and the one-database rule that `stmttest` runs under) and root
[`AGENTS.md`](../../AGENTS.md) for broader rules.

## Storage Scope

- Storage is SQL-first. Prefer SQL-compatible modeling and query patterns.
- Supported databases are PostgreSQL, Spanner, and SQLite.
- SQLite is the zero-config / local (and small homelab) default when no
  `database:` dialect is configured. It is not a production peer to Spanner.
- Dialect implementations (pool, migrations, statements) live under
  `internal/storage/dialect/`. Integration bring-up for Postgres/Spanner
  is in `internal/storage/testdb` (testcontainers or env DSNs).

## Identifier minting

Resource PKs are dialect-owned `prefix_<opaque>` values
([ADR 047](../../docs/adrs/047-dialect-id-generation.md)).

- **Create paths:** assign empty IDs with dialect `Ensure` /
  `ensureManagedID` before INSERT. Do not invent IDs in domain, service, or
  handlers.
- **Pre-persist ceremony** (provisional user handle, passkey registration,
  in-memory flow/session handles): mint with `Statements().NewManagedID(prefix)`
  (same generator as `Ensure`).
- **Empty → assign; non-empty → keep** (ceremony / schema `$id` / fixtures).
  HTTP create must not accept a client-chosen resource PK.
- Do **not** call `idgen` from outside `internal/storage/dialect/`.
- Prefix registry and selection rules for new prefixes live in ADR 047.

## Statements And Transactions

Statement methods are tx-passive: they run on whatever `queryExecutor` they were
built with (pool client or an open transaction).

### Multi-write rule

If a statement method issues **two or more** separate write round-trips
(`Exec` / write `QueryRow` / `Update` / `Write`), it **must** wrap those writes
in the dialect helper:

- Postgres: [`withTransaction`](dialect/postgres/with_transaction.go)
- Spanner: [`withTransaction`](dialect/spanner/with_transaction.go)
- SQLite: [`withTransaction`](dialect/sqlite/with_transaction.go)

Inside the callback, use the **inner** `tx` for every write — never the outer
`s.client` / `s.db`.

A **single** SQL string (including a CTE or upsert that touches many tables) is
already atomic at the database; do not wrap it.

### Do not call `pool.Transaction` from statements

Service / pool `Transaction` opens the multi-entity boundary. Statement methods
must use `withTransaction` so they:

- open a transaction when called via `pool.Statements()`, and
- **join** an outer tx (Postgres savepoint; Spanner no-op nest) when already
  inside `pool.Transaction`.

Calling `pool.Transaction` from a statement would use a different Postgres
connection or nest Spanner RW transactions (forbidden).

### Never flatten the cause of an error returned from a transaction

Spanner aborts read-write transactions under concurrency, and
`ReadWriteTransaction` retries them for you — but only while it can still find
the gRPC status in the error your callback returns. It looks for a
`*spanner.Error` or a `status.FromError` match; anything else it returns
immediately, un-retried.

So inside a transaction callback, always keep the cause in the chain:

- `fmt.Errorf("...: %w", err)`, or `fmt.Errorf("%w: %w", sentinel, err)` when you
  also need a domain sentinel to stay matchable.
- `domain.Err…(err)` / `.WithParent(err)`, never a bare `domain.Err…()` when you
  are holding a real cause.

Writing `fmt.Errorf("%w: %v", sentinel, err)` strips the status off `err` and
silently converts a retryable abort into a user-facing error. That was the cause
of the CI flake in #788.

For the same reason, a transaction callback must be **replayable**: a retry
re-runs the whole closure, so keep it a pure function of state captured before
the transaction opened.

### These Spanner client methods do not retry ABORTED

`ReadWriteTransaction`, `Apply` and `PartitionedUpdate` retry aborts internally.
`BatchWrite` and `NewReadWriteStmtBasedTransaction` do **not** — they are
caller-retried by design. Nothing uses them today; do not adopt one without
writing the retry loop yourself.

### Reference

Multi-write nesting uses the dialect `withTransaction` helpers above
([`dialect/postgres/with_transaction.go`](dialect/postgres/with_transaction.go),
[`dialect/spanner/with_transaction.go`](dialect/spanner/with_transaction.go),
[`dialect/sqlite/with_transaction.go`](dialect/sqlite/with_transaction.go)).

### No call-site SQL concatenation

Do not assemble statement SQL at the call site with `+` or `fmt.Sprintf`
(for example `baseQuery + " WHERE …"`). Attach filters and bind arguments only
through `statementCompiler`:

- Prefer `compileRead` when a field schema exists (Get-by-ID and List).
- For JOIN selects without a matching schema (auth-attempt gets), use
  `WriteString` for the static base and `WriteArg` for each bound value.

Package-level `const` values that are one complete static statement remain
allowed (including adjacent string literals / `+` only for line wrapping).
Compiler-internal `WriteString(" WHERE ")` on a `statementCompiler` is the
supported path for dynamic filters.

## Authz persistence (Wave 1)

[`dialect/authz/`](dialect/authz/) holds the dialect-independent Wave 1 authz
persistence helpers (ADRs 032–034; storage design in
[`docs/design/api/permission-storage.md`](../../docs/design/api/permission-storage.md)):

- **Dual-write lifecycle projection** ([`dual_write.go`](dialect/authz/dual_write.go)):
  user lifecycle events project into the resource-scope index (RSI) **and**
  `authz_membership_edges` in the same transaction — `UserCreated` upserts
  RSI + the initial team edge, `UserDeactivated` clears edges but keeps RSI,
  `UserDeleted` clears both. Any new lifecycle path that touches users or
  team membership must go through these helpers, not write edges directly;
  a missed dual-write silently desynchronizes authorization from identity.
- **Catalog rows** ([`catalog_rows.go`](dialect/authz/catalog_rows.go)) map
  compiler mutations onto catalog tables; catalog versions persist via
  `PersistCatalogVersion`.
- **Identifiers**: authz assignments mint `asgn_<opaque>` ids
  (`domain.PrefixAuthzAssignment`) through the same statements surface as
  every other resource — the Identifier minting section above applies
  unchanged.
- The edge table carries FK + CHECK + cascade parity across all three
  dialects; behavior changes need `stmttest` coverage like any other
  statement (next section). The compiler/validator side lives in
  [`internal/authz/`](../authz/).

## Statement contract tests

Behavioral statement parity across dialects lives in
[`stmttest`](stmttest/) (see ADR 041). Shared suites assert through
`service.AllStatements`; build-tagged registration brings up postgres, spanner,
and/or sqlite, and `forEachDialect` loops dialects. CI still runs one tag per
job; multiple tags are supported in one process for local parity checks.

Each adapter is already its own cache slot: Moon hashes the three
`server:test-*` tasks separately, and `go test` hashes a different
`stmttest` binary per tag (`open_postgres_test.go` vs
`open_spanner_test.go` vs `open_sqlite_test.go`).

**When you add or fix dialect statement or schema behavior** under
`dialect/{postgres,spanner,sqlite}/`, you **must** add or extend a portable
`forEachDialect` suite in [`stmttest/`](stmttest/) for any domain-visible
change: new statement methods, corrected error semantics (for example
`NoRowFoundError`), and filter / cascade / uniqueness contracts that statements
rely on. Assert through `service.AllStatements` only — no dialect SQL.

**Cursor / keyset pagination:** the battle matrix lives in
`stmttest/cursor_*.go` (harness + drain / adversarial / destructive packs).
A new `List*` method must seed fixtures and call `drainIncarnation` from
`TestCursorBattle_DrainAllListIncarnations` (ASC+DESC drain and NextCursor
emission are owned by that helper). Nullable sort columns need an additional
drain case; invalid/mismatched cursors and mid-page mutations are covered by
the shared special suites for representative paths.

`stmttest/schema_nullability_test.go` cross-checks bound columns' `Nullable`
flags against the live DDL (dialect `LiveColumnNullability` helpers walk
`information_schema` / `pragma_table_xinfo`). Coverage is what the suite
enumerates: a new schema must be added to its column list (shared schemas in
`sharedSchemaColumns`, per-dialect schemas in the dialect's
`SchemaColumnNullability`) or its columns go unchecked. `Computed` bindings name
no column and are skipped; sqlite and spanner also exclude
`sessions.expires_at`, a generated column that always introspects as nullable.

Dialect packages keep **engine-specific** tests only: compiler SQL shape, error
wrapping, `withTransaction` nesting, and migration/DDL smoke. Do not duplicate
upward: if `stmttest` can assert the behavior, put it there — not a per-dialect
copy of the same scenario.

Bring-up tags: `postgres_integration`, `spanner_integration`,
`sqlite_integration`.

[`dbtest.Pool`](dbtest/dbtest.go) is the migrated bring-up type
(`database.Pool` + `service.Pool`). Cursor-tie project seeding uses dialect
`SeedProjectsTiedAt` helpers under the integration build tags.