Cached keypairs

[rmgpinto]

ref https://linear.app/ghost/issue/PROD-2541

• importJwk and JSON.parse are CPU intensive operations and the workload is using them on every request. This change caches these operations and avoids unnecessary CPU usage.

[coderabbitai]

Walkthrough

• Added a module-level in-memory cache keypairCache as Map<number, CryptoKeyPair>.
• keypairDispatcher now checks keypairCache for account.id and returns the cached keypair on hit (logs a cache hit).
• If absent, it builds the keypair as before, stores it in keypairCache, and returns it.
• Minor formatting-only adjustments to avatar/banner URL handling in actorDispatcher (no behavioral change).
• Existing error handling and public APIs/method signatures remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title succinctly describes the main change of adding caching for keypairs by directly referencing the cache operation without unrelated terms, making it clear to reviewers what the pull request aims to achieve.
Description Check	✅ Passed	The description clearly references the related issue and explains that CPU-intensive operations such as importJwk and JSON.parse are now cached to reduce unnecessary load, which directly aligns with the code changes in the cache-keypair module.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cache-keypair

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a05edfe and e3ecd96.

📒 Files selected for processing (1)

• src/dispatchers.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/dispatchers.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build / Build Docker Images (linux/amd64, amd64)

---

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.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/dispatchers.ts (1)

117-128: Cache population logic is correct, but consider the architectural context.

The keypair construction and caching logic is implemented correctly. However, note that this adds module-level state to a file marked as legacy that should be refactored into class-based handlers (see TODO comment at top).

As per the coding guidelines: "Do not add new legacy factory handlers to dispatchers.ts (legacy file)". While this doesn't add a new handler, it adds persistent state to a file that should be migrated to a class-based architecture. When refactoring this file (as noted in the TODO), ensure the caching mechanism is incorporated into the class-based design with proper lifecycle management.

Minor optimization note: Concurrent requests for the same account.id will race to populate the cache, performing duplicate importJwk work. While not a correctness issue, a per-key lock or once-per-key mechanism could prevent this. However, this is a low-priority optimization.

Based on coding guidelines.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fc52db and aa3adf4.

📒 Files selected for processing (1)

• src/dispatchers.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.ts: Never use direct string comparisons for ActivityPub IDs; use SHA256 hash-based lookups instead
Use Result helper functions (isError, getError, getValue) and avoid destructuring Result values directly
Awilix uses CLASSIC injection: constructor parameter names must match their registration names
Define routes using decorators rather than direct registration
Prefer immutable entities that emit domain events over mutable entities with dirty flags
Use structured error objects with context in Result types instead of string literals
Prefer class-based handlers with dependency injection over function factories

Files:

• src/dispatchers.ts

src/dispatchers.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not add new legacy factory handlers to dispatchers.ts (legacy file)

Files:

• src/dispatchers.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build / Build Docker Images (linux/amd64, amd64)

🔇 Additional comments (1)

src/dispatchers.ts (1)

111-114: Cache check logic is correct.

The early return optimization correctly reduces redundant importJwk calls. However, its effectiveness depends on addressing the cache invalidation and memory concerns flagged above.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Add cache invalidation and size limits.

The module-level cache has two critical concerns:

1. No invalidation mechanism: If account keypairs are rotated or updated in the database, the cached versions will be used indefinitely, potentially causing authentication failures or security issues.
2. Unbounded growth: The cache will accumulate entries for every account that makes a request, leading to a memory leak in long-running processes.

Consider implementing:

• A cache invalidation strategy triggered by account updates/key rotations
• An LRU cache with a maximum size, or TTL-based expiration
• A more sophisticated caching solution like node-cache or lru-cache

-const keypairCache = new Map<number, CryptoKeyPair>(); +// TODO: Implement proper cache with invalidation and size limits +// Consider using an LRU cache library or adding TTL-based expiration +const keypairCache = new Map<number, CryptoKeyPair>();

Would you like me to generate an implementation using an LRU cache with configurable size limits?

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In src/dispatchers.ts around line 46 the module-level Map lacks invalidation and size limits; replace it with a bounded, TTL-capable LRU cache (or integrate lru-cache or node-cache) configured by an env/const maxSize and optional ttl, use the account ID as key and store CryptoKeyPair values; add API functions to delete a single entry and to clear the cache for account-update/key-rotation flows, and call those invalidation functions from any code path that updates or rotates keys (or subscribe to account-update events) so stale keypairs are removed; ensure cache operations are thread-safe and avoid storing extra sensitive metadata.