fix(l2ps-messaging): canonicalize peer identity — address #978 review

[Shitikyan]

Follow-up to #978, addressing the review there.

CodeRabbit (Major) — duplicate identities. In #978 the 0x strip was validation-only; the raw key was still used as the peer identity (peers map, ws.data.publicKey, broadcasts). So 0x<key> and <key> registered as two separate peers for one cryptographic key, and re-registration didn't close the other socket. Fixed by canonicalising the identity everywhere.

Greptile (P2) — uppercase 0X. hexToUint8Array already accepts 0x/0X, but the register validation only stripped lowercase 0x. The canonical form now strips both.

Changes

• New pure export function canonicalizeKey(key) — strip a leading 0x/0X, lowercase.
• Used for every identity/lookup: register (peer record, ws.data, online-peer list, registered/peer_joined payloads, queued-message delivery), send recipient (to), and request_public_key (targetId).
• The signed proof keeps the client's raw key (they signed register:{their-key}:{ts}), so verification is unaffected.

Tests: 4 canonicalization cases added (0x/0X/bare → one identity; lowercasing; interior-0x untouched; idempotent). 76 pass.

Honest note: requestId echo (from #978) is threaded through all direct responses; verifiable in the diff. Full request/response-level coverage needs driving the real server — the existing integration.test.ts re-implements a fake server, so a real-server harness is a separate improvement worth a ticket.

Base: stabilisation. Refs DEM-778.

Summary by CodeRabbit

• Bug Fixes

  • Improved messaging reliability by treating public keys consistently, regardless of letter casing or a leading 0x/0X prefix.
  • Peer registration, message delivery, recipient lookup, and conversation history now work consistently across key formats.
  • Improved error handling for failed history proof verification while preserving detailed diagnostic logging.

• Data Migration

  • Existing message records are updated to use the standardized key format.

• Tests

  • Added coverage for key normalization, including casing, prefixes, and repeated normalization.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

@Shitikyan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 53478244-b6c4-4297-b72f-88d495a0ca31

📥 Commits

Reviewing files that changed from the base of the PR and between d5e0e04 and 5416ddf.

📒 Files selected for processing (1)

• src/features/l2ps-messaging/L2PSMessagingServer.ts

Walkthrough

L2PS messaging now canonicalizes public-key identities by removing one leading hexadecimal prefix and lowercasing keys. Routing, peer state, persistence, lookups, history queries, migration, and helper tests use this format.

Changes

L2PS key canonicalization

Layer / File(s)	Summary
Canonical key contract and migration src/features/l2ps-messaging/keys.ts, src/migrations/1782680000000-CanonicalizeL2PSMessageKeys.ts, src/features/l2ps-messaging/L2PSMessagingServer.ts	canonicalizeKey removes one leading 0x or 0X prefix and lowercases the key. The migration applies this transformation to stored message keys.
Server identity and routing flow src/features/l2ps-messaging/L2PSMessagingServer.ts	Peer registration, routing, notifications, self-send checks, public-key lookup, and history lookup use canonical identities. Raw keys remain available for proof verification.
Service persistence and validation src/features/l2ps-messaging/L2PSMessagingService.ts, src/features/l2ps-messaging/tests/L2PSMessagingServer.test.ts	Message persistence, queued-message lookup, and conversation history queries use canonical keys. Tests cover normalization and idempotence.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant L2PSMessagingServer
  participant L2PSMessagingService
  participant MessageStore
  Client->>L2PSMessagingServer: Send message with public key
  L2PSMessagingServer->>L2PSMessagingServer: canonicalizeKey
  L2PSMessagingServer->>L2PSMessagingService: Route with canonical identity
  L2PSMessagingService->>MessageStore: Store or query canonical keys
  MessageStore-->>L2PSMessagingService: Matching messages
  L2PSMessagingService-->>L2PSMessagingServer: Messages or history
  L2PSMessagingServer-->>Client: Messaging response

Loading

Possibly related PRs

• kynesyslabs/node#978: Both changes update L2PS public-key handling in L2PSMessagingServer.ts.

Poem

A rabbit saw keys in a casing parade,
“One form for each peer,” the rabbit said.
Prefixes hop off, uppercase sleeps,
Routes and records keep matching beats.
Proofs keep the raw key near,
Canonical messages now persevere.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: canonicalizing L2PS peer identities, and it references the related review issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/l2ps-messaging-canonical-key

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

The PR consistently canonicalizes L2PS peer identities across registration, routing, persistence, queued delivery, and history retrieval.

• Adds a shared key canonicalization helper that removes a leading hexadecimal prefix and normalizes casing.
• Migrates existing message rows so legacy key representations remain reachable through canonical lookups.
• Adds focused tests for equivalent key representations and idempotence.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/features/l2ps-messaging/L2PSMessagingServer.ts	Applies canonical identities consistently to peer registration, routing, event payloads, and lookup paths while preserving raw signed-proof inputs.
src/features/l2ps-messaging/L2PSMessagingService.ts	Canonicalizes persisted message keys and queued-message and history queries, resolving the previously reported lookup gaps.
src/features/l2ps-messaging/keys.ts	Introduces a reusable, idempotent canonicalization helper shared across server and persistence boundaries.
src/migrations/1782680000000-CanonicalizeL2PSMessageKeys.ts	Rewrites legacy prefixed or mixed-case message keys using SQL semantics matching the runtime canonicalization helper.
src/features/l2ps-messaging/tests/L2PSMessagingServer.test.ts	Adds focused coverage for prefixes, casing, interior prefix-like text, and idempotence.

_{Reviews (4): Last reviewed commit: "style(l2ps-messaging): no empty catch on..." | Re-trigger Greptile}

[Shitikyan]

Both P1s addressed in aa341d11:

1. History query was noncanonical — handleHistory passed the raw peerKey to getHistory(), but messages persist under canonical keys, so an existing conversation returned nothing. Now canonicalizes peerKey for the query; the raw form is still used for the signed proof, and myKey was already canonical.

2. Canonical lookup stranded legacy queued rows — getQueuedMessages matched only the canonical identity, so rows an earlier build queued under a non-canonical recipient key stayed queued forever. deliverQueuedMessages now also passes the raw registration key and the query matches to_key IN (canonical, raw), so those rows are delivered on reconnect. (New rows are already canonical — the server canonicalizes before processMessage.)

canonicalizeKey moved to ./keys (shared by server + service, no circular import) and re-exported from the server module so existing imports/tests are unchanged. 76 tests pass.

[Shitikyan]

Addressed in d5e0e048 — and this time at the root, not with another read-time patch.

The dual-lookup was the wrong shape: any read-time variant matching is incomplete (as you showed — a row stored 0XABC… isn't recovered when the peer returns as 0xabc…). The fix is to make the stored data canonical so reads need no variant matching at all:

• Migration CanonicalizeL2PSMessageKeys rewrites existing l2ps_messages.from_key/to_key to canonical form (lower(regexp_replace(k, '^0[xX]', '')), matching canonicalizeKey exactly). Idempotent, runs via migrationsRun. Recovers every legacy variant, not just one.
• persistMessage canonicalizes from_key/to_key at the write boundary — every new row is canonical regardless of caller.
• getQueuedMessages / getHistory are now canonical-only; the rawKey dual-lookup plumbing is gone.

Division of responsibility: the server canonicalizes for the in-memory peer map / routing; the service owns canonicalization at the DB boundary (write + read + migrated data). 76 tests pass.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/l2ps-messaging/L2PSMessagingServer.ts`:
- Line 175: Update the catch block around existing.ws.close() in
L2PSMessagingServer to satisfy the no-empty ESLint rule by adding an appropriate
debug log or explanatory comment while preserving the current close-attempt
behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fd6ceff1-fb69-4acf-ae0b-6123685e3e6d

📥 Commits

Reviewing files that changed from the base of the PR and between e4a40b9 and d5e0e04.

📒 Files selected for processing (5)

• src/features/l2ps-messaging/L2PSMessagingServer.ts
• src/features/l2ps-messaging/L2PSMessagingService.ts
• src/features/l2ps-messaging/keys.ts
• src/features/l2ps-messaging/tests/L2PSMessagingServer.test.ts
• src/migrations/1782680000000-CanonicalizeL2PSMessageKeys.ts

[Shitikyan]

Fixed in 5416ddfc — empty catch now carries the WHY (closing an already-dead socket can throw; we replace the connection regardless). eslint reports 0 errors on the touched files.