fix(routing): unify sortObjectKeys and guard against circular refs

[ambikeesshh]

Problem

MessageRouter.sortObjectKeys recurses without tracking visited nodes, so circular references cause a stack overflow. it also handles arrays differently from MetadataScanner.sortObjectKeys, causing key mismatches that drop valid messages.

Fix

extract shared sortObjectKeys into pattern-key.ts with weakset cycle detection and consistent array handling. both MessageRouter and MetadataScanner now use the same function.

Test Coverage

• 11 unit tests for sortObjectKeys utility
• 2 circular-reference rejection tests in message-router
• 1 array key-order matching test in metadata-scanner

all 975 tests pass. lint and format clean.

Fixes #188

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0da76da0-72cb-4e5f-9d3e-ecc6a451cce3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 and usage tips.}

[wahajahmed010]

Review: sortObjectKeys extraction + circular reference protection

This is a well-structured refactor that extracts the duplicated sortObjectKeys logic from both MessageRouter and MetadataScanner into a shared pattern-key.ts module. Clean work. A few observations:

Strong points

• Circular reference detection via WeakSet is correctly implemented with proper cleanup (seen.delete(obj)) — the shared-reference test confirms this
• Tests are thorough: sorted keys, nested objects, arrays, nested arrays, primitives, circular (direct + deep), shared references, and non-standard values
• Fixes an existing bug: The old MetadataScanner.sortObjectKeys only recursed into plain objects, skipping arrays entirely. The new sortValue correctly maps over array items and sorts keys inside them — validated by the array test case

Minor: function/undefined behavior

The test expects undefined and function values to survive in the sorted output. JSON.stringify will strip them anyway later, so not a bug — just know these values will vanish on serialization regardless.

Summary

Solid refactor with good defensive programming. The extracted module is clean and well-tested. Approve after confirming CI passes.

[wahajahmed010]

Review: #201

Clean extraction of the shared sortObjectKeys utility. The refactoring eliminates the DRY violation between MessageRouter and MetadataScanner while adding proper circular reference protection. A closer look:

WeakSet cycle detection is well-implemented

Using WeakSet over Set for the seen tracker is the right call — it avoids memory leaks since the WeakSet doesn't prevent GC of the tracked objects. The seen.delete(obj) cleanup at the end of each sortObjectKeys call is also critical: without it, shared references in non-circular branches (like the test with { a: shared, b: shared }) would falsely trigger circular reference errors.

Array handling consistency

Previously, MessageRouter.sortObjectKeys treated arrays differently — it used sortValue for array items (recursing), while MetadataScanner.sortObjectKeys recursed objects inside arrays. The unified sortValue path in pattern-key.ts correctly recurses into both objects and arrays at any nesting level. This is the key behavioral fix beyond the cycle detection.

toJSON consideration

The function sorts Object.keys(obj).sort() and then directly assigns values via sorted[key]. If an object has a custom toJSON() method, the serialized form will differ from JSON.stringify() output. For this codebase's use case (NestJS message patterns with plain objects), this isn't an issue, but worth noting if the utility sees wider use.

Tests are thorough

11 unit tests and 3 integration tests cover the important paths: sorting, circular references, deeply nested cycles, arrays with sorted objects, shared references, and edge cases like undefined/function values. The test for arrays containing objects with different key orders (item { b: 1, a: 2 } matching { a: 2, b: 1 }) is the most valuable — it validates the actual bug from #188.

Minor: error message format

The test assertions use .toEqual which is deep-equal, so the returned objects may still have the original key ordering on the object itself (since JS object key order is guaranteed for string keys in ES2015+). The assertions are on Object.keys(), not object equality, which is correct.

Solid work. No blockers.