fix(registry-sync): apply agent.profile_updated events to the local agent index - #7093
Conversation
…gent index RegistrySync.applyEvent() handled agent.discovered and agent.removed but silently no-op'd agent.profile_updated, one of the event_type values the registry-event.json schema has declared since PR adcontextprotocol#5054 (2026-05-26) for "an agent inventory profile changed". The feed poller advances its cursor past these events regardless, so a locally-synced agent's channels, markets, format_kinds, etc. never converge with the registry after the initial bootstrap — a permanent staleness gap the class's own docstring ("polls the change feed to stay current") explicitly promises against. Add a case that merges the event payload onto the existing AgentIndex entry, matching the merge pattern already used for property.updated immediately below it. A plain copy of agent.discovered's full-snapshot upsert would be wrong here: per the schema's changed_fields description, agent.profile_updated payloads may carry only the fields that changed, so defaulting absent fields to [] / 0 / false (as agent.discovered does for a brand-new entity) would silently wipe out unrelated, unchanged profile data on every partial update. Adds two unit tests to registry-sync.test.ts: one confirming a partial update merges onto the existing profile without clobbering untouched fields, one confirming the event is a no-op for an agent not already in the index (matching property.updated's existing convention). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFth3sHBrJbHuFeA8Cegdc
bokelley
left a comment
There was a problem hiding this comment.
Two changes are needed before merge:
-
Do not spread the full event payload into AgentProfile. The payload includes advisory changed_fields and permits additional properties, so get/list/search can return runtime fields that are not part of AgentProfile. Destructure the metadata or explicitly construct a whitelisted profile patch before upsert.
-
The current crawler produceEventsFromDiff emits agent.discovered, agent.removed, and authorization events, but never agent.profile_updated. The new unit tests call applyEvents directly, so the stated end-to-end stale-index path remains untested and, with this server, unproduced. Please either add the producer/diff coverage here or narrow this PR explicitly and link a concrete follow-up that completes the production path.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean, well-tested server-only fix.
This PR adds the missing agent.profile_updated case to RegistrySync.applyEvent(), closing a silent event-drop bug where the poller advanced its cursor past these events while leaving stale profile fields in the local AgentIndex. It correctly uses a partial-merge (mirroring property.updated) rather than agent.discovered's full-snapshot upsert, avoiding field zeroing. Server-only change with accompanying unit tests; no protocol surface touched, so no changeset required.
Checked against the decision table:
- No critical/high/medium findings (row 1, 8 do not fire).
gated_paths: false— row 2 does not apply despitereview_decision: CHANGES_REQUESTED.high_risk: false, no deletions/modifications to sensitive paths (rows 3–5 do not fire).- No prior decision, no no-auto-approve team match (rows 6–7 do not fire).
- No protocol/schema/docs/changeset surface touched — none of the repo-specific spec gates apply.
Falls through to row 9 → approve.
|
Issue #7095 proposes the producer-side counterpart to this PR — wiring Generated by Claude Code |
Addresses review feedback on adcontextprotocol#7093 point 1: applyEvent()'s agent.profile_updated case spread the raw event payload onto the existing AgentProfile. Per registry-event.json's agentProfilePayload def, additionalProperties is true and this event type additionally carries an advisory changed_fields array — neither is part of AgentProfile, so the bare spread let both leak into the stored profile and back out through agents.get()/list()/search(). Add agentProfilePatchFromPayload(), which explicitly whitelists only real AgentProfile fields (name, channels, property_types, markets, categories, tags, delivery_types, format_kinds, property_count, publisher_count, has_tmp, category_taxonomy) from the payload before merging, mirroring collectionFromPayload's existing explicit- construction pattern in the same file rather than property.updated's spread (which has the same latent issue but is out of scope here). Strengthened the existing merge test to assert changed_fields does not leak into the stored profile, and added a dedicated test that sends changed_fields plus an unrelated advisory field and an unrecognized "type" key (the schema's field name for this concept, distinct from AgentProfile's agent_type) and asserts the stored profile's key set is exactly the AgentProfile fields — nothing else. Verified by reverting the source change (keeping the tests): both new assertions fail against the old spread ('changed_fields' present on the stored profile). Restored the fix — 22/22 pass. npm run typecheck clean. Point 2 of the review (producer coverage for agent.profile_updated) is addressed separately: see PR description update and adcontextprotocol#7095 for the follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFth3sHBrJbHuFeA8Cegdc
|
Thanks for the detailed review — both points are addressed. 1. Whitelist the payload before merge. You're right that the spread let 2. Producer coverage. I looked into wiring Taking your suggested alternative: narrowed the PR description to state plainly that this PR fixes the consumer-side dead code and the producer doesn't yet emit this event type, and filed #7095 with the specific scope needed to complete the production path (bulk previous-profile read, an Pushed as a new commit on the same branch; PR description updated with the full detail on both points. |
Problem
RegistrySync(server/src/registry-sync/index.ts) is the reference in-memoryclient that bootstraps from the registry API and then polls the change feed
to "stay current" (its own docstring).
RegistrySync.applyEvent()switcheson
event_typeand handlesagent.discovered/agent.removed, but has nocase for
agent.profile_updated— one of theevent_typeenum values theregistry-event.jsonschema has declared since PR #5054 (merged 2026-05-26)specifically for "an agent inventory profile changed" (e.g. an agent adds a
market, channel, or format_kind after it was first discovered).
Because the
switchhas nodefaultbranch and the poller advances itscursor regardless of whether an event was handled,
agent.profile_updatedevents are silently dropped: the feed cursor moves past them, but the local
AgentIndexentry for that agent is never touched. A consumer ofRegistrySync(this repo's ownregistry-sync-collection-index.test.tsandserver/tests/unit/registry-sync/treat it as a reusable module, and itsclass docstring documents
registry.agents.search({ channels: ['ctv'], markets: ['US'] })as the intended usage) will keep matching stalechannels/markets/format_kinds/etc. for any already-known agentindefinitely, until the next full bootstrap (a
cursor_expiredrecovery orprocess restart).
Confirmed on current
upstream/mainviagit show HEADbefore editing.git log -S "agent.profile_updated" -- server/src/registry-sync/index.ts static/schemas/source/core/registry-event.jsonshows the schema gained thisevent type in #5054 (2026-05-26), which never touched
server/src/registry-sync/index.ts;RegistrySyncitself predates that PR(added in #1807) and was never updated to add the new case. No existing
open or closed issue covers this (searched
RegistrySync,agent.profile_updated,registry-sync applyEvent,agent profile stale).Fix
Added an
agent.profile_updatedcase to theswitchinapplyEvent().Deliberately not a copy of
agent.discovered's full-snapshot upsert:per the schema's
changed_fieldsfield description, anagent.profile_updatedpayload may legitimately carry only the fields that changed. Reusing
agent.discovered's pattern ((payload.channels as string[]) ?? [], etc.)would silently zero out every field the producer omitted on a partial
update — a subtler regression than doing nothing. Instead the new case
merges the payload onto the existing
AgentIndexentry, mirroring the exactpattern the file already uses for
property.updatedimmediately below it,and — matching that same sibling case — is a no-op when the agent isn't
already in the local index (nothing to merge onto).
Test
Added two tests to
server/tests/unit/registry-sync/registry-sync.test.ts,following the file's existing
(sync as any).applyEvents([event])convention:handles agent.profile_updated event by merging onto the existing profile— seeds a full agent profile, applies a partial
agent.profile_updatedevent that only sends
markets, and asserts the changed field updateswhile untouched fields (
channels,property_count,has_tmp) survive.ignores agent.profile_updated for an agent not already in the index— matches the no-op convention already used for
property.updated.Proof the test is real: reverted the source change (kept the tests) and
re-ran
npx vitest run --config server/vitest.config.ts server/tests/unit/registry-sync/registry-sync.test.ts— 20 passed / 1failed, with the merge test failing (
expected [ 'US' ] to deeply equal [ 'US', 'CA' ], i.e. the partial update was silently dropped). Restored thefix and re-ran the same command — 21/21 passed. Ran
npm run typecheck(
tsc --project server/tsconfig.json --noEmit) — clean. Rannode scripts/check-changeset-protocol-scope.cjs upstream/main— this is aserver-code-only change, not protocol-surface-scoped, so no changeset is
required per
.agents/playbook.md. Final diff is two files, +64 lines (17in
index.ts, 47 in the test file), no lines removed.🤖 Generated with a Claude Code session as part of an ongoing series of
narrowly-scoped, independently-verified fixes to this repository.
Update: addressing @bokelley's review
Both requested changes are addressed, with the second one intentionally
scoped down rather than forced in — see below.
1. Whitelist the payload before merge (fixed)
agentProfilePayloadinregistry-event.jsonhasadditionalProperties: true, and theagent.profile_updatedbranch specifically layers anadvisory
changed_fieldsarray on top. The original fix's{ ...existing, ...(payload as Partial<AgentProfile>) }spread let both ofthose — and any other non-
AgentProfilekey a future producer attaches —leak straight into the stored profile and back out through
agents.get()/list()/search().Replaced the spread with an explicit
agentProfilePatchFromPayload()helper that whitelists only real
AgentProfilefields (name,channels,property_types,markets,categories,tags,delivery_types,format_kinds,property_count,publisher_count,has_tmp,category_taxonomy) before merging — mirroringcollectionFromPayload'sexisting explicit-construction pattern in the same file (the sibling that
already does this safely), rather than
property.updated's spreadimmediately below (which has the identical latent leak but is out of scope
for this PR).
Strengthened the existing merge test to assert
changed_fieldsdoesn'tleak into the stored profile, and added a new test
(
does not leak advisory or unrecognized payload fields into the stored profile) that sendschanged_fieldsplus an unrelated advisory field andan unrecognized
typekey (the schema's field name for this concept —distinct from
AgentProfile'sagent_type, so it's correctly excludedeither way) and asserts the stored profile's key set is exactly the
AgentProfilefields, nothing else.Verified: reverted the whitelist change (kept the tests) — both new
assertions fail against the old spread (
changed_fieldspresent on thestored object). Restored the fix — 22/22 pass.
npm run typecheckclean.2. Producer coverage (scoped out, follow-up filed)
I looked at wiring
produceEventsFromDiff(server/src/crawler.ts) up toactually emit
agent.profile_updated, and it's not a narrow addition likepoint 1 was — genuinely new infrastructure is needed, not a copy of the
agent.discovered/agent.removedemission pattern:(
CrawlerService.snapshotAgentState()) only captures domainassociations (
Map<string, { domains: Set<string> }>), builtspecifically for the authorization-granted/revoked diff — it carries no
profile field values to diff against.
AgentInventoryProfilesDatabasehas no bulk "read the previous profilefor these agent URLs" method today (
getProfile()is single-agent;search()is paginated/scored) — one would need to be added, in the samesingle-round-trip spirit as
federatedIndex.getAllAgentDomainPairs()(used in
snapshotAgentState()specifically to avoid O(N) per-agentqueries).
AgentProfilefields are arrays built fresh fromSets each crawl inbuildInventoryProfiles(), e.g.channels: [...channels]), so naive!==/JSON.stringifycomparison would false-positive on reordering.There's already a directly analogous helper to model this on —
adagentsChangedFields()inserver/src/db/publisher-db.ts(~line 101),which normalizes both sides through a sorted-key
stableManifestStringbefore comparing, feeding
publisher.adagents_changed's ownchanged_fields— but no equivalent exists yet for agent profiles.That's a new DB read path + a new diff helper + restructuring the
buildInventoryProfiles→produceEventsFromDiffhandoff in the crawlloop, each of which deserves its own review rather than being folded into
this consumer-side fix. Per your suggested alternative, I'm narrowing this
PR's scope explicitly: this PR fixes the consumer-side dead code in
RegistrySync.applyEvent(); the producer does not yet emitagent.profile_updated, so the tests here exerciseapplyEvents()directly rather than a real end-to-end crawl-to-consumer path.
Filed #7095 to track completing the production side,
with the specific suggested scope (bulk previous-profile read, an
agentProfileChangedFieldshelper mirroringadagentsChangedFields, theproduceEventsFromDiffwiring, and producer-level test coverage inserver/tests/unit/crawler-format-kind-events.test.tsor a sibling file).