feat(context-agent): registry-fed property bitmap + context-signal targeting

[ohalushchak-exadel]

What this does

Brings the production cmd/context-agent closer to feature-complete against the
context-match part of the TMP spec, in two parts, plus a spec-conformance fix to
the registry package that the first part depends on.

1. Registry-fed property bitmap (opt-in)

With REGISTRY_ENABLED=true the agent runs a background registry sync loop and
feeds the engine's global property bitmap from a live registry.PropertyIndex
instead of the static PROPERTY_RIDS env stopgap (which remains the fallback).

• Index backends: memory or redis/Valkey (via registry/redisstore); optional file-backed cursor.
• Sync goroutine is panic-recovered.
• /live gains a registry_sync staleness check, driven by a new
  SyncerConfig.OnSuccessfulPoll beacon so a quiescent-but-healthy feed still proves liveness.

2. Context-attribute signal targeting

PackageContextConfig gains a ContextSignals profile (any_of/none_of)
evaluated against the signal:{signal_owner_id}:{key_types}:{values} keyspace in a
single deduped MGet per request.

• Privacy boundary enforced at three layers (write-time validate, read-time ExpandKeys, engine fail-closed); identity key types are rejected, so the context path can never issue an identity-keyed lookup. Raw URLs normalized to url_hash.
• Topic values are taxonomy-namespaced; per-cfg cartesian expansion is capped, plus a per-profile cfg-count cap and a request-wide plan cap — all fail closed for both any_of and none_of.
• A comma-bearing request value is dropped (it can't shadow a compound-tuple key).
• Skipped end-to-end when no candidate carries a profile.

pkgconfigstore gains a batched MGet so candidate configs load in one round-trip.

3. registry.Property.PropertyRID: uint64 → string (spec fix) ⚠️ public-type change

The AdCP property registry catalog spec defines property_rid as a catalog-assigned
UUID v7 ("the shared key for TMP matching"), and the change-feed property.created
payload carries it as a string. The registry package modeled it as uint64, so a
spec-compliant feed event failed to unmarshal (string → uint64) and was dropped —
the registry-fed bitmap from part 1 could never match an inbound UUID property_rid
and served empty offers for every spec-compliant request.

This changes Property.PropertyRID to string and the index's RID dimension with it
(LookupByRID(string), PropertyRID() string, byRID map[string]*Property). The
context-agent bitmap adapter now matches on property_rid only (not the property_id
slug), per spec. The router's separate string-typed registry (router/registry.go)
was already spec-correct and is untouched.

⚠️ This touches public types in the registry module (Property.PropertyRID,
LookupByRID/PropertyRID signatures). Flagging for release-please / SemVer — same
consideration applies to the new required ContextStorage.SignalMGet method. Confirm
the versioning policy for these modules.

Scope notes

• The writer that populates config:pkg:* and signal:* is out of scope — reader/engine side only.
• No new root-module dependency; registry deps are local replace directives in the cmd/context-agent submodule.

Testing

go test ./... passes in the root, registry, and cmd/context-agent modules
(registry/redisstore + glidestore vet clean).

Known pre-existing failure

registry/TestSyncer_AppliesAuthorizationRevoked fails on this branch, but it
also fails on main with this PR's registry/syncer.go change reverted — not
introduced here.

[aao-ipr-bot]

Approving. Identity-vs-context boundary holds end-to-end — Cfg.Validate, ExpandKeys, and signalsPass are three independent gates and ExpandKeys re-checks IsAllowed before composing any key, so the write-side Store.Set bypass acknowledged in the diff is caught on read.

Things I checked

• signalstore.AllowedKeyTypes matches ArtifactRefType ∪ {country, region, metro, topic} minus raw url — identity types (eid, email, the UIDType enum in tmproto/types_gen.go:88-101) are intentionally absent. (targeting/signalstore/signalstore.go:60-86)
• Topic namespacing {tax.source}:{tax.id}:{topicID} matches topicstore.Taxonomy.String() at topicstore/topicstore.go:87-92, so the writer and reader are guaranteed to key the same bytes from one source of truth. Unaccepted-taxonomy silent drop mirrors the existing topic-match path at engine.go:144-153.
• HashURL canonicalization (tmproto/urlcanon.go:33-70, Blake3-256 + std base64 over scheme/www/query/fragment-stripped lowercase form) is the right shape for the writer/reader to agree on; ArtifactRefTypeURL correctly hashes before key composition and KeyType("url") is rejected by Validate (delimiter collision).
• Fail-closed posture: NoneOf evaluated before AnyOf so a cap-trip on a blocklist still rejects. Pinned by TestMatchProfile_NoneOfCapTripFailsClosed in targeting/signalstore/signalstore_test.go:209-240. MGet error fails-closed every package with a profile while bare packages still evaluate — TestContext_SignalMGetError_FailsClosed.
• Shutdown() idempotency: context.CancelFunc is safe to call repeatedly, receive on a closed syncDone returns immediately, closeRedis tolerates a repeat Close(). No race. (cmd/context-agent/registry_setup.go:170-181)
• syncErr atomic.Pointer[error] and lastSuccess atomic.Int64 are read/written across goroutines correctly; the local nil-check before *err deref in LivenessCheck.Fn is sound.
• pkgconfigstore cache MGet alignment: missIDs[j] == packageIDs[missIdx[j]] by construction so the Add key in cache.go:97 is equivalent to the input id.
• Service.Put validation rejects identity-keyed and missing-signal-id cfgs at write time — TestService_PutRejectsInvalidContextSignals covers KeyType("eid") returning ErrCfgUnsafe.

Follow-ups (non-blocking — file as issues)

• Delimiter injection in signal:* keys. Key() writes custom artifact-ref values, topic IDs, and region / metro.value verbatim with no escape — a publisher-supplied value containing , or : can shadow a legitimately-written compound-tuple key when both sides happen to land on the same byte sequence. The KeyTypes segment is a partial discriminator (different cfg shapes produce different middle segments), so the realistic collision needs the writer's data to already carry , inside a single value — narrow, but real. Fix is one of: reject any value containing : or , inside appendUnique, or have Key() return "" on bad bytes (same fail-closed shape as the empty/mismatched case at signalstore.go:174). (targeting/signalstore/signalstore.go:189-194, targeting/engine_signals.go:48,55,82)
• No profile-level or request-side cardinality cap. maxKeysPerCfg=4096 bounds a single cfg, but PlanLookup can union N cfgs at near-cap each, and req.ArtifactRefs is unbounded at the engine boundary — appendUnique dedupes but a maliciously-sized request feeds the cartesian. Service.Put also accepts arbitrarily many cfgs per profile. Recommend: cap len(data[kt]) at extract, cap len(out) in PlanLookup, and enforce a small len(AnyOf)+len(NoneOf) ceiling in Profile.Validate. All three should fail-closed with ErrCfgUnsafe to inherit the existing engine pattern.
• SignalMGet is a required method on ContextStorage. The interface lives at the module root (targeting/context_storage.go:97) so this is a public Go API change for any external implementer. ContextConfigs was added optionally via feature-detect (contextConfigBatcher) — SignalMGet could have been the same. The branch commit is feat(context-agent): with no ! or BREAKING CHANGE: footer; if release-please treats targeting as a stable surface, the next tag will need a SemVer minor at minimum and arguably a major. Worth confirming the release-please policy here.
• Whole-batch error observability regression. When the storage implements contextConfigBatcher and the whole batch errors, e.metrics.StoreError(ctx, "context_config", err) is recorded once for the whole candidate set; the pre-batch path recorded one per failing package. Either emit N events for the batch path, or document the metric semantic change. (targeting/engine_signals.go:166-175)
• OnSuccessfulPoll fires once per page, not once per poll cycle. The docstring at registry/syncer.go:23-27 reads as a per-iteration beacon but the hook is inside the page loop, so a 10-page bootstrap fires 10 times. Benign for liveness (count lets operators distinguish), but pin the semantic in the test. (registry/syncer.go:206-208)
• Spec confirm: geo.metro is currently flattened to "{system}:{value}" (engine_signals.go:138-147). MetroAreaSystem exists as an enum in tmproto/types_gen.go:30-35 but Geo is map[string]any and the schema for geo.metro shape isn't in this tree. Worth a one-liner confirming against adcp/schemas/tmp/context-match-request.json upstream that the wire shape is {system, value} and not a flat string.
• signal_owner_id naming. Reader/writer use this name uniformly here, but if upstream AdCP signals-agent vocabulary calls it signal_provider_id or similar, reconcile before any cross-agent wire surface lands.

Minor nits (non-blocking)

1. debug.Stack() called twice. registry_setup.go:153,156 — both calls capture the same goroutine stack. Collapse to one local and reuse.
2. signalLogger = slog.Default() falls through to the global. The rest of the engine has no logger field, so this is a one-off surface. If more diagnostic sites land, thread a logger through ContextEngineConfig instead.
3. appendUnique is O(n²). Fine at current request sizes; revisit if the artifact-ref cap ever lifts.

LGTM. Follow-ups noted below.

[aao-ipr-bot]

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

[aao-ipr-bot]

Substantial feature add. Privacy boundary holds end-to-end — signalstore.Cfg.Validate (targeting/signalstore/signalstore.go:116-129), ExpandKeys (targeting/signalstore/signalstore.go:212-254), and Service.Put (targeting/pkgconfigstore/pkgconfigstore.go:65-67) each independently reject any KeyType outside allowedKeyTypes, and extractSignalLookupData (targeting/engine_signals.go:35-91) never projects identity dimensions in the first place. Three load-bearing layers, no escape hatch. Clean fail-closed posture on ErrCfgUnsafe propagation (none_of evaluated before any_of so a cap trip on a blocklist still surfaces). One blocking question below before approve.

Things I checked

• ArtifactRefType → KeyType projection (targeting/engine_signals.go:108-132) matches the canonical AdCP set 1-for-1, with url correctly folded into url_hash via tmproto.HashURL.
• Cartesian cap (signalstore.go:228-230) is checked progressively inside the loop, before the keys allocation — no allocation-DoS path.
• MatchProfile none_of/any_of ordering is correct and tested at signalstore_test.go:244.
• signalsPass returns false on both ErrCfgUnsafe and fetchErr, mirroring URL-filter / topic-match patterns elsewhere in the engine.
• Registry sync: OnSuccessfulPoll fires after cursor save (registry/syncer.go:206), correct liveness semantics. Panic recovery in runSyncer captures and stores; cancel propagation through Shutdown is idempotent.
• Credential paths (REGISTRY_REDIS_PASSWORD, REGISTRY_FEED_TOKEN) flow only into redis.Options{Password} and the bearer header. No echo through fmt.Errorf wrapping — cfg.RedisAddr is host:port, not a URI carrying credentials.
• TLS on registry Redis (registry_setup.go:251-256) — tls.Config{MinVersion: TLS12} with empty ServerName is acceptable: stdlib derives ServerName from the host portion of Addr. Only fails open on IP-literal Addr; example.env uses hostnames.

Open question (would flip me to approve)

registryPropertyBitmap adapter likely cannot match a spec-compliant property_rid. cmd/context-agent/registry_setup.go:236-249. The spec defines ContextMatchRequest.PropertyRID as a UUID-v7 string (tmproto/types_gen.go:119: "Property catalog UUID (UUID v7). Globally unique, stable identifier..."). The registry's Property.PropertyRID is uint64 (registry/property_index.go:13). The adapter tries (a) LookupByID(rid) — matches against property_id, slug-shaped — and (b) ParseUint + LookupByRID(n) — matches the uint64. A UUID-v7 string fails both: it isn't a slug, and ParseUint rejects the hyphens. With REGISTRY_ENABLED=true, every spec-compliant inbound short-circuits at the top of Evaluate and the agent serves empty offers.

This is opt-in (default REGISTRY_ENABLED=false) and the fail mode is fail-closed, so it's not a leak — but it likely means the registry path serves nothing until resolved. What's the intended bridge? Two ways this could already be fine:

• Your registry feed populates Property.PropertyID with the UUID-v7 form so LookupByID(rid) matches (the field-name collision is just confusing).
• Your operators run with property_rid numeric internally and inbound requests carry the numeric form, accepting the spec divergence.

A line in the example env explaining the assumed registry shape would resolve this. If neither bridge holds, the adapter needs a third lookup dimension.

Follow-ups (non-blocking — file as issues)

• Doc/code drift on storage.SignalMGet nil signals. targeting/contextagent/storage.go:85-90 returns (nil, nil) when s.signals == nil; DecodeValues returns nil on length mismatch; MatchProfile(data, nil) is then called. A none_of-only profile passes vacuously — no key hit → no block. Production wires signals: rawStore at setup.go:402 so unreachable today, but buildStorage's doc claims this "skips every package... as a fail-closed safety measure." Either reject nil at buildStorage, or return an error from SignalMGet when signals == nil && len(keys) > 0 so fetchErr propagates and signalsPass fails-closed.
• No request-wide cap on PlanLookup keys. signalstore.go:331-369 — per-cfg cap is 4096, but with N candidate packages × multi-key any_of cfgs the deduped MGet could plan 4096 × N keys. Realistic N keeps this bounded; a maxKeysPerPlan (e.g. 65536) tripping ErrCfgUnsafe for the request would be defense in depth.
• Topic namespacing is a private invariant. Spec ContextSignals carries TaxonomySource/TaxonomyID on the envelope, not per-topic. The {source}:{id}:{topicID} convention in targeting/engine_signals.go:74-85 is necessary for multi-taxonomy disambiguation but isn't a spec form — anyone authoring cfgs out-of-band must know to prefix. Worth a docstring on PackageContextConfig.ContextSignals.
• Registry liveness window during slow first bootstrap. lastSuccess is seeded at bundle-return time (registry_setup.go:119), but Hydrate runs synchronously before that. With BootstrapLimit=10000, a slow first-page fetch plus retries could approach the 5-minute staleness threshold before the first OnSuccessfulPoll fires. Worth either stamping lastSuccess from inside the goroutine just before the first FetchFeed, or documenting the threshold relative to bootstrap budget.
• runSyncer panic recovery uses b.logger without a nil-check (registry_setup.go:149-160). Always non-nil from main, but a test calling runSyncer against a hand-built bundle nil-derefs inside recover. Defensive if b.logger != nil closes the gap.
• Defense-in-depth on signalstore.Key() escaping (signalstore.go:173-196). Publisher-controlled topic values containing , produce malformed read keys. Denial-of-match, not key-smuggling, but appendUnique could reject values containing , or : outside the known system:value / source:id:topic shapes.

Minor nits (non-blocking)

1. Memory-backend file cursor mismatch. cmd/context-agent/registry_setup.go:90-92 lets memory-backend index + file-backed cursor coexist. A restart resumes against the cursor with an empty index, then bootstraps incrementally instead of doing the full reload the memory-backend comment promises. Worth either rejecting the combination or documenting the resulting behavior.

Verdict: LGTM after the property_rid adapter question is resolved — if your registry feed populates PropertyID with the UUID-v7 form, the existing LookupByID branch is the answer and we ship.

[aao-ipr-bot]

LGTM. Follow-ups noted below. Architecturally right: hot path skips the entire signal stage when zero candidates carry a profile, single deduped MGet per request, fail-closed everywhere a buyer's brand-safety cfg could otherwise silently pass.

Things I checked

• Three-layer privacy guard against identity key types: write-time signalstore.Cfg.Validate (signalstore.go:124), write-time gate at pkgconfigstore/pkgconfigstore.go:65 in Service.Put, read-time ExpandKeys IsAllowed (signalstore.go:230), engine signalsPass fail-closed (engine_signals.go:281-297). Verified by TestContext_SignalInvalidKeyType_FailsClosed and TestPlanLookup_PropagatesCfgError.
• extractSignalLookupData (engine_signals.go:36-92) never projects identity dimensions into LookupData — every write site goes through a fixed switch with no identity arms. tmproto.ContextMatchRequest (types_gen.go) carries no identity-bearing field. The Go-side struct enforces the wire contract.
• ArtifactRefType→KeyType projection is complete. ArtifactRefTypeURL is correctly normalized via tmproto.HashURL onto KeyTypeURLHash (engine_signals.go:48-50); ArtifactRefTypeURLHash maps to the same KeyTypeURLHash (engine_signals.go:121-123). KeyTypeURL is deliberately absent — raw URLs would collide with the , delimiter.
• Topic namespacing reuses topicstore.Taxonomy.String() ("source:id" form) — same form the existing topic-match path keys on, so signal and topic-match cannot drift apart on the prefix scheme. Tested by TestContext_SignalTopicTaxonomyNamespacing and TestContext_SignalTopicUnacceptedTaxonomyDrops.
• Comma-bearing values are dropped at a single sink (engine_signals.go:105-115) before reaching signalstore.Key, so the , tuple delimiter is unshadowable from publisher-controlled bytes (artifact_ref custom values, country, region, metro, topic). TestContext_SignalCommaValueDropped exercises the custom-ref path end-to-end.
• maxKeysPerCfg=4096 checked after every multiplication on int (signalstore.go:238-241) — no allocation reaches a memory-unsafe size. maxKeysPerPlan=65536 is the request-wide backstop and fails closed for every profile-bearing package (TestPlanLookup_RequestWideCapFailsClosed). maxCfgsPerProfile=256 is operator-controlled, not request-controlled.
• signalsPass (engine_signals.go:268-300) fail-closes in every error path I traced: fetchErr != nil, MatchProfile err, ErrCfgUnsafe. Logger nil-checked at line 287 so a hand-built engine in tests can't nil-deref. TestContext_SignalMGetError_FailsClosed pins it.
• storage.SignalMGet returns an error when s.signals == nil but keys were requested (contextagent/storage.go:86-99) — without this, a none_of-only profile would pass vacuously when no signal store is wired. Right shape.
• registryBundle.Shutdown is genuinely idempotent: cancel is stdlib-idempotent, <-syncDone on a closed channel returns immediately on repeat, closeRedis tolerates re-close. TestRegistryBundle_ShutdownIdempotent exercises double-call and nil receiver.
• runSyncer (registry_setup.go:158-181) recovers panics into syncErr and surfaces them via the LivenessCheck (lines 207-230). debug.Stack() captured once in the recover, per the third commit's fix.
• buildRegistry Hydrate failure paths all call closeRedis(redisCloser) before returning (registry_setup.go:103-114). No leak on partial-failure.
• clonePackageContextConfig clones the new ContextSignals pointer via cloneSignalProfile + cloneSignalCfg (cache.go:146, 150-176) — profile, AnyOf/NoneOf slices, and per-cfg KeyTypes slice are all freshly allocated, so a cache-returned Profile can't be mutated into a stored one.
• OnSuccessfulPoll callback wired per page in registry/syncer.go:207-209 (not per drain cycle), driving lastSuccess so a quiescent-but-healthy feed still proves liveness. Documented at SyncerConfig.OnSuccessfulPoll (syncer.go:20-29).
• The signal gate runs BEFORE the URL block/allow filter in the per-package loop (engine.go:306 vs :310). Intentional — fail-closed on signal errors drops the package before brand-safety even runs — but inverts the conventional "policy before personalization" ordering. Acceptable; worth a comment.

Follow-ups (non-blocking — file as issues)

• ExpandKeys short-circuits on empty data before validating remaining key types (signalstore.go:223-242). For a cfg with KeyTypes=[url_hash, eid] and an empty data[url_hash], iteration 0 returns (nil, nil) at line 235 before iteration 1 reaches the disallowed eid. In a none_of position that cfg would silently not-block instead of propagating ErrCfgUnsafe. Not wire-exploitable — extractSignalLookupData never populates data[identity_key], and the write-time Validate gate catches this shape today. But the PR's "three-layer" claim is read-time-shape-dependent for one cfg shape. Fix is a single pre-pass: validate every KeyType in a top-of-function loop before any data[kt] lookup, mirroring Cfg.Validate at line 116. One-liner change; carries a test for the mixed-key none_of case.
• PlanLookup is all-or-nothing on cap trip / unsafe cfg (signalstore.go:353-389). One misconfigured cfg in one package blackholes every signal-gated package in the request. Fail-closed direction is correct, but a noisy single cfg can take out the whole signal stage. Consider per-profile isolation in a follow-up — log the offending owner/index so the operator can locate the bad cfg without a transcript.
• cachedReader.MGet does not validate inner.MGet length alignment (pkgconfigstore/cache.go:86-97). Direct reader enforces this at pkgconfigstore.go:142-144; cache wrapper trusts the inner length and indexes missIdx[j] blind. Defense-in-depth — add the same length check.
• OnSuccessfulPoll fires when cursor.Save fails under the 3-failure threshold (registry/syncer.go:192-209). The save-failure branch increments saveFailures and logs but flows through to the hook. Liveness reports healthy while persisted cursor is stale; the escalation to a hard return only triggers on the third consecutive failure. Acceptable for liveness intent (in-memory state did advance) but worth either moving the hook above the save block or commenting the deliberate choice.
• Default empty REGISTRY_KEY_PREFIX (cmd/context-agent/registry_config.go:95, registry/redisstore/store.go:60-71). Empty prefix warns once and defaults; two deployments sharing a Valkey without setting it collide. Hard-fail in production builds would be safer than warn-and-default.
• Metro key shape {system}:{value} is engine-invented (engine_signals.go:151-165). Reader and writer must agree byte-for-byte. Pin the contract in the entity.go ContextSignals docstring alongside the existing topic-namespacing note, otherwise the next writer-side change will drift silently.
• Signal-gate-before-URL-filter ordering (engine.go:306 vs :310). Defensible — fail-closed signal errors should drop the package before the URL filter runs — but inverts the conventional cheapest-reject-first ordering. One-line comment explaining the choice keeps it from being re-litigated.
• maxKeysPerCfg=4096 is tight for a topic-only cfg with a high-cardinality publisher (signalstore.go:270). Realistic publishers send <100 topics so it's theoretical; flag if a buyer needs KeyTypes=[topic] against a publisher fanning out heavy IAB taxonomies.
• Double Hydrate (registry_setup.go:103-114 then registry/syncer.go:87-99). Idempotent via idx.hydrated.CompareAndSwap, so this is safe — just confusing on read. Pick one site to own the call.

Minor nits (non-blocking)

1. Key(0, …) is permitted. signalstore.go:184-207: SignalOwnerID=0 produces signal:0:…. Not a bug, but if the writer convention disallows owner 0 (most do), Cfg.Validate could reject it.

The third commit's title says "address review nits" but the request-wide plan cap and the comma-value signalstore drop landed there — substantive privacy-hardening, not nits. Worth flagging in the squash message if this lands as one commit.

LGTM.

[aao-ipr-bot]

Approving. The three-layer identity-key-type rejection holds end-to-end — I walked every path from ContextMatchRequest to a Valkey MGet and could not reach the identity keyspace. None_of cap-trip propagates ErrCfgUnsafe correctly. Comma drop at extraction prevents writer-key shadowing. Clean fix shape.

Things I checked

• signalstore.Cfg.ExpandKeys (targeting/signalstore/signalstore.go:229-231) is the only call site that builds signal:* keys outside tests — Key is otherwise unreachable from the data path. Identity key types die at IsAllowed before any MGet is planned.
• MatchProfile (signalstore.go:308-334) evaluates NoneOf first; a none_of cap-trip propagates ErrCfgUnsafe and surfaces fail-closed even when any_of would pass. signalstore_test.go:245-280 pins the reproducer.
• signalsPass (engine_signals.go:268-300) returns false on fetchErr != nil AND on MatchProfile err. Bare packages (no profile) still pass vacuously when other packages' fetch failed — TestContext_SignalMGetError_FailsClosed (engine_signals_test.go:186-213) checks that exact shape.
• Service.Put (pkgconfigstore/pkgconfigstore.go:65-67) gates writes on Profile.Validate. The reader-side decodeConfig doesn't re-validate, but the engine path is load-bearing — every ExpandKeys re-checks IsAllowed. Three-layer claim stands.
• extractSignalLookupData (engine_signals.go:36-92) projects ArtifactRefs / geo / metro / topic with comma-drop at the appendUnique seam. Metro is composed system:value; topic is tax.String() + ":" + topic. Operator-controlled acceptedTaxonomies plus strconv.Itoa numeric ID means no , injection vector.
• cachedReader.MGet (pkgconfigstore/cache.go:66-99) deep-clones via cloneSignalProfile, covering KeyTypes slice and per-cfg structs. TestCache_HitClonesConfigToIsolateCallers verifies isolation.
• registry_setup.go lifecycle: runSyncer panic-recovers with debug.Stack; Shutdown is idempotent across cancel/syncDone/closeRedis; LivenessCheck reads lastSuccess atomically.
• registry/syncer.go:207 fires OnSuccessfulPoll after the persist + cursor-advance branch. TestSyncer_OnSuccessfulPollFires covers the zero-event drain case.
• Conventional-commit story: feat(context-agent) + two fix(context-agent) review-nit follow-ups. No wire-path symbol removed or renamed. ContextStorage gains a method (see follow-up below).
• The PR description's "known pre-existing failure" on registry/TestSyncer_AppliesAuthorizationRevoked checks out — the syncer.go change here is the OnSuccessfulPoll hook only, which the failing test doesn't exercise.

Follow-ups (non-blocking — file as issues)

• DecodeValues mismatch path is fail-open. If a signalMGet impl returns len(values) != len(keys) without an error, signalstore.DecodeValues returns nil (signalstore.go:395-398), fetchSignalsForCandidates returns (nil, nil) (engine_signals.go:256), and a none_of-only blocklist profile then sees an empty fetched map and passes vacuously through MatchProfile. Shipping impls align by construction (InMemory, contextagent/storage.go pass-through), so this is not reachable today — but the PR's own posture is fail-closed beats fail-open. Cheapest fix: in fetchSignalsForCandidates, when keys is non-empty and DecodeValues returns nil, return (nil, ErrCfgUnsafe).
• decodeConfig skips Profile.Validate. pkgconfigstore.decodeConfig (pkgconfigstore.go:163-169) doesn't re-validate. The engine-side ExpandKeys still catches identity types; maxKeysPerCfg + maxKeysPerPlan bound the blast radius. Defense-in-depth, not a DoS amp.
• HTTPS pin on REGISTRY_FEED_URL. registry_config.go validate (L117-138) doesn't require an https:// scheme when FeedToken != "". A fat-fingered http://... ships the bearer in plaintext. Operator config but trivial to check at boot.
• OnSuccessfulPoll fires on cursor-save retry path. registry/syncer.go:194-209 invokes the hook after a cursor.Save failure when saveFailures < 3, despite the docstring claim of "fetches AND persists cleanly." Gate on saveFailures == 0 or update the doc.
• lastSuccess seeded to time.Now() before first poll (registry_setup.go:128). If the first feed call hangs > 5 min, /live reports healthy throughout. Seed to zero and special-case zero in the check, or document the intentional window.
• Cfg.Validate accepts comma-bearing SignalID. A misconfigured cfg with SignalID=\"a,b\" validates, persists, and then never matches anything because splitCSV decoded segments can't equal \"a,b\". Reject , in SignalID at Validate-time for symmetry with the read-path comma drop.
• ContextStorage gains SignalMGet without a breaking-change marker. Adding a required method to the exported targeting.ContextStorage interface (targeting/context_storage.go:97) breaks any out-of-repo implementer. MUST FIX §6 covers "remove or rename" — adding a required method isn't strictly in scope — but release-please will tag this as a minor bump, which is understated. Worth a BREAKING CHANGE: footer if anyone external is implementing ContextStorage, otherwise a one-line "no external implementers yet" call-out somewhere durable.

Minor nits (non-blocking)

1. appendUnique comma drop is silent. engine_signals.go:105-108 — bump a counter so an operator can detect a key_type whose values are 100% dropped. The comment explains the why; a metric would explain the what.
2. maxKeysPerPlan test comment. signalstore_test.go:391 — comment says "17 cfgs > 65536 keys" but the loop generates 32. Math is fine, comment is stale.
3. registry_setup.go:142-143 — the empty-bitmap warning fires when properties.Count() == 0 after Hydrate. With a Redis backend resuming from a saved cursor the index should already be populated, so this branch is really for the memory-backend cold-start case. Worth one line distinguishing the two so an oncall doesn't chase a phantom.

Safe to merge.

[aao-ipr-bot]

LGTM. Follow-ups noted below. The privacy partition is the load-bearing claim — verified at all three layers (write Profile.Validate, read Cfg.ExpandKeys + IsAllowed, engine fail-closed via signalsPass on a non-nil signalErr) — and identity key types never reach extractSignalLookupData, so a misconfigured cfg cannot key an MGet on user data even if the gate downstream were ever bypassed.

Things I checked

• Three-layer privacy boundary: targeting/signalstore/signalstore.go:71-101 (allowed set), :223-242 (read-time IsAllowed before any lookup), :148-166 (Profile.Validate rejects identity at write), and targeting/engine_signals.go:225-257/:268-300 (engine signalErr fail-closed on plan or fetch failure). No path from PackageContextConfig.ContextSignals to a Valkey lookup bypasses all three.
• none_of fail-closed direction is the right shape: ErrCfgUnsafe from ExpandKeys propagates through MatchProfile (signalstore.go:308-334) and signalsPass returns false rather than passing vacuously — the specific case the engine pattern needs.
• Cartesian containment: per-cfg cap 4096 at signalstore.go:239-241, request-wide cap 65536 at :371-373, comma-bearing values dropped at engine_signals.go:106-108 so a request value cannot shift the parse and shadow a compound-tuple key. HashURL is base64 (tmproto/urlcanon.go) — no delimiter collision. Topic values are taxonomy-prefixed (engine_signals.go:75-86) so the same numeric topic id under iab:7 vs custom can't collide.
• Property.PropertyRID uint64→string (commit 810ae12): spec-correct against the catalog UUID-v7 per tmproto/types_gen.go:119 and adcp/types_gen.go:7052. router/registry.go was already string-typed, so the two registries are now aligned. The change-feed event property.created carries it as a string ("019539a0-…") and would have failed to unmarshal into the prior uint64, dropping silently — registry/syncer.go confirms the previous path would have produced the empty-bitmap behavior the PR cites.
• Conventional-commit story: no exported wire-path symbol change in adcp/types.go, tmproto/*, router public Router*, targeting public, tmpclient, or cmd/router/main.go env-var contracts. fix(registry): is defensible framing for a spec-correctness fix; see Follow-up #1.
• cmd/context-agent/Dockerfile:7 correctly copies registry/ into the build context to resolve the new local replace directives in cmd/context-agent/go.mod:43-45.
• Test plan: PR cites the pre-existing registry/TestSyncer_AppliesAuthorizationRevoked failure as reproducing on main with this PR's syncer.go reverted — verifiable claim, not introduced here. No unchecked manual smoke item in the PR body.

Follow-ups (non-blocking — file as issues)

1. `fix(registry):` for a public Go API break of the registry submodule. `Property.PropertyRID uint64→string` breaks any external importer of `github.com/adcontextprotocol/adcp-go/registry`. Spec-correctness framing is defensible — the prior code couldn't unmarshal a real feed event — but downstream Go consumers reading release-please notes get no signal. Worth a `BREAKING CHANGE:` footer or a `fix!:` recut on the next registry-submodule release; at minimum, a release-note line in the registry submodule's CHANGELOG when it next versions.
2. Hydrate against pre-PR persisted registry state will fail-fast at boot. `LoadProperties` (`registry/redisstore/store.go:142-146`, `registry/glidestore/store.go:155-167`) will return `json: cannot unmarshal number into Go struct field Property.property_rid of type string` against any Redis populated before this PR. `cmd/context-agent/registry_setup.go:102-104` treats Hydrate failure as fatal. Document a `Clear` / migration step in the upgrade notes so the first deploy on a warm Valkey doesn't refuse to start.
3. `Cfg.SignalOwnerID` is still `uint64` (`signalstore.go:108`). The only numeric ID left on what is now a string-typed surface. Encoded into the key via `strconv.FormatUint` (`:191`). If AdCP's signals/curation spec models owner as `agent_url` (string), this is the next type fix in the same line as PropertyRID. Worth a one-line check against the upstream signals task schema.
4. Bundle `lastSuccess` is seeded from `time.Now()` before the syncer has polled (`cmd/context-agent/registry_setup.go:127`). A registry feed unreachable from boot shows `/live` green for the full `registrySyncStaleThreshold = 5*time.Minute`. The comment at `:124-126` is self-aware about it, but a startup with an unreachable feed currently looks healthy to a load balancer for the first five minutes. Seed to zero, or document the grace window in the deploy guide.
5. Memory backend + non-empty `CursorPath` is a footgun (`cmd/context-agent/registry_setup.go:90-99`). A restart resumes the cursor against an empty index, so everything emitted before the saved cursor stays absent — the staleness probe goes green (feed is polling) while the bitmap is materially incomplete. Reject the combination at config validation, or fail the readiness probe when index size collapses across restart.
6. `pkgconfigstore.decodeConfig` does not re-validate the loaded profile. A pre-PR config (>256 cfgs, or a writer that bypassed `Service.Put`) is accepted on read and only backstopped by the request-wide `MaxKeysPerPlan`. Burns CPU before the cap trips. Have `decodeConfig` validate and treat invalid as "no config" — the engine then skips the package.
7. `REGISTRY_REDIS_TLS=false` with a non-empty password is silently accepted (`example.standalone.env:90`, `registry_setup.go:67-78`). Operators following the example ship the registry password in cleartext. WARN on startup, or refuse to start when `RedisPassword != "" && !RedisTLS`.

Minor nits (non-blocking)

1. `signalstore.Key` silently returns `""` on invariant violation. `targeting/signalstore/signalstore.go:184-207`. An empty key gets seeded into the dedup map and pushed into the MGet batch, where it crowds `maxKeysPerPlan` and produces a `signal::` row no Cfg can match. The invariant is internal — return an error or panic so a future caller that hands in mismatched slices gets a real signal, not a corrupt batch.
2. Signal-store nil-guard is dead code in the only real wiring. `targeting/contextagent/storage.go:90-97` fails closed when `s.signals == nil`, but `buildBundle` always passes `rawStore`. Defense-in-depth is fine; consider asserting non-nil at construction so the runtime check is documentation, not a live branch.
3. `OnSuccessfulPoll` fires per feed page, not per drain cycle (`registry/syncer.go:207-209`). The docstring matches behavior. But `registrySyncStaleThreshold = 5m` sized for "one missed cycle" assumes steady-state polling; a slow bootstrap drain with >5m between pages will trip liveness. Probably fine; flagging in case the feed gets fat.

Two features in one commit (registry sync + signal targeting) — a future bisect for either lands on the 2,848-line merge. Worth noting only because the title spells it out.

Safe to merge.

[aao-ipr-bot]

Approving. Substantive PR with the privacy boundary done right: three-layer identity firewall (write-time Validate, read-time ExpandKeys, engine fail-closed), fail-closed posture on every cap trip and storage error, and the property_rid spec fix unblocks the registry-fed bitmap from serving empty offers on every spec-compliant request.

Independent verdicts that drove the call:

• ad-tech-protocol-expert: spec-correct on all three claims. property_rid as UUID-v7 string matches tmproto/types_gen.go:119 ("primary key for TMP matching"); the signal:* identity firewall matches the TMP context-match "MUST NOT contain user identity" contract; none_of cap-trip propagation is the right posture.
• security-reviewer: identity firewall safe at all three layers, fail-closed posture safe under MGet failure / PlanLookup cap trip / ContextConfigs batch error / nil-store-with-keys, no identity-bearing log fields, no exploitable delimiter shadowing in deployed config.

Things I checked

• Three-layer identity firewall: Cfg.Validate rejects identity key_types at targeting/signalstore/signalstore.go:128-131; pkgconfigstore.Put validates at targeting/pkgconfigstore/pkgconfigstore.go:65-67 before any Set; ExpandKeys re-checks at signalstore.go:234-237; engine signalsPass fails the package closed at targeting/engine_signals.go:285-297.
• extractSignalLookupData at engine_signals.go:36-92 never sources identity dimensions — even a hypothetical downstream bypass cannot supply identity values to the MGet.
• MatchProfile evaluates NoneOf before AnyOf at signalstore.go:317-325 so a blocklist cap trip surfaces before any_of can mask it (pinned by signalstore_test.go:245-280).
• appendUnique drops comma-bearing request values at engine_signals.go:106-108. The writer uses the same Key() encoding so the drop is a no-op for any value that could legitimately have been written.
• KeyTypeURL is intentionally absent from allowedKeyTypes; raw URL refs project onto KeyTypeURLHash via tmproto.HashURL at engine_signals.go:48-51 — no path builds a signal:* key from a raw URL.
• Property.PropertyRID change matches tmproto/types_gen.go:119. The router's separate string-typed registry (router/registry.go) was already spec-correct and is untouched as the PR description claims.
• registryBundle.Shutdown is idempotent — cancel is idempotent, syncDone receivers on a closed channel both unblock, closeRedis tolerates double-close, the panic recover captures stack into syncErr.

Follow-ups (non-blocking — file as issues)

• Versioning the multi-module breaking changes. registry.Property.PropertyRID (uint64→string) and ContextStorage.SignalMGet (new required method) are public-Go-API breaks. release-please-config.json only manages the adcp package, so neither commit needs ! for release-please — but registry is a published Go module (github.com/adcontextprotocol/adcp-go/registry) and downstream consumers will see a compilation break on go get -u. Document the pre-1.0 policy for registry / targeting, or cut explicit module tags. The PR description already flagged this.

• none_of cfg keying on a dimension the publisher omitted silently doesn't fire. ExpandKeys returns (nil, nil) for an absent dimension (signalstore.go:239-241), so a topic-keyed blocklist is bypassed when the publisher sends no topics at all. Documented intent, but worth a metrics.ContextEvaluated(ctx, StageSignalMatchAbsentDimension, ...) or debug log so operators can see blocklist evasion when it matters.

• cmd/context-agent/registry_setup.go:75 — boot-time client.Ping(ctx) has no timeout. Unresponsive Redis stalls buildRegistry until SIGTERM. Wrap the ping with context.WithTimeout(ctx, 5*time.Second).

• url_hash artifact-ref values aren't shape-checked at engine_signals.go:122-123. The spec-canonical form is 44-char base64 from Blake3-256. If the spec already constrains the wire shape, moot; otherwise a length/charset guard would close the gap that appendUnique's , drop doesn't cover.

Minor nits (non-blocking)

1. registry/property_index.go:107 — mirror the Put guard on Remove. Put at :79-81 only inserts into byRID when PropertyRID != ""; Remove deletes unconditionally. The delete-on-"" is a safe no-op today; consistency is cheap.

2. targeting/signalstore/signalstore.go:118-134 — Cfg.Validate doesn't reject SignalOwnerID (or SignalID) containing : or ,. Operator-supplied config, write-time governance — not exploitable in deployed config. Defense-in-depth at validation is cheap.

3. targeting/signalstore/signalstore.go:376 — len(out) > maxKeysPerPlan checks after the append. Slice transiently holds cap+1 entries before erroring. if len(out)+1 > maxKeysPerPlan is tighter.

Approving on the strength of the three-layer identity firewall plus the spec-anchored property_rid fix.

[aao-ipr-bot]

Approving. The three-layer privacy boundary on signalstore composes correctly under adversarial review, and property_rid as UUID-v7 string is independently anchored to the schema-generated tmproto/types_gen.go:119 — the uint64 was one-sided drift, not a representational choice.

Things I checked

• targeting/signalstore/signalstore.go:228-270 — cartesian indices clean; the three caps compose correctly (per-cfg 4096, request-wide 65536, per-profile 256).
• signalstore.go:317-339 + engine_signals.go:268-300 — the PlanLookup skip → MatchProfile re-expand → signalsPass fail-closed chain holds. The recent isolate-unsafe-cfg fix (3f61ca2) preserves per-package isolation: skipped at plan time, re-evaluated and rejected at match time when MatchProfile re-runs ExpandKeys.
• engine_signals.go:36-92 — extractSignalLookupData is the sole producer of LookupData and surfaces only context attributes. The appendUnique comma-drop (L105-L115) blocks the publisher-controlled country=US,eid shadow walk.
• registry/property_index.go — uint64→string switch consistent across Put/Remove/Hydrate. byRID insertion now guards on non-empty RID, replacing the uint64==0 sentinel. ad-tech-protocol-expert confirmed the AdCP property registry catalog spec ships property_rid as UUID-v7 string per tmproto/types_gen.go:119 — the schema-generated type carries the inline comment "Property catalog UUID (UUID v7)."
• cmd/context-agent/registry_setup.go:157-180 — runSyncer panic recovery + atomic syncErr ordering. closeRedis is reached on every error-return path in buildRegistry. Shutdown is idempotent.
• Conventional-commit story: six commits, the feat: headline is correct. The registry / targeting Go-module breaks are not under release-please (release-please-config.json tracks only adcp), so the missing ! markers do not trip MUST FIX #6. PR body calls this out explicitly.

Follow-ups (non-blocking — file as issues)

• signalstore.Cfg.ExpandKeys defense-in-depth gap on empty owner / signal id. Cfg.Validate rejects empty SignalOwnerID and SignalID, but ExpandKeys / Matches do not re-check. Key() returns "" on empty owner, which MGet silently skips — a none_of cfg with an empty owner that bypasses Validate (operator pokes Valkey directly, future writer code path skips Service.Put) fails OPEN. One-line guard at the top of ExpandKeys. targeting/signalstore/signalstore.go:228.
• SignalOwnerID accepts \":\". No symmetric check for the keyspace delimiter on the owner string. A governance-pipeline writer could land SignalOwnerID = \"7:url_hash:HASH_A:custom\" and shadow another owner's keyspace under split-parse. Reject \":\" and \",\" in Cfg.Validate. signalstore.go:118-134.
• KeyTypeCustom is the privacy boundary's load-bearing convention, not a code-enforced rule. The three-layer story works because eid / email / coreid are named in the rejection list. custom is the catch-all the writer can populate with anything — by convention identity material stays out, but nothing in this PR enforces it. Architecturally, a privacy boundary enforced by name-match against a fixed list with one catch-all entry next to it is worth a note. Either remove from AllowedKeyTypes or add a normative writer-side contract test. signalstore.go:56.
• /live masks a misconfigured feed for 5 minutes after startup. lastSuccess is seeded with time.Now().Unix() at cmd/context-agent/registry_setup.go:127 before any poll has run. A deploy with a bad FeedURL / FeedToken reports healthy through the entire registrySyncStaleThreshold = 5 * time.Minute window. Seed to 0 (or sentinel) and require lastSuccess > 0 && age < threshold in the liveness check.
• targeting.ContextStorage interface break needs a CHANGELOG note. Adding a required SignalMGet breaks any external embedder of the public targeting package. Release-please tracks only adcp, so not a release-please block — but the existing contextConfigBatcher extension pattern at engine_signals.go:211-213 is the shape this should have followed for backwards compat. PR body already flags it; add a CHANGELOG entry covering the targeting module break.
• signalstore keyspace shape is implementation-defined, not AdCP-spec. The package doc reads like a contract; spell out that the contract is with this repo's writer, not with the protocol. The 54cff92 byte-compat claim on SignalOwnerID (uint64→string) is load-bearing only against the in-repo writer. signalstore.go:4-12.
• registry/property_index.go Put does not detect RID collision across distinct property_ids. A Put whose new PropertyRID already maps to a different PropertyID overwrites byRID[RID] without clearing the prior owner's byID entry, leaving the two indexes out of sync. Pre-existing — not introduced here — but with strings the collision surface is larger than the old catalog-assigned uint64 space. Detect and either reject or evict the prior byID entry. property_index.go:78-84.

Minor nits (non-blocking)

1. pkgconfigstore.MGet docstring promises a per-key log the code does not emit. The comment at targeting/pkgconfigstore/pkgconfigstore.go:150-156 says the reader logs the package id on decode error; the code just continues with no logger plumbed through. Either wire a logger through NewReader or drop the claim from the comment.
2. signalsPass discards fetchErr after fail-closing the package. targeting/engine_signals.go:281-284. The request-wide error was already recorded via StoreError at the fetch site, but a per-package debug line would help operators chase empty-offer responses to the right cause.
3. signalstore.go:243-246 cap check after multiply. Not exploitable at current bounds (4096 × 4096 stays well inside int64), tidier to check total > maxKeysPerCfg before the next total *= len(vals).

Approving on the strength of the three-layer privacy composition holding against publisher-controlled input plus the property_rid UUID fix being independently anchored to the schema-generated tmproto/types_gen.go:119.