feat(aao-directory): wire ?include=properties server implementation

[bokelley]

Closes #4890

Schema and docs for ?include=properties are already on main (merged via #4894, spec-only changeset 4890-aao-include-properties.md). This PR wires the server side so the endpoint actually parses the flag and serializes property_ids[].

What changed

• federated-index-db.ts — AgentPublisherDetailRow gains property_ids: string[] | null; getPublishersForAgentDetail accepts includePropertyIds?: boolean; SQL adds a CASE WHEN $6 THEN ARRAY_AGG(dp.property_id ORDER BY dp.property_id) ELSE NULL END subquery that PostgreSQL short-circuits when false (zero cost on default calls). Also fixes properties_authorized to filter dp.property_id IS NOT NULL, making it symmetric with the ARRAY_AGG subquery so len(property_ids) == properties_authorized holds (the spec's MUST invariant — without the fix, properties without a canonical string ID would be counted but not named).
• federated-index.ts — passes includePropertyIds through to the DB layer.
• registry-api.ts — parses ?include (repeated-key form, same encoding as ?status; comma-separated form rejected 400; unknown values rejected 400); passes flag to service; spreads property_ids: r.property_ids ?? [] onto each shaped row when set (SQL NULL from empty ARRAY_AGG coerces to [] — correct: empty array signals "flag was honored, zero results" vs absent field which signals "flag not set"); updates ETag to cover the include flag and resolved IDs; updates AgentPublishersEntrySchema and OpenAPI query schema.
• Tests — DB-level (registry-agent-publishers-detail.test.ts): sorted IDs, empty-set returns null at DB layer, default leaves property_ids null. HTTP-level (registry-api-agent-publishers.test.ts): ?include=properties surfaces IDs, default omits field, empty-authorized returns [], unknown value 400, comma-separated 400, ETag differs with/without flag.

Non-breaking justification: ?include is a new optional query parameter; default behavior (no property_ids in response) is unchanged. property_ids is optional in the Zod schema. Existing clients receive an identical envelope. The properties_authorized fix is a data-accuracy correction; any property missing a canonical property_id slug was never reliably surfaceable anyway.

Changeset: --empty (server-side implementation; the protocol changeset 4890-aao-include-properties.md already covers the schema/docs bump).

Pre-PR review:

• code-reviewer: approved — no blockers. Nit: add HTTP-level test asserting property_ids: [] for zero-authorized case (done). Nit: ?include[foo]=bar silently drops nested ParsedQs (noted, consistent with how Express handles analogous edge cases across the codebase). Nit: 404-probe omits includePropertyIds correctly (no change needed).
• ad-tech-protocol-expert: approved — non-breaking per spec. ?include repeated-key form mirrors ?status exactly. Sorted ARRAY_AGG is correct (deterministic ETag). Blocker fixed: properties_authorized now filters dp.property_id IS NOT NULL, making it symmetric with ARRAY_AGG so the MUST len(property_ids) == properties_authorized invariant holds. r.property_ids ?? [] is correct wire form per OpenRTB pattern (empty array distinct from absent field).

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

• Push fixup commits directly: gh pr checkout <num> →
  fix → push.
• Or re-trigger: comment /triage execute on the source
  issue.

See #3121
for context.

Session: https://claude.ai/code/session_012j248bSRq5CcDQEwuberu1

---

Generated by Claude Code

[aao-release-bot]

Server wiring is clean — ?include parser is symmetric with ?status, ETag composition is right, OpenAPI shape matches the wire contract. One silent semantics change to properties_authorized slips through under the "invariant fix" framing — flagging for follow-up, not blocking.

Things I checked

• ?include parser at server/src/routes/registry-api.ts:7197-7220: repeated-key form, comma-separated rejected 400, unknown values rejected 400. Same encoding as ?status. Nested-object ParsedQs (?include[foo]=bar) falls through to generic 400. Right shape.
• SQL change at server/src/db/federated-index-db.ts:373-385: CASE WHEN $6::boolean THEN (SELECT ARRAY_AGG(DISTINCT dp.property_id ORDER BY dp.property_id) ...) ELSE NULL END. Postgres short-circuits when false — zero cost on default calls. Sorted aggregate keeps the ETag stable.
• ETag at server/src/routes/registry-api.ts:7342-7347: covers include flag and per-row IDs. Different response shapes hash differently. Required for cache correctness.
• Empty-set wire semantics: DB returns null for empty ARRAY_AGG (Postgres native), route coerces to [] via r.property_ids ?? []. Absent-vs-[]-vs-populated three-state matches static/schemas/source/aao/agent-publishers.json:39-46, 78-81. Mirrors OpenRTB opt-in extension convention (omit-when-not-requested).
• OpenAPI deltas at static/openapi/registry.yaml:3555-3565, 3615-3619, 3707-3717, 3767-3771 on both /v1 and /api/v1 paths.
• Tests: HTTP-level covers surfaces, default omit, empty [], unknown 400, comma 400, ETag differs. DB-level covers sorted IDs, empty null, default null.
• Changeset: empty frontmatter is the project convention for server-only follow-up PRs after a spec changeset (4890-aao-include-properties.md already shipped the protocol bump). No double-bump needed for the additive surface.

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

1. properties_authorized count semantics narrowed silently. server/src/db/federated-index-db.ts:362-373 flipped from COUNT(DISTINCT apa.property_id) (FK to discovered_properties.id) to COUNT(DISTINCT dp.property_id) filtered to dp.property_id IS NOT NULL (canonical slug). For any authorized property without a canonical slug, the count drops by 1 — including on default calls with ?include=properties not set. The PR claims this enforces a "spec MUST" of len(property_ids) == properties_authorized; neither static/schemas/source/aao/agent-publishers.json nor docs/aao/directory-api.mdx:164 carries that as normative MUST language (both describe the relationship descriptively). Two paths: (a) land the MUST in spec text in a follow-up so the count change is a documented correction — preferred, slug-less properties are un-addressable downstream anyway; or (b) keep the old count on default calls and only narrow when ?include=properties is set. Either way the changeset prose should call out the default-call behavior change; current framing reads as purely additive.

2. ETag input delimiter-collides on operator-controlled IDs. server/src/routes/registry-api.ts:7347 joins r.property_ids with , inside a |-delimited row string. property_id is TEXT with no shape constraint. [\"a,b\",\"c\"] and [\"a\",\"b,c\"] hash to the same ETag. Theoretical for now (operators control their own slugs) but the surrounding code already runs everything through JSON.stringify — push the array in directly instead of pre-joining and the JSON encoder handles escaping.

3. Test gap: ?include=properties × status=revoked interaction. The SQL surfaces property_ids independent of revocation state, which is the right call (set-diff against a revoked publisher is the use case). No integration test covers it.

4. Test gap: ETag changes when set membership changes but count stays the same. The motivating "count-equality is not set-equality" line deserves a direct test — seed two properties, swap one for another with the same count, assert different ETags.

Minor nits (non-blocking)

1. OpenAPI description duplicated. static/openapi/registry.yaml:3555-3565 and 3707-3717 carry the same description string inside the schema: block and again at the parameter level. Functionally fine, redundant.

Approving on the strength of the wire shape, the empty-changeset convention, and the test coverage. A "fix" that hinges on a MUST nobody can locate in-tree is a notable framing — worth tightening before the count change ships to production dashboards.

[aao-release-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.}