feat(crawler): fan publisher_properties[].publisher_domains[] into per-child rows#4840
Conversation
…r-child rows The directory endpoint (#4838) returns one cafemedia.com row with zero counts for any agent listed in a publisher_properties selector, because the crawler only writes the manager-host edge to agent_publisher_authorizations. The 6,800 represented publishers stay invisible. This PR closes the gap. - Migration 486 widens publishers.discovery_method CHECK to include 'adagents_authoritative' (the spec value from #4828 for inline- resolution-discovered publishers). - DiscoveryMethod TS union widened to four values across adagents-manager and federated-index-db. - PublisherDatabase.recordChildPublisherFromManager upserts the child publishers row with source_type='community', discovery_method= 'adagents_authoritative', manager_domain=<host>, no blob. If the child already has its own adagents_json cached (was independently crawled), the upsert preserves the direct row — direct crawl wins over manager-file attribution. - CrawlerService.fanOutPublisherPropertiesAuthorizations walks each authorized_agents[] entry with authorization_type='publisher_properties', resolves the singular publisher_domain + compact publisher_domains[] forms, and writes per-child rows in both publishers and agent_publisher_authorizations. Called from both crawler loops so it fires regardless of which discovery path landed the manager file. Idempotent. - Integration tests cover: child row provenance, no self-attribution, direct-wins-over-attribution upsert behavior, domain canonicalization, singular vs compact selector forms, non-publisher_properties skip, and idempotency. After this lands, the directory endpoint returns the 6,800 expected rows for interchange.io against cafemedia.com.
There was a problem hiding this comment.
LGTM. Follow-ups noted below. The shape is right — the directory endpoint's job is discovery, and the schema at static/schemas/source/aao/agent-publishers.json:53-61 already names adagents_authoritative as the trust value that distinguishes "manager file said so" from a real origin fetch.
Things I checked
- Spec conformance.
ad-tech-protocol-expert: sound. The newdiscovery_methodvalue was specced in adcp#4823 / PR #4828 and the published schema enum atstatic/schemas/source/aao/agent-publishers.json:53-61already lists it; this PR makes it emittable. The inline-resolution rule (adcp#4825 / PR #4827) is endorsed atdocs/governance/property/adagents.mdx:496-522, and themanagerdomainfallback safety rule (lines 242-249) gives the property'spublisher_domainfield the same trust-anchor role this fan-out relies on. - Cross-publisher attribution split.
projectAuthorizationToCatalogatserver/src/db/publisher-db.ts:953-967still refuses cross-publisher claims into the strict catalog. The new fan-out writes only to legacyagent_publisher_authorizationsand stampsdiscovery_method='adagents_authoritative'. Both arms surface viafederated-index-db.ts:265-356, but with distinct provenance — catalog-as-authorization, legacy-as-discovery. Right shape. - Self-attribution. Belt-and-braces:
crawler.ts:1042-1045drops the manager from the child set after canonicalization, andpublisher-db.ts:248checks again at the writer. Both sides canonicalize, so case/whitespace/trailing-dot variants can't escape. - Idempotency. The dedicated test at
server/tests/integration/crawler-publisher-properties-fanout.test.ts:264-284runs the fan-out 3× and asserts 2 rows, not 6. ON CONFLICT semantics on the existing unique constraints hold. - Changeset.
.changeset/4839-crawler-publisher-properties-fanout.mdempty-frontmatter is consistent with the rest of the AAO series (.changeset/4823-aao-directory-inverse-lookup-endpoint.md,.changeset/4836-aao-directory-inverse-lookup-server.md). - Test-plan items. The three unchecked boxes are explicitly Reviewer tasks, not Author. Author shipped what they signed up for.
Follow-ups (non-blocking — file as issues)
-
Stale fan-out rows are never reconciled.
reconcileLegacyAdagentsAgentsatfederated-index-db.ts:593-625deletes rows keyed onpublisher_domain = <manager>. The fan-out writes rows keyed onpublisher_domain = <child>. If a manager drops a child frompublisher_domains[]on a later crawl, the(agent_url, child, source='adagents_json')row persists. The endpoint surfaces it indefinitely. Needs a fan-out-aware reconciler: compute the new desired child set per (manager, agent),DELETE FROM agent_publisher_authorizations WHERE agent_url=? AND source='adagents_json' AND publisher_domain NOT IN (desired) AND publisher_domain IN (SELECT domain FROM publishers WHERE manager_domain=? AND discovery_method='adagents_authoritative'). Same logic on thepublishersrow retirement. -
First-writer-wins through transient child-origin failures.
recordChildPublisherFromManageratpublisher-db.ts:255-267keys "stronger row" offadagents_json IS NULL.recordFailedAdagentsFetchatpublisher-db.ts:285-303writes the row without settingadagents_json(correct — there's no body). If attacker writes first and the child's origin is flapping 5xx for the directory's crawl window, attacker'smanager_domainclaim is durable across the failures. Mitigation: tracklast_direct_success_atseparately and refuse downgrade when it's set. -
Migration 486 takes an
ACCESS EXCLUSIVElock for a full-table scan whose outcome is guaranteed.486_publisher_discovery_method_adagents_authoritative.sql:22-29isDROP CONSTRAINT+ADD CONSTRAINT. The new value set is a strict superset of the old one — every existing row trivially satisfies the new constraint, so the validating scan is pure dead weight. Precedent in this repo (migration 473) usesADD CONSTRAINT ... NOT VALID; VALIDATE CONSTRAINT;— second statement takes onlySHARE UPDATE EXCLUSIVE. Worth backporting before this hits the next populated-table case. -
property_tagssilently dropped during fan-out.crawler.ts:1051-1056callsrecordAgentFromAdagentsJsonwithproperty_ids: undefinedand has no equivalent argument forselection_type='by_tag'selectors'property_tags. For cafemedia this is operationally a no-op (the tag is the whole network), but a manager who uses tags to scope (['video-only']) gets over-reported authorization in the federated index. Document the limitation, or extend the index schema. -
canonicalizePublisherDomainis canonicalization, not validation. It accepts IP literals, ports, and path-bearing strings (nyt.com/admin). The schema pattern atstatic/schemas/source/adagents.jsonis the spec; enforce it at the fan-out writer boundary so a malformed manager file can't pollutepublishers.domainwith rows that won't ever match a real crawl. -
inline_propertiesnot handled. Same inverse-lookup concern, same shape —inline_properties[*].publisher_domainidentifies per-child publishers the same way. Either deliberate scope or an oversight; the PR title is specific topublisher_properties, so this is probably a separate ticket.
Minor nits (non-blocking)
-
Three docblocks stacked above one method.
publisher-db.ts:214-241now has the original (already-orphaned) "Cache an adagents.json manifest…" + "Record a failed adagents.json fetch…" + the newrecordChildPublisherFromManagerblock, all in front ofrecordChildPublisherFromManager.recordFailedAdagentsFetchis now bare. The orphan situation predated this PR but the PR makes it worse. Move the new method belowrecordFailedAdagentsFetch, or relocate the two orphans to their actual targets while you're in the file. -
Same orphan pattern at
crawler.ts:984-1012. The new method's docblock is inserted betweenrecordPropertiesForAgent's docblock at L984-986 and its body at L1069. Notable that the comment-relocation game now has scoreline 2-0 in this PR's favor. -
Test seam reaches into a private method via prototype binding +
anycast (crawler-publisher-properties-fanout.test.ts:127-150). Signal that the helper has no actual dependency onCrawlerServicestate — only onpublisherDbandfederatedIndex. Extracting it to a free function would let the test import directly. Optional. -
Per-child catch logs but has no counter (
crawler.ts:1057-1065). "Partial progress beats silent total failure" is the right call, but a metric (crawler_fanout_per_child_failures_total{manager_domain}) lets operators alert on rows that get swallowed. -
source='adagents_json'on synthesized rows is technically accurate (the manager file IS the adagents.json source per the inline-resolution rule) but a consumer reading the column as "origin-attested" will be wrong. Worth a single line infederated-index-db.tsdocumenting that the value covers inline-resolution-synthesized rows.
Safe to merge.
… enforcement, by_id guard, transactional migration Findings from code-reviewer-deep + ad-tech-protocol-expert on PR #4840: - **Manager-side revocation gap** (critical, protocol-expert). The directory endpoint's is_revoked CTE walked the CHILD's adagents_json blob for revoked_publisher_domains[]. Fan-out children have adagents_json IS NULL, so manager-side revocation of a managed-network child was silently ignored. Adds a LEFT JOIN to the manager's publishers row and ORs the child-side check with the manager-side check. Spec rule "Revocation under inline resolution" is now enforced for the cafemedia case. - **by_id + publisher_domains[] guard** (important, protocol-expert). Schema rejects this combo (property IDs are publisher-scoped — fanning a fixed ID set across N publishers silently cross-authorizes whichever inventory shares an ID). Hand-rolled validator doesn't enforce it; the fan-out now refuses defensively. Singular publisher_domain on by_id stays honored (schema-conformant). - **XOR enforcement on selectors** (important, code-reviewer). Schema requires exactly one of publisher_domain / publisher_domains[]. Both- populated or neither-populated selectors are now skipped in the fan-out helper, mirroring the catalog projection's refuse-both invariant. - **Migration 486 transaction** (important, code-reviewer). DROP and ADD CONSTRAINT now wrapped in BEGIN/COMMIT so they share one lock window. Closes the writer race in the brief gap between the two ALTERs. - **Migration 486 doc comment** (nit, code-reviewer). The "backfilled to 'direct' for previously-validated rows" sentence was migration 470's job, not 486's. Replaced with the trust profile description. Tests added: - manager-side revocation via the directory endpoint (the SQL fix) - by_id + publisher_domains[] refusal (no fan-out for malformed selector) - by_id + singular publisher_domain (schema-conformant, fan-out works) - XOR refusal: both publisher_domain and publisher_domains[] populated - XOR refusal: neither populated Deferred to follow-up (code-reviewer noted, agreed): - Bulk INSERT … VALUES batching for 6,800-child fan-outs (perf, not correctness) - Pure-function extraction of fanOutPublisherPropertiesAuthorizations (test ergonomics; current prototype-binding works) - upsertAdagentsCache COALESCE-defense for null discoveryMethod (defensive vs intentional-overwrite trade-off; current call sites are correct)
|
Issue #4841 proposes projecting fan-out rows into Generated by Claude Code |
There was a problem hiding this comment.
Two real correctness regressions on top of the spec-faithful fan-out. Both are fixable in this PR or as a fast-follow — but I want answers before approve, because the public, unauthenticated directory endpoint at server/src/routes/registry-api.ts:6924 is the consumer of every row this PR writes.
Things I checked
- Migration 486 wraps DROP+ADD CHECK in BEGIN/COMMIT. Constraint is expansion-only so no existing row can violate; validation pass holds ACCESS EXCLUSIVE for one sequential scan. Acceptable at current
publishersscale; would preferNOT VALID+ asyncVALIDATEnext time this constraint shape changes. - XOR + by_id guards at
server/src/crawler.ts:1029-1054are faithful to the schema. Walked all eight test cases: singular-only onby_id(crawler.ts:1037) fans one row,by_id + publisher_domains[]skips at L1050,by_tag + publisher_domains[]fans the plural set, both-populated + neither-populated skip at L1033-1034. Tests atcrawler-publisher-properties-fanout.test.ts:551-721exercise every branch.ad-tech-protocol-expertconfirmedstatic/schemas/source/core/publisher-property-selector.json:38-44,47-76actually enforces the XOR +by_idrule the helper echoes — not over-stated. 'adagents_authoritative'is already on the wire.static/schemas/source/aao/agent-publishers.json:58-60carries the enum on the directory response;docs/aao/directory-api.mdx:92,106documents Medium trust. This PR widens the TS union and the CHECK constraint to let the crawler actually emit a value the response schema already accepts. Not a unilateral protocol extension.- Revocation OR EXISTS at
server/src/db/federated-index-db.ts:322-340correctly implements the normative rule atdocs/governance/property/adagents.mdx:520("Inline resolution MUST honorrevoked_publisher_domains[]on the parent file"). LEFT JOIN tomgris safe whenpub.manager_domain IS NULL— COALESCE returns[], EXISTS returns false, OR short-circuits. Query plan at 6,800 children is 6,800 PK lookups against one manager row. Confirmed safe. recordChildPublisherFromManagerdirect-wins upsert atserver/src/db/publisher-db.ts:251-263preserves the stronger row whenadagents_json IS NOT NULL. Test atcrawler-publisher-properties-fanout.test.ts:469-497covers it. Correct for thediscovery_method/source_typeaxis.
Open questions — answer before I flip to approve
-
manager_domainlast-writer-wins on fan-out children (server/src/db/publisher-db.ts:260-263). For a child whose row already exists withadagents_json IS NULL(i.e., it was previously minted by manager A), the upsert'sCASE WHEN publishers.adagents_json IS NULL THEN EXCLUDED.manager_domainwill overwritemanager_domainto whoever wrote last. After this PR the revocation join atfederated-index-db.ts:340readsrevoked_publisher_domains[]from whichever manager won the race. Net effect: any subsequent manager file claiming the same child seizes revocation authority over that child's edges from the original manager. What's the AAO gate that prevents arbitrary domains from registering manager files? If there isn't one — or if registration is open by design and the trust label is the only signal — this needsWHERE publishers.manager_domain IS NULL(orOR = EXCLUDED.manager_domain) on the manager_domain branch of the CASE before the public directory consumes these rows. -
discovered_agents.source_domainthrashing (server/src/federated-index.ts:248-252called fromserver/src/crawler.ts:1070).upsertAgentkeys onagent_urlalone (server/src/db/federated-index-db.ts:653) andEXCLUDED.source_domainoverwrites on conflict. For cafemedia's 6,800-child fan-out, the sameagent_urlgetssource_domainrewritten 6,800 times in a single crawl pass — final value is the alphabetically-last child, not the manager that actually authorized it. The host-level call atcrawler.ts:461/546already established the agent row withsource_domain=cafemedia.com; the fan-out'srecordAgentFromAdagentsJsonneed only callupsertAuthorization, notupsertAgent. Two-line fix: splitrecordAgentFromAdagentsJsonso the agent-side upsert is opt-out, or inline-callupsertAuthorizationdirectly from the fan-out loop.
Inverse of (1) — caught by ad-tech-protocol-expert, file as follow-up
The same CASE preserves manager_domain (as NULL) on a direct crawled child that is later legitimately inlined by a manager. The manager-side revocation join then can't reach the manager's revoked_publisher_domains[] for that child — spec rule at docs/governance/property/adagents.mdx:520 is unenforceable. manager_domain is information-additive, not provenance-competing; the "direct wins" rule should apply to discovery_method / source_type but not to manager_domain. Net: a single-column manager_domain is the wrong shape; a manager_domains TEXT[] (or a separate publisher_manager_claims table) is the right shape long-term.
Follow-ups (non-blocking — file as issues)
- No length cap on
publishers.domain—server/src/db/migrations/432_publishers_overlay.sql:27declaresdomain TEXT PRIMARY KEYwith no CHECK, whilemigrations/207_registry_requests.sql:6hasCHECK (length(domain) <= 253 AND domain = lower(domain)). A malicious manager file shipping 100KB-per-domain payloads can amplify into a PRIMARY KEY index. Belt-and-suspenders. - Empty-canonical defense —
canonicalizePublisherDomain("....")returns"", whichfanOutPublisherPropertiesAuthorizationswill happily insert intopublishers.domain. Validator already defends invalidator.ts:373-377; the writer should too. Addif (childCanonical === '') continue;aftercrawler.ts:1063. - Aggregate failure counter on fan-out —
crawler.ts:1076-1084logs one warn per failed child. At 6,800-domain scale a poisoned manager file produces a flood indistinguishable from one Postgres retry. Tally per call, log one structured{manager, agentUrl, attempted, succeeded, failed}with a metric so an alarm can fire on rate spikes. - Reconcile
manager_domainon manager-side drops — when a child drops out of a manager'spublisher_properties[].publisher_domains[]across crawls, the syntheticpublishers.manager_domainis never NULLed. Stale manager attribution lingers. - Test seam —
crawler-publisher-properties-fanout.test.ts:154-167reaches into a private method via(CrawlerService as any).prototypewith a hand-rolledctx. Tests SQL behavior cleanly, but locks the test to the helper's private API; a follow-up that exercises the full crawler loop throughvalidateDomainwould be more durable.
Minor nits (non-blocking)
- Changeset out-of-scope claim contradicts what shipped.
.changeset/4839-crawler-publisher-properties-fanout.mdlists "Revocation propagation through fan-out … separate concern" as out-of-scope whilefederated-index-db.ts:322-340implements exactly that. Reconcile the changeset language before release notes auto-generate. - Stale doc comment above
fanOutPublisherPropertiesAuthorizationsatcrawler.ts:983-986— therecordPropertiesForAgentblock doc landed above the new method's doc. insertPublisherhelper inregistry-agent-publishers-detail.test.ts:71-83still typesdiscovery_methodas the 3-value union; the new test at L198-237 bypasses it with directpool.query. Widen the helper for consistency.- Filename
.changeset/4839-*.mdfor PR#4840— a notable choice of off-by-one.
Holding at comment. Flip to approve once (1) the manager_domain overwrite is gated against last-writer-wins (or you confirm the AAO registration model makes that irrelevant) and (2) the discovered_agents.source_domain thrash is bounded.
…+ catalog variant (#4851) #4840 wired the fan-out helper into the two periodic-crawl loops but missed the on-demand crawlSingleDomain path (called by POST /api/registry/crawl-request) and crawlSingleDomainForCatalog (called by the manager revalidation queue worker). Both iterate authorized_agents[] independently and need the same fan-out call. Symptom: admin-triggered crawl of cafemedia.com refreshed the manifest cache (last_verified_at updated) but didn't synthesize the 6,800 child rows — the directory inverse-lookup kept returning a single cafemedia.com row with properties_total=0. Two lines added: fanOutPublisherPropertiesAuthorizations call at the end of each per-agent loop, mirroring the periodic paths.
…ishers (#4850) (#4873) recordChildPublisherFromManager (PR #4840) created publishers rows for each child synthesized from a manager file's publisher_properties[] selector but never scheduled them for refresh. A manager-asserted child stamped last_validated=NOW() at fan-out time stayed at that timestamp indefinitely. Fix: also enqueue the child into manager_revalidation_queue (existing infra from migration 471) with next_attempt_after = NOW + 24h. The existing worker drains at 50/5min, so 6,800 cafemedia children spread across ~12h after first becoming due. ON CONFLICT DO NOTHING preserves any existing backoff state — re-enqueue from a later fan-out doesn't reset a child that 404'd recently. What the worker does per child: crawlSingleDomain(child) runs the full discovery cascade — tries the child's own /.well-known first, falls back to ads.txt MANAGERDOMAIN. Children with their own file get promoted to discovery_method='direct' (gives the divergence-detector use case real data). 404s record failed-fetch and the row deletes; re-enqueue happens on next cafemedia rotation. Tests pin: 24h initial delay; idempotent re-enqueue preserves backoff; self-attribute case skips both publisher and queue writes. Resolves #4850.
…t_authorizations (#4841) (#4879) Closes the gap surfaced after #4840 landed fan-out writes in the legacy agent_publisher_authorizations arm. The catalog projection in publisher-db.ts:upsertAdagentsCache deliberately refuses cross- publisher claims (L968: "publisher_properties claims a different publisher — cross-publisher refused"). Result: ~6,800 cafemedia child authorization edges existed in the legacy arm but never landed in catalog_agent_authorizations. The auth-gated partner-sync endpoints (/registry/authorizations, /registry/authorizations/snapshot) read the catalog only and missed all of them. Design: the existing cross-publisher guard is load-bearing for catalog trust — a hostile manager file can't project authoritative rows for publishers that didn't authorize anything. The guard stays. The fan- out gets its OWN evidence value so the projection is explicit and consumers can filter. - Migration 488: widen catalog_agent_authorizations.evidence CHECK to add 'adagents_authoritative'. Backfill insert from publishers + agent_publisher_authorizations where discovery_method='adagents_authoritative'. - New PublisherDatabase.recordCatalogFanoutAuthorization writes the edge with evidence='adagents_authoritative', created_by='system', no property_rid (publisher-wide). - crawler.ts fan-out helper now also calls the new writer per (agent, child) pair alongside the existing publishers + legacy authz writes. - 6 evidence-mapping CASE statements in federated-index-db.ts add WHEN 'adagents_authoritative' THEN 'adagents_json' so the dual-read path round-trips correctly. Trust profile: 'adagents_authoritative' is lower than 'adagents_json' (manager-asserted, no bilateral confirmation). Per #4825 inline- resolution rule, the manager file naming the publisher in publisher_properties[].publisher_domains[] is the out-of-band corroboration that makes the projection safe. Tests pin: catalog row shape, idempotent re-write, coexistence with adagents_json for same (agent, publisher), invalid agent_url silent-skip, canonicalization, view round-trip. Resolves #4841.
Summary
Write-path counterpart to #4838 (directory inverse-lookup read endpoint) and #4827 (inline-resolution spec rule). The directory endpoint currently returns one cafemedia.com row with
properties_authorized=0 / properties_total=0for any agent listed in apublisher_propertiesselector — the 6,800 represented publishers are invisible. This PR makes the crawler fan the selector'spublisher_domains[]into per-child rows.Files
server/src/db/migrations/486_publisher_discovery_method_adagents_authoritative.sql— adds'adagents_authoritative'to thepublishers.discovery_methodCHECK constraint. Spec value from feat(aao): define directory inverse-lookup endpoint spec (#4823) #4828 for inline-resolution-discovered publishers; the crawler can now emit it.server/src/adagents-manager.ts+server/src/db/federated-index-db.ts— DiscoveryMethod TS union widened.server/src/db/publisher-db.ts— newrecordChildPublisherFromManagermethod. Upserts the child publishers row withsource_type='community',discovery_method='adagents_authoritative',manager_domain=<host>, NO blob. Preserves stronger rows (a child that was independently crawled withdiscovery_method='direct'is NOT downgraded).server/src/crawler.ts— newfanOutPublisherPropertiesAuthorizationshelper, called from both crawler loops (per-discovered-publisher + per-agent-claimed-publisher). Walks eachauthorized_agents[]entry whoseauthorization_typeispublisher_properties, resolves singularpublisher_domain+ compactpublisher_domains[]forms, and writes per-child rows in both tables. Per-child failures are logged but don't abort — partial progress beats silent total failure at managed-network scale.server/tests/integration/crawler-publisher-properties-fanout.test.ts— 8 tests covering DB invariants (child row provenance, no self-attribution, direct-wins-over-attribution upsert, canonicalization) and the crawler helper (singular vs compact selector forms, non-publisher_properties skip, manager-in-publisher_domains drop, idempotency).What it produces on cafemedia.com
agent_publisher_authorizations(interchange.io → cafemedia.com)publishersdiscovery_method='adagents_authoritative'+manager_domain='cafemedia.com'0/0countsProperties were already correctly scoped per-child by the existing
recordPropertiesForAgentflow (the property's ownpublisher_domainfield wins atcrawler.ts:1002) — so once the child publishers + authz edges exist,properties_totalandproperties_authorizedresolve correctly without further changes.Trust profile of
adagents_authoritativeMedium — the manager file names each represented publisher on each property's
publisher_domainfield (the same anchor themanagerdomainfallback safety rule requires), but no bilateral confirmation from the publisher's own origin. Distinct fromdirect(origin-fetched),authoritative_location(publisher actively delegated), andads_txt_managerdomain(delegation discovered via ads.txt fallback).Idempotency
All writes use ON CONFLICT semantics on the existing unique constraints. Three consecutive crawls of the same manager file produce exactly 6,800 child rows, not 18,400.
Out of scope
revoked_publisher_domains[]already short-circuits the directory's status emission via the existing JSONB walk on the host row.Resolves the deferred follow-up flagged in #4838's PR description.
Test plan
npm run typecheck)publisherstable at production scaleGET /api/v1/agents/https%3A%2F%2Finterchange.io/publishers— expect 6,800 rows🤖 Generated with Claude Code