Skip to content

fix(registry): flip type==='buying' filter inversions to 'sales' + add sales to by_type tally#3540

Merged
EmmaLouise2018 merged 5 commits into
mainfrom
EmmaLouise2018/fix-sales-type-inversions
Apr 30, 2026
Merged

fix(registry): flip type==='buying' filter inversions to 'sales' + add sales to by_type tally#3540
EmmaLouise2018 merged 5 commits into
mainfrom
EmmaLouise2018/fix-sales-type-inversions

Conversation

@EmmaLouise2018
Copy link
Copy Markdown
Contributor

@EmmaLouise2018 EmmaLouise2018 commented Apr 29, 2026

Refs #3538, #3495.

#3495 fixed agent-type inference (sales tools → 'sales', not 'buying'). The sweep missed 6 call sites that filtered on agent.type === 'buying' for logic that semantically operates on sales agents (the ones that hold publisher authorizations and call list_authorized_properties). Pre-fix, both classification and filter were inverted — they accidentally aligned. Post-fix, the filters silently match zero real sales agents and entire code paths went dead.

Sites flipped

File:line What it does Effect post-#3496
crawler.ts:419 reverse-crawl publishers from sales agents' list_authorized_properties dead — no sales agent matches 'buying'
health.ts:131 fetchStats populates property_count, publishers, publisher_count sales agents returned empty stats
publishers.ts:204 publisher-domain map in trackPublishers publisher tracking missed sales agents
http.ts:8784 agent cache warmer calling propertiesService.getPropertiesForAgent property cache cold for sales agents
registry-api.ts:3667 withProperties enrichment dispatch publisher_domains/property_summary empty in /api/registry/agents
registry-api.ts:3677 withProperties enrichment result mapping same path downstream

The receiving function at federated-index.ts:281 already documents the source as "a sales agent's list_authorized_properties" — the caller was the bug. Comment + log message at crawler.ts:416-417 updated to match.

Stats tally fix

/api/stats and /agents JSON by_type returned {creative, signals, buying} but never counted sales. Added a sales line to both at http.ts:2021 and :2087.

Behavior change: path 2 reverse-crawl revives

crawler.ts:417-465 is a previously-dead reverse-crawl path. With this PR merged, every sales agent's list_authorized_properties response will be walked, and each publisher_domain it surfaces will be crawled (adAgentsManager.validateDomain per domain).

Rate-limit shape — verified, not a fanout risk on first pass:

  • The loop is sequential for (const agent of agents) containing a sequential for (const domain of auth.publisher_domains). Each validateDomain call awaits before the next domain starts. No Promise.all fanout.
  • processedDomains (crawler.ts:437) deduplicates within a crawl pass — a publisher domain referenced by multiple sales agents only crawls once.
  • Errors are caught per-domain (try { ... } catch (err) { log.error(...) }), so a single bad domain doesn't stall the loop.
  • Today's fleet: 2 sales agents (Bidcliq, Swivel). Bounded.

Registry growth expectation:

  • Once revived, expect /api/registry/agents and /api/registry/publishers to gain rows from any publisher domain that Bidcliq/Swivel claim authorization on. These will appear as source: 'discovered' (not registered). Pre-emptively flag for whoever monitors registry growth metrics — first crawl pass after merge is the moment to watch.
  • Subsequent sales agents (when they register) will trigger their own first-pass crawl. If the fleet grows substantially, the sequential loop is still bounded but a parallelism cap + per-domain rate-limit becomes worth designing.

