Repository navigation
fix: bridge session isolation — NATS queues, per-chat agents, dedup, cleanup - #1126
Conversation
…eanup, dedup, orphan-all
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a skipHooks parameter to avoid orchestration side-effects for omni-originated sessions and implements NATS message deduplication within the OmniBridge to handle potential JetStream redeliveries. Additionally, it simplifies the session recovery process by orphaning all active sessions upon startup and adopts NATS queue groups for better message distribution. Feedback was provided regarding the efficiency of the deduplication cache pruning logic, suggesting an early exit from the cleanup loop to take advantage of the insertion order in JavaScript Maps.
| if (this.recentMessageIds.size > 1000) { | ||
| const cutoff = Date.now() - 60_000; | ||
| for (const [id, ts] of this.recentMessageIds) { | ||
| if (ts < cutoff) this.recentMessageIds.delete(id); | ||
| } | ||
| } |
There was a problem hiding this comment.
The dedup cache pruning logic iterates through the entire Map every time a message arrives once the size threshold is exceeded. Since JavaScript Map objects preserve insertion order, and entries are added with the current timestamp, the oldest entries are always at the beginning. You can optimize this by breaking the loop as soon as a non-stale entry is encountered, avoiding unnecessary iterations over the rest of the cache. The use of hardcoded limits here is acceptable as an intentional cap to prevent memory growth.
if (this.recentMessageIds.size > 1000) {
const cutoff = Date.now() - 60_000;
for (const [id, ts] of this.recentMessageIds) {
if (ts < cutoff) {
this.recentMessageIds.delete(id);
} else {
break;
}
}
if (this.recentMessageIds.size > 2000) {
for (const [id] of this.recentMessageIds) {
this.recentMessageIds.delete(id);
if (this.recentMessageIds.size <= 1000) break;
}
}
}References
- It is acceptable to use hardcoded numeric limits (magic numbers) in non-critical fallback logic, especially when they serve as intentional caps to prevent performance issues like excessive I/O.
fix: bridge session isolation — NATS queues, per-chat agents, dedup, cleanup
All six wishes have their deliverables merged to dev via the PRs listed below. Moving them from active tracking into _archive/ keeps the wish directory scoped to in-flight and draft work. | Wish | Shipped via | |-----------------------------|------------------------| | velocity-dashboard | #1142 + #1143 (hotfix) | | fix-ghost-approval-p0 | #1134 | | perfect-spawn-hierarchy | #1133 | | brain-help-passthrough | #1129 | | scaffold-auto-memory | #1128 | | unify-bridge-revamp-skills | #1125 + #1126 + #1127 | No code changes — purely a directory reorganization. Wish artifacts remain readable at .genie/wishes/_archive/<slug>/WISH.md for historical reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Comprehensive fix for genie omni-bridge session isolation defect. Addresses 7 root causes from session mixing trace:
Changes
src/services/omni-bridge.ts:
{ queue: 'genie-bridge' }to all 6 NATS subscriptions (G1)recentMessageIdsdedup cache with 60s TTL + 1000 entry pruning (G4)markAllOrphaned()(G5)||(G3, defense-in-depth)src/services/executors/claude-code.ts:
${agentName}:${chatId}(G2)skipHooks: truein omni spawn params (G6)src/lib/provider-adapters.ts:
skipHooks?: booleantoSpawnParams(G6)if (!params.skipHooks)(G6)Test Plan
Related