Skip to content

feat: schema mapping, delete fix, and gap closures CEP-6#81

Merged
ContextVM-org merged 2 commits into
ContextVM:mainfrom
harsh04044:feat/cep6-pr4-integration-tests-cleanup
May 21, 2026
Merged

feat: schema mapping, delete fix, and gap closures CEP-6#81
ContextVM-org merged 2 commits into
ContextVM:mainfrom
harsh04044:feat/cep6-pr4-integration-tests-cleanup

Conversation

@harsh04044
Copy link
Copy Markdown

@harsh04044 harsh04044 commented May 21, 2026

Final PR closing CEP-6 implementation #76.

What's in this PR:

Schema-to-kind mapping table - replaced the field-presence if-else chain in handle_announcement_response() with a typed mapping table. Each entry validates field types (arrays must be arrays, strings must be strings), matching TS SDK's safeParse semantics. Rejects malformed payloads like {"tools": null}.

Delete fix - delete_announcements() now fetches existing events per kind via fetch_events() and publishes kind 5 with ["e", event_id] tags for each found event. Added fetch_events() to RelayPoolTrait and MockRelayPool.

Integration tests - roundtrip publish→discover for all 5 kinds, delete tag format verification, schema rejection tests.

Gap closures - 5 tests added for CEP-6 parity with TS SDK:

  • profile metadata custom field preservation through kind 0 content
  • profile metadata error resilience (no panic on publish failure)
  • private server publishes relay list but not announcements
  • relay_list_urls and bootstrap_relay_urls are independent
  • profile metadata published regardless of is_announced_server

Also fixed a wrong integration test that expected the old ["k", kind] deletion behavior.

@ContextVM-org
Copy link
Copy Markdown
Collaborator

I reviewed this against the TS reference and overall this looks good to me.

The schema-to-kind mapping and the delete fix both look aligned with the intended CEP‑6 behavior, and the added parity coverage is solid.

I only have a few small follow‑ups:

  • I’d add a real failure‑path test for publish_profile_metadata() (src/transport/server/announcement_manager.rs:519) by making the mock relay pool return an error from publish, just to prove we really swallow/log publication failures without panicking.
  • I’d rename delete_announcements_k_tags_match_kinds() (tests/transport_integration.rs:1521) to reflect the new e‑tag behavior, since the current name still reads like the old expectation.
  • As a small cleanup, I think delete_announcements() (src/transport/server/announcement_manager.rs:362) is already correct as written, but it could use the SDK’s dedicated NIP‑09 helper EventBuilder::delete() with EventDeletionRequest instead of manually assembling the kind 5 event. I don’t see this as a correctness issue, just a slightly more idiomatic rust‑nostr usage.

Nothing here feels blocking to me; these are just small cleanup/follow‑up improvements.

@harsh04044
Copy link
Copy Markdown
Author

harsh04044 commented May 21, 2026

addressed all three - renamed the test to delete_announcements_uses_e_tags_for_published_events, replaced the no-op failure test with a real FailingPool that injects publish errors, and refactored delete_announcements() to use EventBuilder::delete() with EventDeletionRequest.

@ContextVM-org ContextVM-org merged commit 48fe6c6 into ContextVM:main May 21, 2026
3 checks passed
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.

2 participants