OpenAPI follow-ups: parameters-block parity, overload merge, per-role privileges cache (#53)

[milmazz]

Closes items 2–4 of #53 (item 1 follows in a stacked PR).

What's here

Parameters-block parity with PostgREST v14.12 (Response/OpenAPI.hs, cited per change in the code):

• preferParams shared definition (no enum key — v14.12 suppresses the empty enum) and its $ref on every RPC POST path item (OpenAPI.hs#L219-226)
• RPC args body param required: true (L222)
• preferPost enum extended with the resolution=* values (L184-186, L234)
• on_conflict shared definition (L242-248)
• PATCH/DELETE parameter lists no longer carry select (L336-339)
• rowFilter.* definitions carry description (column COMMENT) instead of format (L299-308)

Overloaded-RPC path-item merge (#53 item 2): one /rpc/<fn> path item; the most-parameters overload supplies it entirely, mirroring PostgREST's overload sort + last-insert-wins assembly (SchemaCache.hs#L292, Routine.hs#L89, OpenAPI.hs#L381).

Per-role privileges cache (#53 item 4): GET / under openapi-mode = follow-privileges no longer runs two catalog queries per request. New Bier.PrivilegesCache (GenServer-owned public ETS, tid via persistent_term) caches per {instance, role}, stamped with a new generation ref on the %Bier.SchemaCache{} snapshot — a schema-cache reload invalidates with zero coordination, matching PostgREST's reload-time freshness. Hardened against table-gone races following the Bier.JwtCache pattern.

Notes

• No conformance case pins any changed shape (verified before implementation); the frozen spec/ / test/support/ / test/conformance/ trees are untouched. Unit-test corrections in test/bier/openapi_test.exs each carry their PostgREST source citation.
• mix precommit green: 864 passed, 5 excluded.
• Remaining verified-but-uncased wire gaps (root "/" path item, RPC produces, default host/basePath) are tracked in a follow-up issue rather than here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vdh3cbYLtKJfGkZ1ijrYGt

[milmazz]

@claude review once.

[claude]

Claude finished @milmazz's task in 4m 14s —— View job

---

Review complete

• Gather context (PR body, changed files, CLAUDE.md)
• Review OpenAPI parameters-block parity changes (lib/bier/openapi.ex)
• Review overloaded-RPC path-item merge logic
• Review Bier.PrivilegesCache (new GenServer/ETS cache)
• Review action_controller.ex / schema_cache.ex integration
• Review test coverage
• Post final review feedback

Note: this sandbox has no mix/elixir toolchain installed, so I could not run mix precommit/mix test myself — review is based on static reading of the diff (git diff origin/main...HEAD) against CLAUDE.md's stated contract.

Summary

This is a clean, well-cited PR. Each behavioral change traces to a specific PostgREST v14.12 source line, and the PR body correctly notes no frozen spec//conformance case pins any of these shapes, so nothing risks the conformance freeze.

lib/bier/openapi.ex — parameters-block parity

• preferParams (no enum), preferPost resolution values, on_conflict shared def, args required: true, dropped select from PATCH/DELETE, and rowFilter.* description instead of format — all match their cited OpenAPI.hs lines and are exercised by the new assertions in test/bier/openapi_test.exs.
• Minor observation: the new on_conflict shared definition (openapi.ex#L317-322) is added to the top-level parameters map but nothing in relation_path_item/1 actually $refs it from any table's POST parameter list — it's defined but currently unreachable from any path item. If that mirrors a genuine PostgREST quirk (definition present, never referenced) that's fine and consistent with the "wire-format match is the contract" goal in the moduledoc, but worth a quick sanity check against a real postgrest OpenAPI dump for a table with a unique constraint, since this is one of the shapes explicitly called out as not covered by any conformance case.

Overloaded-RPC merge (function_paths/1, winning_overload/2)

• Grouping by name and picking the overload with the most in_params correctly fixes the old behavior (arbitrary Map.new overwrite order for same-named overloads). The comment is upfront that parameter-count ties fall back to stable-sort input order as an approximation of PostgREST's full Ord tuple — reasonable given real overload sets differ in arity, and it's clearly flagged rather than silently assumed exact.
• Good test coverage: both input orderings for the tie-break, plus the volatility-drops-GET case.

Bier.PrivilegesCache (new)

• Solid mirror of the existing Bier.JwtCache "public ETS + persistent_term tid" pattern, including the crash/restart race handling (table_gone rescue, terminate/2 erasing the persistent_term key).
• Correctly bounded: since the ETS table is a :set keyed by role (not {role, generation}), each role has exactly one row that gets overwritten on generation mismatch, so there's no unbounded growth across repeated schema-cache reloads — just one entry per distinct role ever seen, matching the small/fixed set of Postgres roles in practice.
• fetch/4's nil generation short-circuit (bypass cache before the first schema-cache load) is a nice touch and is tested.
• Good coverage in test/bier/privileges_cache_test.exs for the per-role/per-generation hit/miss matrix, the no-table fallback, and the stale-tid-after-crash fallback.

action_controller.ex / schema_cache.ex

• generation: make_ref() stamped once per snapshot swap and threaded through filter_by_mode/6 into the cache lookup is straightforward and matches the existing snapshot-per-request pattern already used elsewhere in the file.
• No functional concerns here beyond what's inherent to using the previously-fetched cache.generation for the whole request (same staleness window that already exists for the rest of the snapshot).

Docs/misc

• CLAUDE.md/README.md updates accurately describe the new boot-order and follow-privileges caching behavior.
• mix.lock mint 1.9.2 → 1.9.3 bump (CVE fix) is an unrelated but reasonable drive-by include.
• The two new docs/superpowers/plans/*.md files match this repo's existing convention of committing implementation plans alongside the PRs that execute them; the openapi-30-emitter one is scoped to the next stacked PR (item 1) and is informational only here.

No blocking issues found. The on_conflict reachability point above is worth a quick look but doesn't block merge given it's explicitly flagged as unverified-by-conformance in the PR description.

[milmazz]

Verified the on_conflict reachability question against the v14.12 source (authoritative for the wire contract, and stronger than a single dump since it proves the behavior is unconditional):

• grep -n on_conflict src/PostgREST/Response/OpenAPI.hs (tag v14.12) → exactly one occurrence: the makeParamDefs entry at L242. There is no Ref (Reference "on_conflict") anywhere — postOp's parameter list is ["body." <> tn, "select", "preferPost"] (L333).
• PostgREST's own test/spec/Feature/OpenApi/OpenApiSpec.hs never mentions on_conflict either.

So "definition present in the shared parameters block, referenced by no path item" is PostgREST's genuine (if quirky) v14.12 behavior, independent of fixtures or unique constraints — mirroring it verbatim is the intended wire-format match. No change needed.

🤖 Generated with Claude Code