Conversation
|
Review complete. No issues found — approved ✅. This PR overhauls the GraphQL build cache. The change is functionally coherent and the scoping/pool-cleanup wiring is consistent, but the adjudicated review surfaced three lower-confidence concerns that did not clear the reporting threshold: a TOCTOU window where an in-flight build can be re-published after a flush, the compute loader being awaited on every cache-hit request, and a nullish
Reviewed commit: 9f64b13 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
|
Followed up on the three observations in the approval summary:
Validation: 6 focused invalidation tests on this branch; the final dependency stack passes all 84 graphile-cache tests, all 8 server flush tests, and the graphile-cache build. |
1 participant
Implements F17 from constructive-io/constructive-planning#1693, stacked on #1747 (F04).
A logical service key can resolve to different databases, schemas, roles, plugin settings, or compute bindings. Server and Explorer snapshot their effective build inputs and use a domain-separated, process-local HMAC fingerprint for cache reuse. Functions and opaque objects retain reference identity; credentials stay out of cache keys. The captured inputs also construct the preset.
Service, database, and physical-pool invalidation use explicit entry metadata to remove every build variant. Public schema notifications invalidate by database before hostname lookup. Existing scoped-introspection wiring is preserved.
The server reuses the API, pool, service key, module loader, and PostgreSQL settings owned by
req.constructive. Missing request claims were added to the express-context owner, including current request protection, principal/read-only credentials, trusted private identity headers, and anonymous database attribution. Shared schemas read these settings per request. The canonical errors package retains internal causes and registered HTTP status/code while excluding private messages and context./flushauthenticates and invalidates before starting a Graphile build.Validation: 130 express-context tests; 50 graphile-cache tests at this layer; targeted error/snapshot/response/observability tests; seven real PostgreSQL/scoped-routing HTTP scenarios replace the mocked middleware tests. Server, Explorer, context and error-package builds passed. Final stack validation is recorded in #1855.
Dependency stack: #1747 → #1852 (F17) → #1853 (F19) → #1854 (F20) → #1855 (F21). F20 logically depends on F04/F17 and is stacked after F19 to share the admitted publication owner.