Out-of-scope guardrails (file separately if growth surprises us):

  • Per-domain crawl cooldown (e.g., don't re-validate the same domain within 1h across passes).
  • Concurrency cap on the publisher-domain inner loop.

Backwards compatibility

Non-breaking on the wire. /api/stats and /agents by_type responses gain a new sales: <count> field — additive. The other five fixes restore previously-broken behavior; consumers that depended on the broken behavior (zero property_count, empty publisher_domains) will start seeing populated data, which is the intended state.

Test plan

  • New: server/tests/unit/health-stats-type-filter.test.ts pins the type filter for sales/buying/empty cases — 3/3 pass.
  • npx vitest run server/tests/unit/ — 2623/2623 pass.
  • npx tsc --noEmit -p server/tsconfig.json — clean.
  • Pre-commit hooks green (test:unit, test:test-dynamic-imports, typecheck).
  • Server-integration job is red on a pre-existing stripe-client.js mock issue (listCustomersWithOrgIds not exported in the vi.mock blocks at content-my-content.test.ts:137 and admin-endpoints.test.ts:62). Stack does not touch any file in this PR. Tracked as fix(test): add listCustomersWithOrgIds to stripe-client mock — server integration tests fail at HTTPServer.start #3548. Confirm fix lands before merging this PR so CI is green.

Stack ordering

Independent of #3541 (different lines in crawler.ts) and #3542 (different lines in registry-api.ts). Recommended merge order from #3538: 3540 → 3542 → 3543 → 3541.

Out of scope

Tracked as separate issues from #3538:

…d sales to by_type tally

Refs #3538, #3495.

#3495 fixed agent-type inference (sales tools -> 'sales', not 'buying'). The
sweep missed 6 call sites that filtered on `agent.type === 'buying'` for logic
that semantically operates on sales agents. Pre-fix, both classification and
filter were inverted — they accidentally aligned. Post-fix, the filters
silently match zero real sales agents and entire code paths went dead.

## Sites flipped

- `crawler.ts:419` — reverse-crawl publishers from sales agents'
  list_authorized_properties. Receiving function at `federated-index.ts:281`
  already documents the source as a sales agent. Caller was the bug.
- `health.ts:131` — fetchStats populates property_count, publishers,
  publisher_count from PropertyIndex. Sales agents are the ones that have
  authorizations; filter excluded them.
- `publishers.ts:204` — publisher-domain map in trackPublishers.
- `http.ts:8784` — agent cache warmer calling getPropertiesForAgent.
- `registry-api.ts:3667` and `:3677` — withProperties enrichment in
  /api/registry/agents (promise dispatch + result mapping).

Comment + log message at `crawler.ts:416-417` updated to match.

## Stats tally fix

/api/stats and /agents JSON by_type returned {creative, signals, buying} but
never counted sales. Added a sales line to both at `http.ts:2021` and `:2087`.

## Test plan

- New: `server/tests/unit/health-stats-type-filter.test.ts` pins fetchStats
  type filter for sales/buying/empty cases. 3/3 pass.
- `npx vitest run server/tests/unit/` — 2623/2623 pass.
- `npx tsc --noEmit -p server/tsconfig.json` — clean.
EmmaLouise2018 added a commit that referenced this pull request Apr 30, 2026
…egration tests (#3553)

Closes #3548.

Two integration tests vi.mock src/billing/stripe-client.js but the mock
factories did not export listCustomersWithOrgIds. Server startup calls
OrganizationDatabase.syncStripeCustomers which uses that method, threw at
HTTPServer.start, broke every integration test that booted through these
mocks.

Fix: switch the mock factories to vitest's importOriginal pattern so partial
mocks inherit real exports. listCustomersWithOrgIds (and any future
stripe-client method) flow through automatically.

Files:
- server/tests/integration/content-my-content.test.ts
- server/tests/integration/admin-endpoints.test.ts

This unblocks the integration-tests CI job on #3540 and any future PR that
adds tests booting the server through these mocks.
…d polarity

The test at registry-reader-baseline-public-endpoints.test.ts:450 was
written under the pre-#3495 inverted behavior — it seeded a 'buying'
agent and asserted enrichment, then asserted no enrichment for the
'sales' AGENT_X. Both expectations are exactly backwards now that this
PR flips the readers to filter on type='sales' (the corrected polarity).

Swap the assertions so the buying agent is the negative branch and
AGENT_X (sales, already seeded with property auth on PUB_A's home
property earlier in the suite) is the positive branch. Test name
renamed to "enriches sales agents only" to match.

Surfaced by failing CI on this PR; the test was the last hold-out
pinning the inverted behavior the rest of #3540 removes.
@EmmaLouise2018 EmmaLouise2018 merged commit da42453 into main Apr 30, 2026
13 checks passed
@EmmaLouise2018 EmmaLouise2018 deleted the EmmaLouise2018/fix-sales-type-inversions branch April 30, 2026 02:53
EmmaLouise2018 added a commit that referenced this pull request May 1, 2026
#3776)

Closes #3774. Surfaced by Brian-style red-team of #3766. Same inversion
class as #3540 swept across filter sites, but 5 sites slipped through
because they had a different shape — function-call arguments and local-
inference re-implementations rather than equality comparisons against
stored type.

## listAgents("buying") -> listAgents("sales") at 3 call sites

- server/src/http.ts:2007 — POST /api/crawler/run
- server/src/http.ts:8729 — periodic property crawler boot (variable
  also renamed buyingAgents -> salesAgents and the comment fixed)
- server/src/mcp-tools.ts:831 — find_agents_for_property MCP tool

These pass the agent set into property-crawl logic that semantically
operates on sales agents (the ones with publisher authorizations and
list_authorized_properties responses). Pre-fix the filter was a no-op
because the crawler internally re-filters on agent.type !== "sales" at
crawler.ts:420 — but the call sites were misleading and load-bearing
on the defensive re-filter.

## Diagnostic-endpoint local inference consolidated + polarity flipped

server/src/http.ts:8442 and server/src/routes/registry-api.ts:5802 had
duplicated inline inference that returned 'buying' for agents exposing
get_products / create_media_buy / create_media — exactly inverted vs the
canonical CapabilityDiscovery.inferAgentType. Visible bug for any consumer
of the public discovery diagnostic endpoints: every sales agent reported
as type='buying'.

Extracted into a single helper:

  server/src/lib/diagnostic-agent-type-inference.ts

Both endpoints now call inferDiagnosticAgentType. Loose substring matching
preserved (separate concern from polarity); canonical strict-match path
in CapabilityDiscovery is unchanged.

## Missing 'sales' in resource handler

server/src/mcp-tools.ts:2058 hard-coded ["creative", "signals", "buying"]
in its agent-resource type filter — pre-#3540 'buying' was the
(incorrectly) only sell-side type, so 'sales' was never added. Post-#3540
this means agents://sales returns "Unknown resource type". Added 'sales'
to the list. 'buying' kept because per #3766 the type is now meaningful.

## Tests

New: server/tests/unit/diagnostic-agent-type-inference.test.ts pins the
12-case polarity matrix, including an exhaustive "NEVER returns 'buying'"
invariant assertion. If anyone reverts polarity back to 'buying', the
cases marked `// pinning #3774` fail loudly.

12/12 pass. Typecheck clean.

## Out of scope

- Tightening the diagnostic helper's loose substring matching to align
  with CapabilityDiscovery.inferAgentType's strict matching. Loose
  matching is preserved here intentionally — diagnostic endpoints serve
  pre-onboarding probes where tool-name conformance is incomplete.
  Worth a follow-up if the diagnostic endpoints' false-positive rate
  becomes a concern.
- Replacing the diagnostic helper with the canonical CapabilityDiscovery
  inference helper (different match semantics — strict vs loose). Same
  follow-up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant