v0.6.0 — reliability + remote control#2
Merged
Merged
Conversation
- Fix infinite retry loop: scheduleRetry no longer resets retryCount after MAX_RETRIES - Orphan PID cleanup on startup: track agent PIDs in data/agent-pids.json, kill survivors on boot - /status command: active agents, queue depth, uptime via Telegram - /skills command: lists installed skills with descriptions from .claude/skills/ - setMyCommands() at startup: Telegram command menu auto-populated - GroupQueue.getStatus(): exposes live queue state for external consumers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds v0.6.0: Telegram /status and /skills commands plus command registration; exposes queue internals via GroupQueue.getStatus(); fixes infinite-retry reset in scheduleRetry; tracks agent PIDs in Changes
Sequence Diagram(s)sequenceDiagram
participant Startup
participant PIDStore as AgentPIDStore
participant OS
participant Logger
Startup->>PIDStore: readAgentPids()
PIDStore-->>Startup: list of PIDs
loop for each pid
Startup->>OS: process.kill(pid, 0) (liveness check)
alt alive
Startup->>OS: process.kill(pid, 'SIGKILL')
OS-->>Logger: killed pid
else not alive
OS-->>Logger: pid not running
end
end
Startup->>PIDStore: writeAgentPids([]) (clear persisted list)
PIDStore-->>Logger: persisted list cleared
sequenceDiagram
participant User
participant TelegramBot as Bot
participant Queue as GroupQueue
participant Index as Server
User->>TelegramBot: /status
TelegramBot->>Index: invoke onGetStatus()
Index->>Queue: getStatus()
Queue-->>Index: status object
Index-->>TelegramBot: formatted status (HTML)
TelegramBot-->>User: reply with status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/channels/telegram.ts`:
- Around line 133-136: The /skills handler is inserting unescaped values into an
HTML message: escape both the directory name variable dir and the extracted
description desc using the exported escapeHtml() function (from src/router.ts
via src/index.ts) before interpolating them into lines; replace the raw dir and
desc usage in the block that builds lines (where content is read with
fs.readFileSync and desc is derived from descMatch) with escapeHtml(dir) and
escapeHtml(desc.slice(0,80)) (or empty string when desc is empty) so the message
sent with parse_mode: 'HTML' cannot be broken or injected with raw HTML
characters.
- Around line 114-115: The /skills command handler (this.bot.command('skills',
...)) must be made async and every call to ctx.reply() inside the skills handler
(including the chunking loop that sends parts of the skills message) must be
awaited; change the handler to async (e.g., this.bot.command('skills', async
(ctx) => { ... })) and prepend await to each ctx.reply(...) so messages are sent
sequentially and send failures surface properly—mirror the pattern used in the
/update handler.
In `@src/index.ts`:
- Around line 492-519: readAgentPids currently trusts JSON.parse and may return
unsafe values; change it to parse into unknown, ensure the result is an array of
positive integers (reject 0/negatives/non-integers), dedupe and return only
valid PIDs; update writeAgentPids to sanitize the pids array before persisting
(use the same validation). In cleanupOrphanedAgents (and related logic around
lines referenced) verify each PID still belongs to a GhostClaw agent before
sending SIGKILL by checking the process command/argv (e.g., /proc/<pid>/cmdline
on POSIX or platform-appropriate process listing), skip invalid or non-matching
PIDs, and handle all errors without throwing; ensure
trackAgentPid/untrackAgentPid use the validated read/write helpers so only safe
PIDs are stored and acted upon.
- Around line 311-317: The scheduler's onProcess callback passed into
runContainerAgent/task-scheduler.ts is missing the PID tracking wrapper present
in the interactive agent callback; update the scheduler-side callback (the
function that currently calls queue.registerProcess(chatJid, proc,
containerName, group.folder)) to mirror the interactive version by calling
trackAgentPid(proc.pid) when proc.pid is present and registering a
proc.once('exit', () => untrackAgentPid(proc.pid!)) handler; ensure this change
references the same queue.registerProcess(...) call and integrates with
runContainerAgent's onProcess hook so scheduled agents are tracked and untracked
the same way.
- Around line 614-620: The group label interpolation uses unescaped values
(registeredGroups, group?.name and g.jid) when building the status lines and
sending HTML to Telegram; wrap both group?.name and g.jid with the existing
escapeHtml(...) helper from router.js before composing name and before calling
lines.push (i.e., compute name = escapeHtml(group?.name || g.jid) or escape both
individually) so any '<', '>' or '&' are encoded prior to lines.push and
subsequent send with parse_mode: 'HTML'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3499f306-e92b-40a1-8807-c226b3075824
📒 Files selected for processing (5)
CHANGELOG.mdpackage.jsonsrc/channels/telegram.tssrc/group-queue.tssrc/index.ts
Comment on lines
+492
to
+519
| function readAgentPids(): number[] { | ||
| try { | ||
| return JSON.parse(fs.readFileSync(agentPidsFile, 'utf-8')); | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| function writeAgentPids(pids: number[]): void { | ||
| try { | ||
| fs.mkdirSync(DATA_DIR, { recursive: true }); | ||
| fs.writeFileSync(agentPidsFile, JSON.stringify(pids)); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| } | ||
|
|
||
| function trackAgentPid(pid: number): void { | ||
| const pids = readAgentPids(); | ||
| if (!pids.includes(pid)) { | ||
| pids.push(pid); | ||
| writeAgentPids(pids); | ||
| } | ||
| } | ||
|
|
||
| function untrackAgentPid(pid: number): void { | ||
| const pids = readAgentPids().filter((p) => p !== pid); | ||
| writeAgentPids(pids); |
There was a problem hiding this comment.
Validate and identify persisted agent PIDs before cleanup.
readAgentPids() trusts raw JSON.parse() output, and cleanupOrphanedAgents() later iterates and SIGKILLs whatever numbers come back. A valid-but-wrong payload can crash startup, 0/negative values have process-group semantics on POSIX, and a reused PID can target an unrelated host process. Parse into unknown, accept only positive integers, and verify the record still belongs to a GhostClaw agent before killing it.
Based on learnings: "Run agents directly on the host machine as Node.js child processes without containerization".
Also applies to: 522-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 492 - 519, readAgentPids currently trusts
JSON.parse and may return unsafe values; change it to parse into unknown, ensure
the result is an array of positive integers (reject 0/negatives/non-integers),
dedupe and return only valid PIDs; update writeAgentPids to sanitize the pids
array before persisting (use the same validation). In cleanupOrphanedAgents (and
related logic around lines referenced) verify each PID still belongs to a
GhostClaw agent before sending SIGKILL by checking the process command/argv
(e.g., /proc/<pid>/cmdline on POSIX or platform-appropriate process listing),
skip invalid or non-matching PIDs, and handle all errors without throwing;
ensure trackAgentPid/untrackAgentPid use the validated read/write helpers so
only safe PIDs are stored and acted upon.
Comment on lines
+614
to
+620
| const group = registeredGroups[g.jid]; | ||
| const name = group?.name || g.jid; | ||
| const parts: string[] = []; | ||
| if (g.active) parts.push('running'); | ||
| if (g.queuedTasks > 0) parts.push(`${g.queuedTasks} task(s) queued`); | ||
| if (g.queuedMessages) parts.push('messages queued'); | ||
| lines.push(`• ${name}: ${parts.join(', ')}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and get context around lines 614-620
wc -l src/index.tsRepository: b1rdmania/ghostclaw
Length of output: 79
🏁 Script executed:
# Read the relevant section with surrounding context
sed -n '600,650p' src/index.tsRepository: b1rdmania/ghostclaw
Length of output: 1615
🏁 Script executed:
# Search for where the 'lines' variable is used and how the status message is constructed
rg -n "lines\." src/index.ts | head -20Repository: b1rdmania/ghostclaw
Length of output: 185
🏁 Script executed:
# Look for parse_mode usage to understand if HTML mode is being used
rg -n "parse_mode" src/index.tsRepository: b1rdmania/ghostclaw
Length of output: 45
🏁 Script executed:
# Search for any HTML escaping utilities in the codebase
rg -n "escapeHtml|htmlEscape|escapeXml" src/Repository: b1rdmania/ghostclaw
Length of output: 1323
🏁 Script executed:
# Let's get more context to identify the function containing this code
sed -n '575,625p' src/index.tsRepository: b1rdmania/ghostclaw
Length of output: 1642
🏁 Script executed:
# Search for where this function result is used and what parse_mode is set
rg -n "sendMessage\|parse_mode" src/index.ts | head -20Repository: b1rdmania/ghostclaw
Length of output: 45
🏁 Script executed:
# Look for TelegramChannel class definition to understand parse_mode handling
rg -n "class TelegramChannel\|parse_mode.*HTML" src/Repository: b1rdmania/ghostclaw
Length of output: 45
🏁 Script executed:
# Search for onGetStatus usage/calls
rg -n "onGetStatus" src/Repository: b1rdmania/ghostclaw
Length of output: 262
🏁 Script executed:
# Find TelegramChannel class and implementation
rg -n "class TelegramChannel" src/Repository: b1rdmania/ghostclaw
Length of output: 140
🏁 Script executed:
# Search for sendMessage implementation
rg -n "sendMessage\(" src/ -A 2Repository: b1rdmania/ghostclaw
Length of output: 8602
🏁 Script executed:
# Let's see the telegram.ts sendMessage implementation more clearly
sed -n '392,430p' src/channels/telegram.tsRepository: b1rdmania/ghostclaw
Length of output: 1072
🏁 Script executed:
# And check where onGetStatus is called in telegram.ts
sed -n '100,130p' src/channels/telegram.tsRepository: b1rdmania/ghostclaw
Length of output: 1102
🏁 Script executed:
# Verify escapeHtml function in router.ts
sed -n '93,102p' src/router.tsRepository: b1rdmania/ghostclaw
Length of output: 364
🏁 Script executed:
# Check the markdownToTelegramHtml function to understand how it handles escaping
rg -n "markdownToTelegramHtml" src/ -A 10Repository: b1rdmania/ghostclaw
Length of output: 7576
🏁 Script executed:
# Let's also verify the exact path where onGetStatus is called in telegram.ts
sed -n '105,115p' src/channels/telegram.tsRepository: b1rdmania/ghostclaw
Length of output: 420
🏁 Script executed:
# Let's verify there's no escaping happening at the ctx.reply level
sed -n '1,50p' src/channels/telegram.ts | grep -i "import\|from"Repository: b1rdmania/ghostclaw
Length of output: 553
🏁 Script executed:
# Check if ctx.reply is from grammyjs and if it does any escaping
rg -n "ctx.reply" src/channels/telegram.ts | head -5Repository: b1rdmania/ghostclaw
Length of output: 250
Escape dynamic group labels in the HTML status message body.
The group.name and g.jid values interpolated at lines 614–620 are sent directly into a Telegram message with parse_mode: 'HTML' (line 111 of telegram.ts). A group title or JID containing <, >, or & will be parsed as HTML, allowing an attacker to inject formatting or break the message layout. Use the existing escapeHtml() function (available from router.js) to escape these values before interpolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 614 - 620, The group label interpolation uses
unescaped values (registeredGroups, group?.name and g.jid) when building the
status lines and sending HTML to Telegram; wrap both group?.name and g.jid with
the existing escapeHtml(...) helper from router.js before composing name and
before calling lines.push (i.e., compute name = escapeHtml(group?.name || g.jid)
or escape both individually) so any '<', '>' or '&' are encoded prior to
lines.push and subsequent send with parse_mode: 'HTML'.
- Make /skills handler async; await all ctx.reply() calls to preserve chunk order - Escape skill dir name and description with escapeXml() before HTML interpolation - Escape group name/JID in /status output with escapeXml() - Mirror PID tracking on scheduler onProcess hook (scheduled tasks were not tracked) - Validate agent-pids.json before killing: accept only positive integers, ignore malformed entries Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-commit hook was reformatting but not re-staging, causing committed versions to diverge from what prettier --check expects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…age formatted files - telegram.test.ts: add setMyCommands vi.fn() to the mock bot api so connect() doesn't throw - .husky/pre-commit: add 'git add -u' after format:fix so prettier-reformatted files are included in the commit rather than left as unstaged changes (was causing CI format checks to fail) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/index.ts (3)
611-618:⚠️ Potential issue | 🟠 MajorEscape group labels before formatting the HTML status body.
group?.name/g.jidare inserted directly into a Telegram HTML reply. A group title containing<,>or&can break the message or inject markup.Suggested fix
-import { findChannel, formatMessages, formatOutbound } from './router.js'; +import { escapeXml, findChannel, formatMessages, formatOutbound } from './router.js'; @@ - const name = group?.name || g.jid; + const name = escapeXml(group?.name || g.jid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 611 - 618, The code inserts group?.name or g.jid directly into Telegram HTML-formatted replies (in the loop over status.groups and the lines.push call), which can break/allow markup; locate the loop using status.groups, registeredGroups and the lines.push(`• ${name}: ...`) and sanitize the label by escaping HTML entities (&, <, >, " and ') before interpolation (create or use an escapeHtml helper and call it on group?.name and g.jid), then use the escaped value in the lines.push string.
311-317:⚠️ Potential issue | 🟠 MajorMirror this PID wrapper in the scheduler path too.
Only interactive agents are tracked here. The later
startSchedulerLoop(... onProcess ...)callback still just forwardsqueue.registerProcess(...), so scheduled agents never reachagent-pids.jsonand won't be reaped on restart.Suggested fix
- onProcess: (groupJid, proc, containerName, groupFolder) => - queue.registerProcess(groupJid, proc, containerName, groupFolder), + onProcess: (groupJid, proc, containerName, groupFolder) => { + queue.registerProcess(groupJid, proc, containerName, groupFolder); + if (proc.pid) { + trackAgentPid(proc.pid); + proc.once('exit', () => untrackAgentPid(proc.pid!)); + } + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 311 - 317, The scheduler path for starting processes (the callback you pass into startSchedulerLoop / the onProcess handler) currently only calls queue.registerProcess(...) and therefore never invokes trackAgentPid/untrackAgentPid, so scheduled agents aren't recorded in agent-pids.json; update the scheduler's onProcess callback to mirror the interactive-agent wrapper: after calling queue.registerProcess(chatJid?, proc, containerName, group.folder) (use the same argument order and identifiers used in this diff), if proc.pid is present call trackAgentPid(proc.pid) and attach proc.once('exit', () => untrackAgentPid(proc.pid!)); ensure the same registerProcess invocation remains but add the pid tracking/untracking logic so scheduled agents are tracked and reaped on restart.
492-541:⚠️ Potential issue | 🔴 CriticalValidate persisted PIDs before issuing
SIGKILL.
readAgentPids()trusts arbitrary JSON asnumber[], so a valid-but-wrong payload can either crash startup ({}/"123") or signal the wrong target (0, negative values, or a reused PID). Sanitize to positive integers on read/write, verify the live process still belongs to a GhostClaw agent before killing it, and only drop entries you actually proved dead. Based on learnings: "Run agents directly on the host machine as Node.js child processes without containerization".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 492 - 541, Sanitize and validate persisted PIDs and only kill confirmed agent processes: update readAgentPids to parse JSON defensively and return a deduplicated array of positive integers (filter out non-numbers, <=0, NaN, strings, objects), update writeAgentPids/trackAgentPid/untrackAgentPid to persist only the sanitized, deduped PIDs; in cleanupOrphanedAgents, skip invalid or current process PID, and before sending SIGKILL verify the PID belongs to a GhostClaw agent by checking a platform-specific marker (e.g. read /proc/<pid>/cmdline on Linux for a known AGENT_EXECUTABLE_NAME constant or fall back to verifying parent PID matches this process when /proc is unavailable), only remove entries that you proved dead and write back any remaining live PIDs.src/channels/telegram.ts (2)
116-159:⚠️ Potential issue | 🟡 MinorMake
/skillsasync and await eachctx.reply().The chunking path fires multiple replies concurrently right now, so long skill lists can arrive out of order and send failures are silently dropped.
Suggested fix
- this.bot.command('skills', (ctx) => { + this.bot.command('skills', async (ctx) => { const chatJid = `tg:${ctx.chat.id}`; const group = this.opts.registeredGroups()[chatJid]; if (!group) { - ctx.reply('Not a registered chat.'); + await ctx.reply('Not a registered chat.'); return; } const skillsDir = path.join(process.cwd(), '.claude', 'skills'); if (!fs.existsSync(skillsDir)) { - ctx.reply('No skills directory found.'); + await ctx.reply('No skills directory found.'); return; } @@ const MAX = 4096; if (text.length <= MAX) { - ctx.reply(text, { parse_mode: 'HTML' }); + await ctx.reply(text, { parse_mode: 'HTML' }); } else { let chunk = ''; for (const line of lines) { if (chunk.length + line.length + 1 > MAX) { - ctx.reply(chunk, { parse_mode: 'HTML' }); + await ctx.reply(chunk, { parse_mode: 'HTML' }); chunk = line; } else { chunk = chunk ? `${chunk}\n${line}` : line; } } - if (chunk) ctx.reply(chunk, { parse_mode: 'HTML' }); + if (chunk) await ctx.reply(chunk, { parse_mode: 'HTML' }); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/telegram.ts` around lines 116 - 159, Change the skills command handler to be async and await all ctx.reply() calls so replies are sent sequentially and failures surface: update the this.bot.command('skills', (ctx) => { ... }) handler to async (ctx) => { ... } and replace each ctx.reply(...) (the early exit replies, the single whole-text reply, and every reply inside the chunking loop) with await ctx.reply(...); ensure you await the reply before returning from the handler and await each chunk reply inside the for loop so messages preserve order and errors propagate.
128-140:⚠️ Potential issue | 🟠 MajorEscape skill metadata before building the HTML response.
diranddescare interpolated into aparse_mode: 'HTML'message verbatim. A skill name or description containing<,>or&will break formatting or inject markup into the reply.Suggested fix
-import { markdownToTelegramHtml } from '../router.js'; +import { escapeXml, markdownToTelegramHtml } from '../router.js'; @@ - const desc = descMatch ? descMatch[1].trim() : ''; + const skillName = escapeXml(dir); + const desc = descMatch + ? escapeXml(descMatch[1].trim().slice(0, 80)) + : ''; lines.push( - `• <code>/${dir}</code>${desc ? ` — ${desc.slice(0, 80)}` : ''}`, + `• <code>/${skillName}</code>${desc ? ` — ${desc}` : ''}`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/telegram.ts` around lines 128 - 140, The HTML response is built using unescaped values (dir and desc) which can contain <, >, & and break or inject markup; update the construction inside the loop (where lines, dirs, dir, desc, skillsDir and skillMd are handled) to HTML-escape both the skill name and the description before interpolation into the `<code>…</code>` and surrounding text (escape &, <, >, " and '), then truncate the escaped description to 80 characters if needed and use the escaped strings in the lines.push call so parse_mode: 'HTML' is safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/channels/telegram.ts`:
- Around line 360-373: The call to bot.api.setMyCommands in connect() can throw
synchronously because the test Bot mock lacks an api.setMyCommands
implementation; either add a mock implementation (e.g., setMyCommands:
vi.fn().mockResolvedValue(undefined) on the test's api mock in telegram.test.ts)
or make connect() defensive by wrapping the await
this.bot.api.setMyCommands(...) call (or the whole setMyCommands chain) in
try/catch and logging the error non-fatally (same pattern as the existing
.catch) so a synchronous TypeError won't break connect() before bot.start()
runs.
---
Duplicate comments:
In `@src/channels/telegram.ts`:
- Around line 116-159: Change the skills command handler to be async and await
all ctx.reply() calls so replies are sent sequentially and failures surface:
update the this.bot.command('skills', (ctx) => { ... }) handler to async (ctx)
=> { ... } and replace each ctx.reply(...) (the early exit replies, the single
whole-text reply, and every reply inside the chunking loop) with await
ctx.reply(...); ensure you await the reply before returning from the handler and
await each chunk reply inside the for loop so messages preserve order and errors
propagate.
- Around line 128-140: The HTML response is built using unescaped values (dir
and desc) which can contain <, >, & and break or inject markup; update the
construction inside the loop (where lines, dirs, dir, desc, skillsDir and
skillMd are handled) to HTML-escape both the skill name and the description
before interpolation into the `<code>…</code>` and surrounding text (escape &,
<, >, " and '), then truncate the escaped description to 80 characters if needed
and use the escaped strings in the lines.push call so parse_mode: 'HTML' is
safe.
In `@src/index.ts`:
- Around line 611-618: The code inserts group?.name or g.jid directly into
Telegram HTML-formatted replies (in the loop over status.groups and the
lines.push call), which can break/allow markup; locate the loop using
status.groups, registeredGroups and the lines.push(`• ${name}: ...`) and
sanitize the label by escaping HTML entities (&, <, >, " and ') before
interpolation (create or use an escapeHtml helper and call it on group?.name and
g.jid), then use the escaped value in the lines.push string.
- Around line 311-317: The scheduler path for starting processes (the callback
you pass into startSchedulerLoop / the onProcess handler) currently only calls
queue.registerProcess(...) and therefore never invokes
trackAgentPid/untrackAgentPid, so scheduled agents aren't recorded in
agent-pids.json; update the scheduler's onProcess callback to mirror the
interactive-agent wrapper: after calling queue.registerProcess(chatJid?, proc,
containerName, group.folder) (use the same argument order and identifiers used
in this diff), if proc.pid is present call trackAgentPid(proc.pid) and attach
proc.once('exit', () => untrackAgentPid(proc.pid!)); ensure the same
registerProcess invocation remains but add the pid tracking/untracking logic so
scheduled agents are tracked and reaped on restart.
- Around line 492-541: Sanitize and validate persisted PIDs and only kill
confirmed agent processes: update readAgentPids to parse JSON defensively and
return a deduplicated array of positive integers (filter out non-numbers, <=0,
NaN, strings, objects), update writeAgentPids/trackAgentPid/untrackAgentPid to
persist only the sanitized, deduped PIDs; in cleanupOrphanedAgents, skip invalid
or current process PID, and before sending SIGKILL verify the PID belongs to a
GhostClaw agent by checking a platform-specific marker (e.g. read
/proc/<pid>/cmdline on Linux for a known AGENT_EXECUTABLE_NAME constant or fall
back to verifying parent PID matches this process when /proc is unavailable),
only remove entries that you proved dead and write back any remaining live PIDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4799702-8979-4c2d-a9a7-f876f2d26292
📒 Files selected for processing (3)
src/channels/telegram.tssrc/group-queue.tssrc/index.ts
- container-runner.ts: replace dynamic require('child_process') with static
import so vi.mock() intercepts execSync in tests
- container-runner.test.ts: add execSync mock to child_process stub
- telegram.test.ts: add setMyCommands to mock bot api (already in previous commit)
All 32 test files, 448 tests now pass locally.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scheduleRetryno longer resetsretryCount = 0afterMAX_RETRIES. Previously a group that hit max retries would silently loop forever.data/agent-pids.json. On boot, any survivors from the previous run are killed before accepting messages. Fixes the Mar 18 cascade (zombie process from 12:50AM consuming slots)./statuscommand — shows active agents per group, queue depth, and uptime. Available over Telegram./skillscommand — lists all installed skills with descriptions, read live from.claude/skills/. Auto-chunks if over Telegram's 4096 char limit.setMyCommands()called at startup so all commands appear with descriptions when the user types/.GroupQueue.getStatus()— exposes live queue state for external consumers (active count, waiting groups, per-group task/message queues).Test plan
/statusin Telegram — confirm uptime, active count, group names show correctly/skillsin Telegram — confirm skill list appears with descriptions/in Telegram — confirm command menu shows all 6 commands with descriptionsMAX_RETRIESfailures — confirm retry loop stops and doesn't restart🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores