Repository navigation
chore: rolling promotion dev -> main #1059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d879422
b43af75
dc211ac
0efa9ea
68fc650
97dd106
9382f29
f3c84c9
0c5f624
79b1420
dbc8af1
b9364e3
26b2fd3
b3b71bd
59aaff0
5136c8d
16415a8
61102bc
c91f2bf
d855e94
0a637e9
42e9d53
fe81d12
84059d4
70e2c93
6fc4376
d092708
aecdf19
415f993
69fafca
ed48772
b04becc
26d33e2
19f0313
e35b73f
d18e697
ca47bdf
fff2625
47f4364
4d3da3e
5b799cc
f9b06ef
5f71cde
942d112
2b226b3
a68b856
3b08d27
94a85eb
c71ef7b
c7cf1b3
4d9a691
4bc9773
e1d2064
7ba308e
7fa673b
f4fe5d2
2c7a946
2bec532
749b98b
e8cc2ae
34270a6
f6e9840
4cf61ed
95af851
72fded8
52d4d3f
f4bb180
05f2112
0f32c7c
314bbe5
fa0cac5
5c6ff8d
a13d6d4
adcbfeb
dbc2246
b87b8e2
df4ef81
8d66453
c455204
a2b626a
c408c2e
fdd8bf9
9c4878e
b7d21c7
057295b
7074ab3
485c366
24daa57
a12ad17
6793e63
cafa880
c6ce9ff
5907abe
7ee4ae3
dcfbc38
5e66cee
beb3555
f11347b
4733d19
4c78ea9
147c693
d68716d
6adf989
8df36b1
603c105
bb60a72
5150760
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,87 @@ | ||||||||||||
| # Wish: Fix Omni Bridge & ClaudeCode Executor Hardening | ||||||||||||
|
|
||||||||||||
| | Field | Value | | ||||||||||||
| |-------|-------| | ||||||||||||
| | **Status** | SHIPPED | | ||||||||||||
| | **Slug** | `fix-omni-bridge-hardening` | | ||||||||||||
| | **Date** | 2026-04-04 | | ||||||||||||
| | **Priority** | P1 | | ||||||||||||
| | **Repo** | `automagik-dev/genie` | | ||||||||||||
| | **Issues** | #1036, #1035 | | ||||||||||||
| | **depends-on** | `fix-sdk-executor-correctness` (P0) | | ||||||||||||
|
|
||||||||||||
| ## Summary | ||||||||||||
|
|
||||||||||||
| The ClaudeCode Omni executor has chat ID collisions, broken JSON escaping, and weak liveness checks. The OmniBridge silently drops messages when buffer is full, leaks executor processes on shutdown, and allows concurrency bursts during spawn. These are production reliability issues for the Omni→agent pipeline. | ||||||||||||
|
|
||||||||||||
| ## Deliverables | ||||||||||||
|
|
||||||||||||
| ### Group 1: Fix ClaudeCode executor — chat ID collision + JSON escaping + liveness (#1036) | ||||||||||||
|
|
||||||||||||
| **File:** `src/services/executors/claude-code.ts` | ||||||||||||
|
|
||||||||||||
| **Bug 1 — Chat ID collision (line 23-25):** | ||||||||||||
| `sanitizeWindowName` strips non-alphanumeric chars and truncates to 40. Different JIDs like `5511999999999@s.whatsapp.net` and `5511888888888@s.whatsapp.net` can collide after stripping. | ||||||||||||
|
|
||||||||||||
| **Fix:** Use a hash-based approach: | ||||||||||||
| ```typescript | ||||||||||||
| function sanitizeWindowName(chatId: string): string { | ||||||||||||
| const hash = createHash('md5').update(chatId).digest('hex').slice(0, 12); | ||||||||||||
| const prefix = chatId.replace(/[^a-zA-Z0-9]/g, '').slice(0, 24); | ||||||||||||
| return `${prefix}-${hash}` || 'chat'; | ||||||||||||
| } | ||||||||||||
| ``` | ||||||||||||
|
|
||||||||||||
| **Bug 2 — JSON escaping (line 210-214):** | ||||||||||||
| Only `sed 's/"/\\"/g'` is used. Backslashes, tabs, carriage returns produce malformed JSON. | ||||||||||||
|
|
||||||||||||
| **Fix:** Use `JSON.stringify()` in the Node layer before passing to bash, or use a proper escaping function that handles all JSON-special characters (`\`, `\n`, `\t`, `\r`, control chars). | ||||||||||||
|
|
||||||||||||
| **Bug 3 — Liveness check:** | ||||||||||||
| `isPaneAlive` at `src/lib/tmux.ts:548-555` only checks `pane_dead` flag, not whether Claude process runs inside. | ||||||||||||
|
|
||||||||||||
| **Fix:** Add process check: `tmux list-panes -t <pane> -F '#{pane_pid}'` then verify the process tree contains `claude` or the expected process. | ||||||||||||
|
|
||||||||||||
| **Acceptance:** Different chat IDs always map to different tmux windows. Messages with special characters produce valid JSON. Liveness detects crashed Claude processes in alive panes. | ||||||||||||
|
|
||||||||||||
| ### Group 2: Fix OmniBridge session lifecycle (#1035) | ||||||||||||
|
|
||||||||||||
| **File:** `src/services/omni-bridge.ts` | ||||||||||||
|
|
||||||||||||
| **Bug 1 — Silent message drop (line 265-268):** | ||||||||||||
| When `entry.buffer.length >= MAX_BUFFER_PER_CHAT` (50), messages silently dropped. | ||||||||||||
|
|
||||||||||||
| **Fix:** Log a warning, emit a runtime event, and reply to the sender with an error message (e.g., "Queue full, please retry"). | ||||||||||||
|
|
||||||||||||
| **Bug 2 — Resource leak on stop() (line 158-193):** | ||||||||||||
| `stop()` clears timers and drains NATS but never calls `executor.shutdown()` on live sessions. | ||||||||||||
|
|
||||||||||||
| **Fix:** Iterate `this.sessions` and call `executor.shutdown()` for each active session before clearing: | ||||||||||||
| ```typescript | ||||||||||||
| for (const [key, entry] of this.sessions) { | ||||||||||||
| try { await entry.executor?.shutdown(); } catch (e) { /* log */ } | ||||||||||||
| } | ||||||||||||
| this.sessions.clear(); | ||||||||||||
| ``` | ||||||||||||
|
Comment on lines
+59
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect property access in suggested fix — Session entries have a Corrected fix for (const [key, entry] of this.sessions) {
- try { await entry.executor?.shutdown(); } catch (e) { /* log */ }
+ if (entry.session) {
+ try { await this.executor?.shutdown(entry.session); } catch (e) { /* log */ }
+ }
}
this.sessions.clear();🤖 Prompt for AI Agents |
||||||||||||
|
|
||||||||||||
| **Bug 3 — Concurrency burst (line 294-295):** | ||||||||||||
| Spawning entries excluded from concurrency count, allowing oversubscription. | ||||||||||||
|
|
||||||||||||
| **Fix:** Count spawning entries in the active count: | ||||||||||||
| ```typescript | ||||||||||||
| const activeCount = Array.from(this.sessions.values()).filter(e => !e.idle).length; | ||||||||||||
| ``` | ||||||||||||
|
Comment on lines
+70
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect property name in suggested fix — The session entry structure uses Corrected fix-const activeCount = Array.from(this.sessions.values()).filter(e => !e.idle).length;
+const activeCount = this.sessions.size; // All entries (active + spawning) count toward concurrency📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
|
|
||||||||||||
| **Bug 4 — Buffered messages lost on spawn failure (line 337-340):** | ||||||||||||
| When spawn fails, `this.sessions.delete(key)` removes placeholder including all buffered messages. | ||||||||||||
|
|
||||||||||||
| **Fix:** Before deleting, attempt to re-queue buffered messages or log them for recovery. | ||||||||||||
|
|
||||||||||||
| **Acceptance:** No silent message drops — all failures logged and visible. `stop()` cleans up all executor processes. Concurrency limit respected during spawn bursts. | ||||||||||||
|
|
||||||||||||
| ## Validation | ||||||||||||
|
|
||||||||||||
| ```bash | ||||||||||||
| cd /home/genie/workspace/repos/genie | ||||||||||||
| bun run build && bun test --filter "omni|bridge|executor|claude-code" | ||||||||||||
| ``` | ||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edge case: empty prefix produces window name starting with hyphen.
If
chatIdcontains only non-alphanumeric characters,prefixbecomes empty and the result is-{hash}. Tmux window names starting with-can cause issues with command-line parsing.Suggested improvement
function sanitizeWindowName(chatId: string): string { const hash = createHash('md5').update(chatId).digest('hex').slice(0, 12); const prefix = chatId.replace(/[^a-zA-Z0-9]/g, '').slice(0, 24); - return `${prefix}-${hash}` || 'chat'; + return prefix ? `${prefix}-${hash}` : `chat-${hash}`; }🤖 Prompt for AI Agents