fix(crawler): skip fan-out when crawl source delegates via ads.txt MANAGERDOMAIN - #4864
Merged
Merged
Conversation
…NAGERDOMAIN When the crawler hits a publisher that delegates via ads.txt MANAGERDOMAIN, the manifest comes from the manager domain — but the source publisher being processed is the delegating child. Firing fan-out from those calls used the CHILD as the manager_domain attribution for all 6,800 siblings. Observed: after cafemedia crawl, 07.gg.manager_domain was '2foodtrippers.com' instead of 'cafemedia.com'. A 2foodtrippers.com crawl had fired fan-out after the cafemedia crawl and overwrote the manager attribution. The publishers row UPSERT preserves direct rows but happily overwrites adagents_authoritative rows with whichever crawl ran last. Prospective fix: gate all four fan-out call sites on validation.discovery_method !== 'ads_txt_managerdomain'. Delegating- child crawls skip fan-out; the manager's own crawl handles it. Retrospective fix (migration 487): recursive CTE walks child.manager_domain → ... → root, fixes every adagents_authoritative row whose manager_domain points to another adagents_authoritative row, pointing it at the first non-adagents_authoritative ancestor (the actual manager). Bounded depth 10.
Contributor
There was a problem hiding this comment.
LGTM. Real bug, minimum surgical fix, plus a backfill that resolves the existing damage. The gate predicate matches the failure mode: when the manifest came from the manager (via ads.txt MANAGERDOMAIN), the source publisher being processed is the wrong attribution anchor for fan-out.
Things I checked
- Gate predicate correct at all four sites (
server/src/crawler.ts:488, 572, 1361, 1642).validationis theAdAgentsValidationResultfromvalidateDomain()in every scope; the literal'ads_txt_managerdomain'matches the enum atserver/src/adagents-manager.ts:18. - Correct to NOT also gate on
'authoritative_location'. That's the publisher's own stub redirecting to a canonical URL — the source publisher is still the authoritative entity for fan-out attribution. - Coverage complete:
fanOutPublisherPropertiesAuthorizationshas exactly four call sites incrawler.ts, andrecordChildPublisherFromManageris only called fromfanOutPublisherPropertiesAuthorizations. No other write path can stampmanager_domainon anadagents_authoritativerow. - Migration 487 seed (
p1andp2bothadagents_authoritative) is the right base case — precisely the bug shape. - Recursive step walks one hop past the chain (the row with non-
adagents_authoritativeprovenance), andresolvedpicks it viaWHERE hop_discovery_method != 'adagents_authoritative'+DISTINCT ON (child_domain) ORDER BY child_domain, depth DESC. Correct. - Cycle defense (
depth < 10) is sufficient: a pure-adagents_authoritativecycle never satisfies theresolvedpredicate, so the child is skipped rather than mis-updated. Fail-closed beats fail-open. - Idempotency: re-runs produce no new seeds (fixed rows' parent is now
direct, failing the seed predicate) and themanager_domain != root_manager_domainguard short-circuits the rest.
Follow-ups (non-blocking — file as issues)
- No unit test for the gate. Table-driven test asserting
fanOutPublisherPropertiesAuthorizationsis not called whenvalidation.discovery_method === 'ads_txt_managerdomain', exercised at each of the four call sites, would lock the regression. Test plan has it post-deploy, which is fine for the data fix but doesn't lock the code path. - Manager-not-crawled edge case. If a manager chain dead-ends at a row with
discovery_method='adagents_authoritative'(no non-adagents_authoritativeancestor reachable in ≤10 hops), the migration leaves descendants unfixed and the live code never fans them out — because fan-out only fires from the manager's own crawl. Cafemedia isdirectso the Raptive set is covered. Worth either enqueueing manager revalidation for orphan chain heads or a metric that surfaces them post-migration.
Minor nits (non-blocking)
- Nested
BEGIN;/COMMIT;in the migration.migrate.tsalready wraps the body in a transaction. The innerBEGINis a no-op warning, but the innerCOMMITcloses the outer transaction — the runner'sINSERT INTO schema_migrationsthen runs outside any transaction. Net effect: UPDATE and the migrations row are not atomic. Pre-existing pattern (migration 486 same shape), and themanager_domain != root_manager_domainidempotency guard makes re-runs safe — so this is a Minor here, but the repo-wide pattern is worth a cleanup pass at some point.
Safe to merge.
4 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
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
Empirical bug found in production after #4851 deployed. The fan-out is wired at all four call sites, but the source publisher being crawled isn't always the manifest's host. When a crawl hits a publisher that delegates via
ads.txt MANAGERDOMAIN=, the file comes from the manager — but the crawl source is the delegating child. Firing fan-out from those calls used the child as themanager_domainfor all 6,800 siblings.Observed in production (the smoking gun)
After triggering a cafemedia.com crawl-request:
07.gg's manager should be
cafemedia.com. It's2foodtrippers.combecause a 2foodtrippers.com crawl (viamanagerdomain=cafemedia.comin 2foodtrippers' ads.txt) fired fan-out after the cafemedia crawl and overwrote sibling attribution. ThepublishersUPSERT preserves direct rows but happily overwritesadagents_authoritativerows.Prospective fix (crawler.ts)
Gate all four fan-out call sites on
validation.discovery_method !== 'ads_txt_managerdomain'. When the crawl source is a delegating child, skip fan-out; the manager's own crawl handles it with correct attribution.The four sites (per #4851):
crawlSingleDomain(admin crawl-request)crawlSingleDomainForCatalog(manager revalidation worker)Retrospective fix (migration 487)
Recursive CTE walks
child.manager_domain → grandparent → rootand fixes everyadagents_authoritativerow whosemanager_domainpoints to anotheradagents_authoritativerow, pointing it at the first non-adagents_authoritativeancestor (the actual root manager —cafemedia.comfor the Raptive set,discovery_method='direct'). Bounded recursion depth 10 to defend against cycles.Out of scope
Test plan
07.gg) and confirmmanager_domain = 'cafemedia.com'againdiscovery_methods: ["adagents_authoritative"]for the 6,800 Raptive children, all withmanager_domain: "cafemedia.com"🤖 Generated with Claude Code