feat(relay,desktop): canonicalize agent definitions on kind:30175 (Phase 2)

[wesbillman]

What (Phase 2 — relay canonicalization, Bundle 1)

kind:30175 becomes the single canonical agent-definition event; kind:30177 slims to instance-only state. One PR per the bundling directive: spec edit + relay conformance + desktop writer/reader/reconcile changes.

Spec (docs/nips/NIP-AP.md)

• display_name is the sole required content field. system_prompt is optional — a definition can be pure configuration (product ruling: prompt visibility stays status-quo world-readable; encrypted variant logged as a future candidate).
• Four behavioral fields added as reserved definition-level defaults: respond_to (+ allowlist), mcp_toolsets, parallelism. Status this bundle: parsed but not yet applied — the wire type tolerates and preserves them (foreign data survives deserialization), but the local store does not carry them and writers do not emit them; the instance-copy-at-creation behavior activates with the create-path unification (B5). Locked by behavioral_defaults_are_staged_not_applied. RespondTo reuses the existing wire strings — one encoding.
• "Slimming: kind:30177 (instance state)" replaces the deprecation framing (reviewer Finding 1): 30177 is keyed by agent pubkey — it is the per-instance cross-device sync channel and stays write-live for instance fields. Writers omit the definition quad (system_prompt/model/provider/persona_source_version) for definition-linked instances only.
• Definition-less exception (reviewer addendum): an instance with no linked definition IS its own definition and keeps emitting the quad — old readers parse a slimmed event successfully and would otherwise overwrite their local snapshot with absent values, with no restore path. Self-retires once B5's backfill makes every record definition-backed; also makes Phase 2 order-independent from B5.
• Mixed-version note: pre-revision clients drop prompt-less 30175s at serde parse (benign divergence, not corruption; logged not error-per-event).

Hash stability (reviewer Finding 3 — the hard pin)

persona_content_hash = sha256 of the serde encoding, and it drives the drift badge + persona_source_version. The widening uses skip_serializing_if on every new/widened field and the writer always emits Some(prompt), so pre-revision content bytes are unchanged on upgrade — no fleet-wide badge flip. Guarded by a fixture-bytes invariance test: real old-shape JSON (the NIP-AP Event-1 vector) parses and re-serializes byte-identically, and its digest equals the direct sha256 of the fixture.

Desktop changes

• agent_event_content: slims the quad when persona_id.is_some(); the exception branch lives in the projection fn itself so all write paths (save + boot reconcile) inherit it (reviewer gate 2).
• apply_inbound_managed_agent: mirror logic — definition-linked inbound never touches the local quad (absent = not-carried, never clear); definition-less inbound applies it unconditionally.
• build_persona_event now delegates to persona_event_content (single projection point).
• Boot reconcile: one-time republish wave disclosed — first boot after upgrade re-retains every definition-linked agent (fat→slim content change); slimming_republish_wave_is_one_time proves the second boot is a true no-op.

Relay

Envelope validation unchanged (content was never parsed). Conformance tests prove: prompt-less 30175 ingests, behavioral-field 30175 ingests (unknown-field tolerance), legacy fat 30177 stays accepted (e2e).

Frozen-shape renegotiation (disclosed)

PersonaEventContent gains fields (hash-stable per above). ManagedAgentRecord and UpdateManagedAgentInput untouched.

Validation

• Desktop Rust: 1057/1057, clippy clean, both fmt gates clean
• Relay: 16/16 persona-envelope tests (2 new); pre-existing audit_chain/audit_records failures reproduced on clean main — not this PR
• Frontend: tsc clean, 2174/2174 unit (no frontend changes; suite run as merge-base sanity)
• New/updated tests: fixture-bytes hash invariance, slim-vs-standalone projection, linked-edit-not-wire-state, inbound quad-skip + definition-less apply, one-time republish wave idempotence, legacy-fat e2e

[wpfleger96]

Two independent reviews (spec+relay+hash-stability, and desktop writer/reader/reconcile) plus my own source pass. I verified every critical finding at the current head a95c3ca.

The two high-blast-radius traps are handled correctly — no defects there:

• Hash-stability holds. persona_event_content always emits Some(system_prompt) (including the empty string), every widened field has skip_serializing_if, declaration order matches the NIP-AP vector, and the fixture-invariance test starts from a real pre-revision JSON literal → parse → reserialize byte-identical → hash equals a direct SHA-256 of the fixture (not regenerated from the new struct, so not tautological). The drift badge will not flip on upgrade.
• Quad-slimming is correct. Single projection fn agent_event_content gated on record.persona_id.is_some(); both write paths inherit it. Definition-linked → quad absent (not null); definition-less → quad present.
• Inbound never-clear-on-absent is correct, including the linked→definition-less flip edge — an unlink carries the full quad, so it's an overwrite-with-real-values, not a clear. No new corruption path.
• Relay stays content-opaque for kind:30175, still accepts legacy fat kind:30177, and a prompt-less body parse-errors before apply on old clients (skip, not clear).

Requesting changes on one substantive spec/impl divergence plus three test-coverage gaps (inline). The spec/code disagreement is the only thing I'd truly gate on; the rest is test hardening.

