fix(db): exclude kind:30179 ciphertext from brownfield FTS - #6822
Conversation
Kind 30179 (NIP-PMA private managed agent) joined AUTHOR_ONLY_KINDS in #4593, but only fresh installs stopped indexing it: migration 0008's positive allowlist never touches populated databases, and 0014 wrapped the retained brownfield expression for 30350 alone. A relay upgraded in place still tokenizes the NIP-44 ciphertext into events.search_tsv. Add migration 0033 in the 0014 shape: capture the current generated expression and re-add search_tsv wrapped with kind = 30179 -> NULL, so every other kind keeps whatever policy the database already had. Mirror the exclusion in schema/schema.sql. Tests: extend the populated-upgrade migration test with a 30179 row and a pre-0033 checkpoint (fails without the new file); assert 0033's shape in the embedded-migrator test; apply 0033 in the buzz-search FTS setup. Co-authored-by: Meli <5aaa86bce934fc3445fc254aab560a40923f10252f92107e665073dede0e04d3@buzz.block.builderlab.xyz> Signed-off-by: Meli <5aaa86bce934fc3445fc254aab560a40923f10252f92107e665073dede0e04d3@buzz.block.builderlab.xyz>
Chessing234
left a comment
There was a problem hiding this comment.
the gap is real and well-traced — 0008 only guards fresh installs, 0014 wrapped 30350 alone, and schema/schema.sql still carried the negative skip-set without 30179 — and copying 0014's pg_get_expr shape is right, since it preserves whatever policy each database already had rather than flattening everyone onto one expression.
the thing i'd want in the pr before it ships is an honest statement of what it costs to run, because "no heap-wide policy rewrite" isn't accurate.
ALTER TABLE events DROP COLUMN search_tsv;
ALTER TABLE events ADD COLUMN search_tsv TSVECTOR GENERATED ALWAYS AS (...) STORED;
CREATE INDEX idx_events_search_tsv ON events USING GIN (search_tsv);adding a stored generated column makes postgres compute the value for every existing row, which is a full table rewrite under ACCESS EXCLUSIVE — the drop is cheap, the add is not. then the GIN index is built from scratch, also blocking, and since migrations run inside a transaction CONCURRENTLY isn't available. on a relay whose events table is the biggest thing in the database, that's the whole relay stopped for the duration, and there's no lock_timeout so it queues behind (and then blocks) every reader.
0014 has the same shape, so this isn't a new sin — but 0014 ran when the tables were smaller, and the pr's framing invites an operator to apply this without a maintenance window. i'd either say plainly "this rewrites events, expect downtime proportional to table size" in the migration header, or use the online shape: add a plain (non-generated) column, backfill in batches, build the index CONCURRENTLY outside the transaction, then swap. the first is fine if someone decides the tables are small enough; it just has to be a decision.
two concrete things:
the index is recreated from a literal, not from what was there. DROP COLUMN takes every index on search_tsv with it, and the migration puts back exactly one, idx_events_search_tsv ... USING GIN (search_tsv). that matches schema/schema.sql:279 today, so it's correct for a stock deployment — but any relay that added a second index, a partial index, or GIN storage parameters (fastupdate, gin_pending_list_limit) loses them silently, and unlike the generated expression those aren't captured and replayed. worth a line in the header saying the index definition is assumed stock, or capturing it from pg_indexes the same way the expression is captured from pg_attrdef.
the schema.sql comment changed provenance without explanation. it went from
-- Keep in sync with migrations (final state: 0001 + 0005 + 0009).
to 0001 + 0005 + 0014 + 0033. adding 0033 is obviously right and adding 0014 looks like a correction, but 0009 was dropped from the list in the same edit and 0009_nip_rs_database_guards.sql doesn't touch search_tsv, so i think the old comment was simply wrong. worth saying so in the pr, because a reviewer diffing that line sees a reference disappear and can't tell whether it was a fix or a slip.
smaller: assert_eq!(migrations.len(), 33) plus migrations[32] indexing means every future migration edits this test in two places and any reordering silently retargets the index assertions. matching on version == 33 rather than position would survive that — not this pr's problem to fix, but it's growing.
the migration itself reads correct to me: pg_get_expr against pg_attrdef keyed on attname is the right lookup, the RAISE EXCEPTION on a missing expression fails the migration rather than silently installing a bare expression, and the run_to(32) step in the test genuinely pins the brownfield gap rather than just the end state. that middle assertion is the best part of the diff.
The 0033 header claimed the wrap-the-expression shape avoids a heap rewrite. It avoids a *policy* rewrite; it does not avoid a heap rewrite. DROP COLUMN + ADD ... GENERATED ... STORED recomputes every row under ACCESS EXCLUSIVE and rebuilds the GIN index in-transaction (no CONCURRENTLY), with no lock_timeout — downtime proportional to events size, and the index is recreated from the stock definition only. Say so in the migration header so operators of large brownfield databases can schedule a window instead of discovering it live. Comment-only change; no SQL semantics touched. Raised by Carl in review of #6822. Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
…h-coordinator * origin/main: fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822)
…arer-auth * origin/main: fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822) # Conflicts: # crates/buzz-db/src/migration.rs
…arer-auth * origin/main: fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> # Conflicts: # crates/buzz-db/src/migration.rs
…at-vacuum * origin/main: fix(projects): allow owners to delete agent projects (#6533) Fade expanded video controls on hover (#6926) fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822) fix(client): resurface hidden DMs from live activity (#6885) fix(desktop): keep the draft space when typing right after a mention pick (#6875) broker: define the agent-to-broker action contract (#6742) fix(desktop): keep project sheets independent from threads (#6901) Add gated security reviews (#6816) fix(desktop): accent-colored mention badges that count thread mentions (#6900) Add Buzz benchmark evaluation layers (#6823) fix(desktop): show edited head content in thread panel (#6887) fix(desktop-tooltip): increase surface contrast (#6897) Deduplicate ACP thread prompt context (#6706) Apply access policy when reusing channel agents (#6838) feat(sidebar): prioritize unread DMs in overflow navigation (#6842) feat(projects): add agent and CLI project-home support (#6590) Signed-off-by: Luke Tornquist <tornquist@squareup.com>
…agent-edit * origin/main: (39 commits) chore(deps): update dependency vitest to v4.1.11 (#6667) chore(deps): update dependency @tanstack/react-virtual to v3.14.10 (#6666) chore(deps): update ubuntu:24.04 docker digest to 33ceb71 (#6664) fix(projects): allow owners to delete agent projects (#6533) Fade expanded video controls on hover (#6926) fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822) fix(client): resurface hidden DMs from live activity (#6885) fix(desktop): keep the draft space when typing right after a mention pick (#6875) broker: define the agent-to-broker action contract (#6742) fix(desktop): keep project sheets independent from threads (#6901) Add gated security reviews (#6816) fix(desktop): accent-colored mention badges that count thread mentions (#6900) Add Buzz benchmark evaluation layers (#6823) fix(desktop): show edited head content in thread panel (#6887) fix(desktop-tooltip): increase surface contrast (#6897) Deduplicate ACP thread prompt context (#6706) Apply access policy when reusing channel agents (#6838) feat(sidebar): prioritize unread DMs in overflow navigation (#6842) feat(projects): add agent and CLI project-home support (#6590) feat(desktop): restore message quick reactions (#6892) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…late-cardinality-hints * origin/main: (145 commits) chore(deps): update rui314/setup-mold digest to 7e4f20a (#6663) chore(deps): update dependency vitest to v4.1.11 (#6667) chore(deps): update dependency @tanstack/react-virtual to v3.14.10 (#6666) chore(deps): update ubuntu:24.04 docker digest to 33ceb71 (#6664) fix(projects): allow owners to delete agent projects (#6533) Fade expanded video controls on hover (#6926) fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822) fix(client): resurface hidden DMs from live activity (#6885) fix(desktop): keep the draft space when typing right after a mention pick (#6875) broker: define the agent-to-broker action contract (#6742) fix(desktop): keep project sheets independent from threads (#6901) Add gated security reviews (#6816) fix(desktop): accent-colored mention badges that count thread mentions (#6900) Add Buzz benchmark evaluation layers (#6823) fix(desktop): show edited head content in thread panel (#6887) fix(desktop-tooltip): increase surface contrast (#6897) Deduplicate ACP thread prompt context (#6706) Apply access policy when reusing channel agents (#6838) feat(sidebar): prioritize unread DMs in overflow navigation (#6842) feat(projects): add agent and CLI project-home support (#6590) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…c-agent-commit-identity * origin/main: chore(deps): update rui314/setup-mold digest to 7e4f20a (#6663) chore(deps): update dependency vitest to v4.1.11 (#6667) chore(deps): update dependency @tanstack/react-virtual to v3.14.10 (#6666) chore(deps): update ubuntu:24.04 docker digest to 33ceb71 (#6664) fix(projects): allow owners to delete agent projects (#6533) Fade expanded video controls on hover (#6926) fix(db): exclude kind:30179 ciphertext from brownfield FTS (#6822) fix(client): resurface hidden DMs from live activity (#6885) fix(desktop): keep the draft space when typing right after a mention pick (#6875) broker: define the agent-to-broker action contract (#6742) fix(desktop): keep project sheets independent from threads (#6901) Add gated security reviews (#6816) fix(desktop): accent-colored mention badges that count thread mentions (#6900) Add Buzz benchmark evaluation layers (#6823) fix(desktop): show edited head content in thread panel (#6887) fix(desktop-tooltip): increase surface contrast (#6897) Deduplicate ACP thread prompt context (#6706) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Problem
Kind
30179(NIP-PMA private managed agent — NIP-44 ciphertext carrying the agent nsec, env vars, prompt) joinedAUTHOR_ONLY_KINDSin #4593, but only fresh installs stopped indexing it:migrations/0008installs the positive FTS allowlist only wheneventsis empty;migrations/0014wraps the retained brownfield expression for30350alone;schema/schema.sqlstill carries the legacy negative skip-set without30179.So a relay upgraded in place keeps tokenizing
30179ciphertext intoevents.search_tsv. Found by Wren while reviewing #4999 (which activates publication of 30179 from Desktop). No readable leak:/queryappliessearch_hit_accepted+event_visible_to_readerbefore serialization and live foreign searches against builderlab returned[]— this is the storage-layer privacy invariant (docs/nips/NIP-PMA.mddeployment step 2) plus wasted FTS work.Fix
migrations/0033_private_managed_agent_fts.sql— same shape as 0014: capture the current generated expression viapg_get_expr, drop/re-addsearch_tsvwrapped withkind = 30179 → NULL. Every other kind keeps whatever policy the database already had (fresh allowlist or brownfield skip-set). Rebuilds the GIN index; no heap-wide policy rewrite.schema/schema.sql— add30179to the desired-state skip-set.Tests
populated_upgrade_preserves_search_policy_except_for_private_kinds(renamed from…_push_leases): provisions the legacy negative expression (migrations 1–7), inserts kind 1 / 30179 / 30350 rows, checkpoints after 0014 (30179 still searchable, 30350 not), then runs to head and asserts 30179 is NULL. Proven red with the 0033 file removed:left: [(1, Some(true)), (30179, Some(true)), (30350, None)].embedded_migrator_contains_consolidated_initial_schema: count 32→33, asserts 0033 shape andschema.sqlparity.buzz-search/tests/fts_integration.rssetup applies 0033 so the FTS tripwires run against the real chain.Verification (HEAD 6154cc3, same shell)
cargo test -p buzz-db -- --include-ignored --test-threads=1: 305 passed / 3 failed — the 3 (create_community_with_owner_enforces_per_owner_limit,insert_mentions_indexes_rosters_past_bind_parameter_cap,transfer_ownership_returns_limit_reached_for_maxed_transferee) fail identically on pristinemainf249710 with a fresh DB; unrelated to this diff.cargo test -p buzz-search -- --include-ignored: 19 passed (incl.author_only_kinds_are_storage_level_unsearchable,p_gated_persistent_kinds_have_storage_null_tsvector).desired_state_schema_bootstrap_progresses_beyond_fencing+run_migrations_applies_consolidated_initial_schema_on_fresh_database: pass.cargo fmt --all --checkclean;cargo clippy -p buzz-db -p buzz-search --all-targets -D warningsclean.Independent of #4999 — both main and #4999 are brownfield-exposed today; this lands either order.