test(registry): baseline coverage for reader cutover (PR 3 of #3177)#3221
Merged
Conversation
bokelley
added a commit
that referenced
this pull request
Apr 25, 2026
Address code-reviewer + nodejs-testing-expert feedback on PR #3221: - by_tag selector: pin total_requested / total_authorized / coverage_percentage so PR 4 cannot silently change tag-counting semantics. Fix the comment that said "two requested" — the SQL counts properties matched by `tags && selector_tags`, so all three fixture properties match and requested is 3. - Rename "reports only property-level authorizations" to spell out the negative invariant ("returns [] when only publisher-level rows exist (no bleed into property-level reads)"). The contract is the load-bearing thing PR 4 must preserve. - New: getPropertiesForAgent ordering — pin ORDER BY (publisher_domain, property_type, name) across two publishers. - New: findAgentsForPropertyIdentifier ordering — pin ORDER BY (publisher_domain, agent_url) with two publishers and two agents per side. - New: findAgentsForPropertyIdentifier INNER JOIN behavior — orphan property with the matching identifier and no agent_property_authorizations row must not surface. Catches a silent LEFT JOIN swap in the cutover. - New: bulkGetFirstAuthForAgents('*') — pin the bulk path on the literal wildcard so PR 4 cannot special-case it differently from the single-agent reader. All 64 tests pass alone; 104 pass alongside registry-feed, registry-search, registry-overlay-schema in a single vitest run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pins the I/O of the current federated-index + property-registry reader functions before PR 4 swaps them onto the new publishers / adagents_authorization_overrides schema (#3195). Tests-only, no production changes — same fixtures must produce identical responses across the cutover. Four new integration files: - registry-reader-baseline-properties.test.ts: getPropertiesForDomain, getDiscoveredPropertiesByDomain, getPropertiesForAgent, getPublisherDomainsForAgent, findAgentsForPropertyIdentifier, hasValidAdagents, getAllPropertiesForRegistry, getPropertyRegistryStats, getStats lower-bounds. - registry-reader-baseline-authorizations.test.ts: getAgentsForDomain, getDomainsForAgent, getAgentAuthorizationsForDomain, bulkGetFirstAuthForAgents (incl. source-priority contract), validateAgentForProduct (all/by_id/by_tag selectors + source reporting), wildcard-agent ('*') literal handling. - registry-reader-baseline-public-endpoints.test.ts: supertest against /registry/agents (with and without ?properties=true), /registry/publishers, /registry/publisher, /registry/operator, /registry/stats, /registry/feed envelope smoke. - registry-reader-baseline-mcp.test.ts: lookup_domain (separates adagents_json vs agent_claim) and list_publishers (dedupe by domain). Each file uses a disjoint domain prefix (prop-/auth-/endpoint-/mcp-) under *.registry-baseline.example so the four can run in parallel with each other and with sibling registry-* tests without trampling state. Stats and registry-view assertions are lower-bounds where shared-DB residue would otherwise cause flakes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address code-reviewer + nodejs-testing-expert feedback on PR #3221: - by_tag selector: pin total_requested / total_authorized / coverage_percentage so PR 4 cannot silently change tag-counting semantics. Fix the comment that said "two requested" — the SQL counts properties matched by `tags && selector_tags`, so all three fixture properties match and requested is 3. - Rename "reports only property-level authorizations" to spell out the negative invariant ("returns [] when only publisher-level rows exist (no bleed into property-level reads)"). The contract is the load-bearing thing PR 4 must preserve. - New: getPropertiesForAgent ordering — pin ORDER BY (publisher_domain, property_type, name) across two publishers. - New: findAgentsForPropertyIdentifier ordering — pin ORDER BY (publisher_domain, agent_url) with two publishers and two agents per side. - New: findAgentsForPropertyIdentifier INNER JOIN behavior — orphan property with the matching identifier and no agent_property_authorizations row must not surface. Catches a silent LEFT JOIN swap in the cutover. - New: bulkGetFirstAuthForAgents('*') — pin the bulk path on the literal wildcard so PR 4 cannot special-case it differently from the single-agent reader. All 64 tests pass alone; 104 pass alongside registry-feed, registry-search, registry-overlay-schema in a single vitest run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
force-pushed
the
bokelley/registry-reader-tests
branch
from
3f83738 to
2cb0c5a
Compare
Address second-pass expert feedback on PR #3221: ad-tech-protocol-expert - Narrow the wildcard agent ('*') describe block to a single storage round-trip assertion. The AdCP 3.0 adagents.json schema requires authorized_agents[].url to be format:"uri" — '*' is an internal storage convention, not a protocol-defined value. Pinning cross-reader expansion-prevention semantics would lock PR 4 into a non-protocol contract; remove getDomainsForAgent expansion-prevention and bulkGetFirstAuthForAgents wildcard assertions, leaving only the storage round-trip. - Pin unauthorized_items absence for selection_type=all so PR 4 can't accidentally start emitting property IDs for the all case (which has different field-shape semantics from by_id and by_tag). adtech-product-expert - /registry/agents projection: pin protocol, discovered_from. publisher_domain, and added_date — the DSP-discovery breadcrumbs that get sourced from the bulk-auth join. - /registry/operator: replace the "no profile" empty-path-only test with a seeded test that pins the publisher-ops projection (member.{slug, display_name} + agents[].{url, name, type, authorized_by: [{publisher_domain, authorized_for, source}]}). PR 4 swaps getAuthorizationsForAgent under this projection, so it must round-trip identically. data-analyst - getAllPropertiesForRegistry precedence carve-out: previously untested. Seed a single domain with BOTH a public hosted row AND discovered_properties; assert exactly one row in the result, sourced from hosted, with the hosted authorized_agents count. This is the most non-trivial logic in the reader — the SQL carve-out (NOT IN public hosted_properties) — and PR 4 could silently invert or remove it. - Source priority test renamed to "verified over unverified" and agent_claim source pin added to the bulk-result test so PR 4 can't normalize source to verified/unverified or drop unverified agents. - JSONB extra-field containment: pin that an identifier with {type, value, region} still surfaces under findAgentsForProperty Identifier('domain', value). PR 4 may normalize identifiers to a (property_id, type, value) table and silently drop extras. - selection_type=by_id with property_ids:[] short-circuit pinned. - Source=none even when discovered_publishers has a row for the domain (vs. when domain is unseen): publisher-side cache must not bleed into agent-side source determination. Tests now: 66 pass alone; 106 pass alongside registry-feed, registry-search, registry-overlay-schema in a single vitest run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
bokelley
added a commit
that referenced
this pull request
Apr 26, 2026
… (#3244) * feat(registry): swap property readers to catalog UNION (PR 4a of #3177) Property-side readers in federated-index-db.ts and property-db.ts now union the catalog-side publishers cache (PR 1 of #3177) with the legacy discovered_properties / discovered_publishers tables. Crawl-sourced properties that landed via the new writer (PR 2 / #3218) but missed the legacy table — Setupad escalation #218's gatavo.com is the canonical example — now surface on the registry endpoints alongside legacy data. Functions changed: - hasValidAdagents: returns true if either publishers (with source_type='adagents_json') or discovered_publishers has a positive signal. Three-state contract preserved (null when neither has the domain; false only when legacy says false and catalog has no row). - getPropertiesForDomain (federated-index-db): UNION over discovered_properties and properties extracted from publishers.adagents_json JSONB, deduped on (publisher_domain, name, property_type) with legacy preferred so callers holding a discovered_properties.id keep working. Catalog-only rows surface property_rid as id (also a UUID; same call sites). - getDiscoveredPropertiesByDomain (property-db): same UNION shape. - getAllPropertiesForRegistry: three-way UNION (hosted + legacy crawl + catalog-only). Hosted wins over crawl (existing precedence carve-out preserved); legacy wins over catalog within the crawl side so the agent_count subquery against agent_property_authorizations remains meaningful during the dual-write window. - getPropertyRegistryStats: mirrors the registry's three-way UNION so a domain with both legacy and catalog rows is counted exactly once. Authorization-side readers (getPropertiesForAgent, findAgentsForPropertyIdentifier, getPublisherDomainsForAgent) stay on legacy tables — those need the authorization model decision still deferred from PR 1 and ship in PR 4b. PR 3 baseline coverage (registry-reader-baseline-* test files) all pass unchanged: 27 properties + 12 public-endpoints + 24 authorizations + 3 mcp = 66 cases. Sibling registry tests (registry-overlay-schema, registry-search, registry-feed, registry-pipeline-client, registry-crawler-cache) also pass — 66 more cases, no regressions. Refs #3177. Builds on #3195 (schema), #3218 (writer cutover), #3221 (reader baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(registry): harden property readers against malformed adagents.json Code- and security-review on PR #3244 converged on one Must Fix and a companion writer-side defense. publishers.adagents_json is publisher- controlled JSONB (the migration 432 comment is explicit about it), and the AdAgentsManager validator only type-checks `authorized_agents` — nothing forces `properties`, `tags`, or `identifiers` to be arrays. A publisher serving a JSON-valid manifest with `properties: "x"` (string) or any non-array value used to crash the readers via: jsonb_array_elements(p.adagents_json->'properties') -- 22023 error jsonb_array_length(p.adagents_json->'properties') -- 22023 error jsonb_array_elements_text(prop->'tags') -- 22023 error For per-domain readers (getPropertiesForDomain, getDiscoveredProperties ByDomain) that's a per-tenant 500. For the registry list/stats readers the catalog_only CTE selects across every publisher, so one poisoned manifest takes down the entire `properties://registry` MCP listing. Two layers of defense: 1. SQL-side jsonb_typeof guards. Every jsonb_array_elements / jsonb_array_length call in the four readers now wraps its argument with `CASE WHEN jsonb_typeof(...) = 'array' THEN ... ELSE '[]'::jsonb END` (or `0` for length). Identifiers and tags get the same guard so a property with `tags: "shopping"` or `identifiers: null` no longer throws. 2. Writer-side normalization. publisher-db.upsertAdagentsCache now normalizes `properties` and `authorized_agents` to arrays before stringifying into JSONB. Defends against future validator regressions and rows that landed before this PR. Both belt and suspenders so the readers are safe regardless of writer hygiene, and the writer doesn't rely on validator details that may loosen later. Also adds: - migration 433 — partial index on catalog_properties(created_by, property_id) WHERE created_by LIKE 'adagents_json:%'. The new readers do a per-property LATERAL join against catalog_properties; without this index the lookup is a sequential scan per row. - registry-reader-poisoned-manifest.test.ts — 6 cases that seed malformed bodies directly via SQL (bypassing the writer's normalization) and assert each reader returns []/0 instead of throwing. All 66 PR 3 baseline cases still pass. 14 crawler-cache + 16 overlay- schema cases also pass. 6 new poisoned-manifest cases pass — 96 cases total green. Refs #3177, #3244. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 26, 2026
Closed
8 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
PR 3 of the property registry unification (tracking issue #3177). Tests-only, no production code changes.
PR 1 (#3195) shipped the empty
publishers+adagents_authorization_overridesschema. PR 4 will swap the federated-index + property-registry readers to consult that schema. This PR pins the I/O of the current readers — same fixtures, same response shapes, identities, counts, ordering — so PR 4 will fail loudly if the cutover changes any caller-visible behavior.Coverage
Four new files under
server/tests/integration/, all integration tests against a real Postgres atprocess.env.DATABASE_URL(defaultpostgresql://adcp:localdev@localhost:5432/adcp_test).registry-reader-baseline-properties.test.ts—getPropertiesForDomain,getDiscoveredPropertiesByDomain,getPropertiesForAgent,getPublisherDomainsForAgent,findAgentsForPropertyIdentifier,hasValidAdagentsthree-state contract,getAllPropertiesForRegistrysource-projection + ORDER BY,getPropertyRegistryStats,FederatedIndexDatabase.getStatslower-bounds.registry-reader-baseline-authorizations.test.ts—getAgentsForDomain/getDomainsForAgentordering contract,getAgentAuthorizationsForDomain,bulkGetFirstAuthForAgents(incl.adagents_json > agent_claimsource preference),validateAgentForProductforselection_type∈ {all,by_id,by_tag} with source reporting (adagents_json/agent_claim/none), wildcard-agent ('*') literal handling.registry-reader-baseline-public-endpoints.test.ts— supertest againstGET /api/registry/agents(with and without?properties=true, asserting thebuying-only enrichment branch),/registry/publishers,/registry/publisher?domain=X,/registry/operator?domain=X,/registry/stats, plus a smoke test for/registry/feedenvelope shape.registry-reader-baseline-mcp.test.ts—lookup_domain(separatesadagents_jsonauthorized agents fromagent_claimsales claims) andlist_publishers(dedupes by domain across multiple discovering agents).list_authorized_propertiesis the upstream sales-agent tool name and intentionally out of scope.Assertion discipline
Behavior only — response shape, returned counts, returned identities, ordering contracts. No assertions on internal types or implementation details. PR 4 must be able to swap implementations and these tests still pass with identical I/O.
Parallelism
Each file uses a disjoint domain prefix (
prop-/auth-/endpoint-/mcp-) under*.registry-baseline.exampleso the four can run concurrently with each other and with siblingregistry-*tests without trampling state. Stats and registry-view assertions use lower-bounds where shared-DB residue would otherwise cause flakes.Test plan
mainbehavior, individuallyregistry-feed.test.ts,registry-search.test.ts,registry-overlay-schema.test.tsin the same vitest run (100 tests)🤖 Generated with Claude Code