Inline comments below.

[wpfleger96]

Spec/impl divergence on the four behavioral defaults (the one substantive finding).

This paragraph — and the field table just above (respond_to, respond_to_allowlist, mcp_toolsets, parallelism) plus the PR body — state these are live definition-level defaults "copied onto instances at creation." The wire type PersonaEventContent now parses them, but the local data path drops them on the floor:

• persona_from_event maps only the original persona fields into PersonaRecord (persona_events.rs:186 restores system_prompt but nothing maps the four behavioral fields).
• persona_event_content hardcodes all four to None/empty on outbound (persona_events.rs:303-306).
• PersonaRecord (types.rs) has no fields for them at all.

So a spec-compliant kind:30175 from another client carrying these defaults is parsed here, the defaults are silently dropped, and it re-publishes without them — the documented contract says they're active, the code makes them evaporate. There's no "reserved/future-bundle" language to mark this as intentional staging.

The cheap fix (recommended for this bundle): narrow the spec + PR body to mark these four as reserved/parsed-but-not-yet-applied in this bundle, and add a test locking that staged behavior. Wiring the full data path (new PersonaRecord fields + mapping through persona_from_event/persona_event_content + instance-creation copy) is clearly later-bundle work and shouldn't gate this canonicalization step. Widening the wire type now so future clients' data survives is defensible — but only if the doc says "parsed, not yet applied." As written, doc and code disagree.

[wpfleger96]

This is the impl side of the spec divergence flagged on NIP-AP.md. All four behavioral defaults are hardcoded absent on outbound, and persona_from_event (line 186) never maps them inbound — so they round-trip to nothing. Either wire the data path or narrow the spec to "reserved/parsed-but-not-applied" this bundle (recommended) + add a staged-behavior test.

[wpfleger96]

Test gap: republish-wave test asserts only one of four slimmed fields.

This asserts !row.content.contains("system_prompt") but not model, provider, or persona_source_version. The fixture sets persona_source_version = Some("abc123") (line 232) and the seeded fat row carries it too — a regression that re-emits model/provider/persona_source_version while dropping the prompt would still pass.

assert!(!row.content.contains("system_prompt"), "must be slimmed");
assert!(!row.content.contains("\"model\""), "must be slimmed");
assert!(!row.content.contains("\"provider\""), "must be slimmed");
assert!(!row.content.contains("persona_source_version"), "must be slimmed");
assert!(!row.content.contains("abc123"), "source version value must be absent");

[wpfleger96]

Test gap: definition-less apply omits persona_source_version.

The fixture has no persona_source_version field and the assertions don't verify it, yet local_agent() seeds persona_source_version: Some("local-hash"). A failure to apply this 4th quad field on a definition-less sync would silently leave the stale local value — wrong for a record that is its own definition.

Fix: add "persona_source_version": "remote-version" to the fixture JSON and assert a.persona_source_version == Some("remote-version".to_string()) after apply.

[wpfleger96]

Test gap: relay conformance tests don't exercise the real ingest path.

persona_envelope_accepts_promptless_content and persona_envelope_accepts_behavioral_fields call only validate_persona_envelope. The production path calls that helper from ingest_event_inner and then stores via the parameterized-replaceable branch, but these tests skip auth/scope/global-storage/round-trip — and the e2e addition covers legacy fat kind:30177, not 30175. A future content parse or gate introduced in the real ingest path would pass these helper tests silently.

For a relay conformance claim, the boundary should be the actual ingest or a round-trip: add an ingest-level unit test (or ignored e2e) that publishes a prompt-less kind:30175 and a behavioral-field kind:30175, asserts OK accepted, then queries the coordinate and compares content byte-for-byte.

[wpfleger96]

🟢 Optional (nit): the hash-stability path is sound by inspection, but the two easiest regressions to miss if someone later changes the projection or skip attributes are system_prompt: "" (empty prompt) and the minimal old-writer shape (display_name + system_prompt, no optional fields). Worth an explicit test case for each.

[wesbillman]

All six review findings addressed in b558b6e0b:

1. Spec/impl divergence (the substantive one): spec narrowed — the four behavioral fields are now marked Reserved in the field table and a Status: reserved paragraph states parsed-but-not-yet-applied with activation deferred to the create-path unification (B5). PR body updated to match. The staged behavior is locked by a test (behavioral_defaults_are_staged_not_applied): fixture with all four populated → wire parse preserves them → PersonaRecord round-trip drops them → re-serialize emits none. B5 flipping the behavior must consciously break this test.
2. Republish-wave test: now asserts all four slimmed fields absent (system_prompt, model, provider, persona_source_version) plus the value-level abc123 check — exactly the suggested block.
3. Definition-less apply: fixture gains persona_source_version: "remote-version" and the assert verifies it applies.
4. Real ingest path: two ignored e2e tests added to e2e_persona.rs — prompt-less and behavioral-field 30175s published through the live relay, OK accepted asserted, coordinate queried back, content compared byte-for-byte.
   5+6. Hash-stability nits: explicit cases for system_prompt: "" and the minimal old-writer shape, both asserting byte-identical round-trip.

Validation: 1061/1061 desktop Rust, clippy clean, both fmt gates clean.