-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(hooks): PostgresPredicateStateBackend (durable backend PR 2/4, replaces #3932) #3933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3aac87a
feat(hooks): PostgresPredicateStateBackend (durable backend PR 2/4)
zmanian 806eacb
fix(hooks-postgres): take victim-key advisory lock during scope-LRU e…
zmanian a784b0d
refactor(hooks-postgres): make migration SQL the single source via in…
zmanian 0c102a6
fix(hooks-postgres): rank LRU eviction victims by MIN(ts), matching i…
zmanian ef93722
refactor(hooks-postgres): canonical typed two-table predicate schema
zmanian 741e0ec
fix(hooks-postgres): typed RecordPlan + fail-closed scope quota + ass…
zmanian 7547ebf
fix(hooks-postgres): index-only scope-quota cover, u64 hash length pr…
zmanian 2ee6f0f
fix(hooks-pg): address henrypark133 review on #3933
zmanian 34d1d60
fix(hooks-pg): sanitize quota fail-closed messages + cutoff overflow …
zmanian da1aaaf
docs(hooks-pg): make dual-backend compliance explicit (C-1)
zmanian ebec0ba
fix(hooks-pg): round-2 review fixes for predicate state backend (#3933)
zmanian 3528357
Merge main into hooks predicate postgres backend
zmanian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # Agent Map — ironclaw_hooks_postgres | ||
|
|
||
| ## Start Here | ||
|
|
||
| - No crate-local `CLAUDE.md` exists yet; use this map plus the contracts below. | ||
| - Read `src/lib.rs` first — it documents why this is a separate crate, the | ||
| dual-backend split (Postgres here, libSQL in `ironclaw_hooks_libsql`, parity | ||
| in `ironclaw_hooks_parity`), and the shared two-table typed schema. | ||
| - Read `Cargo.toml` for actual dependencies and the `postgres` feature gate. | ||
| - The trait contract this crate implements lives in | ||
| `ironclaw_hooks::predicate_state` (`PredicateStateBackend`); the shared | ||
| contract harness is exposed via that crate's `contract-tests` feature. | ||
|
|
||
| ## What This Crate Owns | ||
|
|
||
| - The durable PostgreSQL `PredicateStateBackend` implementation: the atomic | ||
| record-and-read transaction body, per-key advisory-lock serialization, the | ||
| fail-closed per-key sample cap, and the per-scope LRU quota enforcement. | ||
| - The Postgres predicate-state schema (`hooks_predicate_invocations` / | ||
| `hooks_predicate_values`) and its migration SQL. | ||
| - Crate-local public API, tests, and fixtures needed to prove that ownership. | ||
|
|
||
| ## Do Not Move In Here | ||
|
|
||
| - The `PredicateStateBackend` trait itself, the in-memory backend, or the | ||
| libSQL backend (those live in `ironclaw_hooks` / `ironclaw_hooks_libsql`). | ||
| - Evaluator policy, hook-framework wiring, or backend selection. | ||
| - Secrets, raw connection strings, host names, schema details, or raw DB | ||
| error text in errors, events, logs, or docs — `PredicateBackendError` | ||
| payloads are sanitized (`DB_UNAVAILABLE_MSG` and the quota fail-closed | ||
| constants); keep the raw error behind `tracing` only. | ||
|
|
||
| ## Validation | ||
|
|
||
| - Fast local check: `cargo test -p ironclaw_hooks_postgres` | ||
| - Postgres-backed integration/adversarial tests are env-gated on | ||
| `IRONCLAW_HOOKS_POSTGRES_URL` / `DATABASE_URL` and skip (passing) when no | ||
| DB is reachable; run them against a live Postgres to exercise the advisory | ||
| locks, cap fail-closed, and LRU eviction paths. | ||
| - If production persistence behavior changes, keep PostgreSQL/libSQL parity: | ||
| update the libSQL counterpart (`ironclaw_hooks_libsql`) and the | ||
| cross-backend parity suite (`ironclaw_hooks_parity`) in lockstep. | ||
|
|
||
| ## Agent Notes | ||
|
|
||
| - Keep edits inside this crate unless the trait contract in `ironclaw_hooks` | ||
| explicitly requires a neighboring crate change. | ||
| - The advisory-lock protocol is load-bearing: same-key writers serialize on a | ||
| per-key `(int4,int4)` lock, scope-quota passes serialize on a per-scope | ||
| `(int8)` lock folded with a kind byte, and victim eviction uses the | ||
| NON-blocking `pg_try_advisory_xact_lock` to stay deadlock-free. Do not change | ||
| lock derivation, isolation level, or fail-closed posture without updating the | ||
| module-level docs and the disjointness/lock-equality unit tests. | ||
| - Cap and quota are fail-closed by contract: overflow returns | ||
| `WindowOverflow`, an unenforceable scope quota returns `Unavailable`. Never | ||
| silently drop-oldest or commit an over-quota scope. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| [package] | ||
| name = "ironclaw_hooks_postgres" | ||
| version = "0.1.0" | ||
| edition = "2024" | ||
| publish = false | ||
| description = "Durable PostgreSQL-backed PredicateStateBackend for the reborn hook framework (durable backend PR 2/4)." | ||
| authors = ["NEAR AI <support@near.ai>"] | ||
| license = "MIT OR Apache-2.0" | ||
| homepage = "https://github.com/nearai/ironclaw" | ||
| repository = "https://github.com/nearai/ironclaw" | ||
|
|
||
| [features] | ||
| default = [] | ||
| # The Postgres backend itself. Mirrors the per-crate `postgres` feature gate | ||
| # used by `ironclaw_reborn_event_store` / `ironclaw_filesystem`: nothing in | ||
| # this crate compiles a DB dependency unless `postgres` is on. | ||
| postgres = ["dep:deadpool-postgres", "dep:tokio-postgres"] | ||
|
|
||
| [dependencies] | ||
| async-trait = "0.1" | ||
| blake3 = "1" | ||
| chrono = { version = "0.4", features = ["serde"] } | ||
| deadpool-postgres = { version = "0.14", optional = true } | ||
| ironclaw_hooks = { path = "../ironclaw_hooks" } | ||
| ironclaw_host_api = { path = "../ironclaw_host_api" } | ||
| rust_decimal = { version = "1", features = ["serde", "serde-with-str", "db-tokio-postgres"] } | ||
| thiserror = "2" | ||
| tokio-postgres = { version = "0.7", optional = true, features = ["with-chrono-0_4"] } | ||
| tracing = "0.1" | ||
|
|
||
| [dev-dependencies] | ||
| # `contract-tests` exposes the trait-level contract harness from | ||
| # `ironclaw_hooks` so this crate's Postgres impl runs the SAME suite the | ||
| # in-memory backend runs (proven-by-construction, not per-impl). | ||
| ironclaw_hooks = { path = "../ironclaw_hooks", features = ["contract-tests"] } | ||
| tokio = { version = "1", features = ["macros", "rt", "rt-multi-thread", "sync"] } | ||
| uuid = { version = "1", features = ["v4"] } | ||
93 changes: 93 additions & 0 deletions
93
crates/ironclaw_hooks_postgres/migrations/V1__predicate_state.sql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| -- Durable predicate sliding-window state for the reborn hook framework. | ||
| -- | ||
| -- This crate owns its own schema (per-crate pattern, like | ||
| -- ironclaw_reborn_event_store / ironclaw_filesystem) rather than going | ||
| -- through the legacy main-binary refinery `migrations/` directory. The | ||
| -- DDL is embedded verbatim into `schema.rs` via `include_str!` and applied | ||
| -- as an idempotent `CREATE TABLE IF NOT EXISTS` batch by `run_migrations()`; | ||
| -- this file is the single human-reviewable canonical source for that schema. | ||
| -- | ||
| -- ## Canonical typed two-table shape (cross-backend invariant) | ||
| -- | ||
| -- The two durable backends (Postgres + libSQL) share ONE logical schema: | ||
| -- two typed tables — one for invocation-count samples, one for | ||
| -- numeric-value samples — with identical column names and semantics. The | ||
| -- storage TYPES differ per backend (Postgres uses native TIMESTAMPTZ + | ||
| -- NUMERIC; libSQL uses epoch-ms INTEGER + TEXT) but the table count, column | ||
| -- names, primary keys, and eviction/dedup/quota semantics are identical. | ||
| -- The cross-backend parity suite (ironclaw_hooks_parity) proves they are | ||
| -- behaviorally interchangeable. Replacing the earlier single | ||
| -- `hook_predicate_counters(kind CHAR(1), …)` table with two explicit typed | ||
| -- tables removes the `kind` discriminator AND the `value: Option<Decimal>` | ||
| -- "count smuggled through a NUMERIC" abstraction the shared record path used. | ||
| -- | ||
| -- ## Hash columns | ||
| -- | ||
| -- `scope_hash` and `key_hash` are blake3 digests (32 raw bytes, BYTEA): | ||
| -- scope_hash = blake3(len-prefixed tenant_id) -- tenant grain | ||
| -- key_hash = blake3(map-discriminant ++ hook_id ++ tenant_id | ||
| -- ++ capability [++ field]) -- full bucket | ||
| -- `scope_hash` is the trust boundary + per-tenant LRU-quota grain; `key_hash` | ||
| -- is the full bucket identity (the dedup + count/sum grain). BYTEA (not TEXT) | ||
| -- keeps the index keys fixed-width and avoids collation surprises. | ||
| -- | ||
| -- ## event_id column type (codex #3635 finding) | ||
| -- | ||
| -- The replay-dedup id is a `PredicateEventId` — an opaque host-assigned | ||
| -- string whose canonical synth shape is a 64-char blake3 hex digest, but | ||
| -- callers may stamp other formats. Postgres `uuid` is a fixed 128-bit type | ||
| -- and will REJECT a 64-char hex digest, so `event_id` is `TEXT`, NOT `uuid`. | ||
| -- (#3635 docs pinned a 64-char id while the old schema said uuid; TEXT | ||
| -- resolves that contradiction, and matches the libSQL sibling.) | ||
| -- | ||
| -- ## Window-clock basis | ||
| -- | ||
| -- `occurred_at` is the wall-clock timestamp passed by the caller | ||
| -- (TIMESTAMPTZ). Window comparisons are performed against a caller-supplied | ||
| -- cutoff computed from the same clock the in-memory backend uses, so the trim | ||
| -- semantics (`occurred_at < cutoff`, entry at exact cutoff retained) match the | ||
| -- in-memory backend bit-for-bit. See `schema.rs` for the DB-clock rationale. | ||
|
|
||
| -- Invocation-count samples. One row per recorded invocation event; the | ||
| -- in-window COUNT(*) is the invocation count. | ||
| CREATE TABLE IF NOT EXISTS hooks_predicate_invocations ( | ||
| scope_hash BYTEA NOT NULL, | ||
| key_hash BYTEA NOT NULL, | ||
| event_id TEXT NOT NULL, | ||
| occurred_at TIMESTAMPTZ NOT NULL, | ||
| PRIMARY KEY (key_hash, event_id) | ||
| ); | ||
|
|
||
| -- Per-key window-trim + COUNT scan: every record_invocation prunes and | ||
| -- aggregates over (key_hash, occurred_at). | ||
| CREATE INDEX IF NOT EXISTS hooks_predicate_invocations_key_ts_idx | ||
| ON hooks_predicate_invocations (key_hash, occurred_at); | ||
| -- Per-scope (tenant) distinct-key LRU eviction. enforce_scope_quota runs | ||
| -- COUNT(DISTINCT key_hash) and ranks victims by MIN(occurred_at) per key, | ||
| -- both scoped by scope_hash; the (scope_hash, key_hash, occurred_at) cover | ||
| -- lets those run as index-only scans for tenants with many recorded keys. | ||
| CREATE INDEX IF NOT EXISTS hooks_predicate_invocations_scope_idx | ||
| ON hooks_predicate_invocations (scope_hash, key_hash, occurred_at); | ||
| -- Operator reaper (`evict_older_than`) deletes globally by age. | ||
| CREATE INDEX IF NOT EXISTS hooks_predicate_invocations_ts_idx | ||
| ON hooks_predicate_invocations (occurred_at); | ||
|
|
||
| -- Numeric-value samples. One row per recorded value event; the in-window | ||
| -- SUM(value) is the running sum. `value` is NOT NULL here (the typed tables | ||
| -- make the count-vs-sum distinction explicit, so no nullable double-duty). | ||
| CREATE TABLE IF NOT EXISTS hooks_predicate_values ( | ||
| scope_hash BYTEA NOT NULL, | ||
| key_hash BYTEA NOT NULL, | ||
| event_id TEXT NOT NULL, | ||
| occurred_at TIMESTAMPTZ NOT NULL, | ||
| value NUMERIC NOT NULL, | ||
| PRIMARY KEY (key_hash, event_id) | ||
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS hooks_predicate_values_key_ts_idx | ||
| ON hooks_predicate_values (key_hash, occurred_at); | ||
| -- Same index-only-scan cover for the value table's scope-quota pass. | ||
| CREATE INDEX IF NOT EXISTS hooks_predicate_values_scope_idx | ||
| ON hooks_predicate_values (scope_hash, key_hash, occurred_at); | ||
| CREATE INDEX IF NOT EXISTS hooks_predicate_values_ts_idx | ||
| ON hooks_predicate_values (occurred_at); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Low —
crates/AGENTS.mdstates "Every crate with Cargo.toml has a crate-local AGENTS.md" and directs agents to treat it as the first file to load. Sibling durable-backend cratesironclaw_reborn_event_storeandironclaw_filesystemship one;ironclaw_hooks_postgresdoes not.Fix: Add
crates/ironclaw_hooks_postgres/AGENTS.mdfollowing the shape used byironclaw_reborn_event_store/AGENTS.md(What This Crate Owns / Do Not Move In Here / Validation / Agent Notes).New finding — not raised in prior reviews.