v0.5.5 — /update command + /reset queue clear#1
Merged
Conversation
- /update: git pull + npm run build + launchd restart via process.exit(0) - /reset: now also clears pendingTasks and pendingMessages so the queue doesn't keep draining after a kill - GroupQueue.clearQueue() method added
📝 WalkthroughWalkthroughThis PR introduces a remote update capability via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Telegram as Telegram Channel
participant Shell as Shell (execSync)
participant Launchd as launchd
participant Agent as Agent Process
User->>Telegram: /update
Telegram->>User: Pulling latest code...
Telegram->>Shell: git pull
Shell-->>Telegram: code updated
Telegram->>User: Building project...
Telegram->>Shell: npm run build
Shell-->>Telegram: build complete
Telegram->>Launchd: restart service
Launchd->>Agent: terminate
Launchd->>Agent: spawn new process
Agent-->>User: Update complete
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 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 Tip Migrating from UI to YAML configuration.Use the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/channels/telegram.ts (1)
1-447:⚠️ Potential issue | 🟡 MinorCI is currently blocked by Prettier in this file.
Please run formatting on
src/channels/telegram.tsbefore merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/channels/telegram.ts` around lines 1 - 447, The file fails CI due to Prettier formatting; run your project's formatter (e.g. npx prettier --write src/channels/telegram.ts or npm run format) to apply consistent formatting across the TelegramChannel class and its methods (connect, sendMessage, sendDocument, setTyping, etc.), then re-stage and push the formatted file so CI can pass.
🤖 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 68-70: The /reset handler always replies "Agent killed..."
regardless of whether anything was running; change it to inspect the return
value of this.opts.onReset?.(chatJid) (e.g., store it in a variable like const
result = await this.opts.onReset?.(chatJid)) and then send a conditional reply
via ctx.reply: if result indicates an agent was killed, keep "Agent killed and
queue cleared — send me something to start fresh.", otherwise reply with "No
agent was running; queue cleared — send me something to start fresh." Ensure you
await the onReset call if it returns a promise and only call ctx.reply once with
the appropriate message.
- Around line 73-97: The /update handler (this.bot.command('update')) currently
allows any registered chat (using registeredGroups() and chatJid) to
pull/build/restart; restrict it by checking the caller against an
admin/allowlist before performing actions. Add an explicit authorization check
at the top of the handler that verifies the chatJid is in a configured admin set
(e.g., this.opts.adminGroups or a single this.opts.adminGroup/mainGroup value)
or that group has an admin flag (e.g., group.isAdmin); if the check fails reply
with an "unauthorized" message and return. Ensure you reference the same symbols
(chatJid, registeredGroups(), this.opts) so the check integrates with existing
config and log unauthorized attempts via logger before returning.
In `@src/group-queue.ts`:
- Around line 205-219: The clearQueue method currently empties pendingTasks and
removes the group from waitingGroups but does not cancel any scheduled
retry/backoff for that group; update clearQueue (and related state on the group
object returned by getGroup) to cancel and clear any retry timer or backoff
state so retries cannot fire after reset — specifically: if the group state has
a timer id (e.g., state.retryTimer or state.retryTimeoutId) call
clearTimeout/clearInterval and delete/null that property, and if there is a
centralized backoff map (e.g., this.retryBackoffs or this.backoffTimers keyed by
groupJid) clear and delete its entry and reset any backoff counters (e.g.,
state.backoffCount) to 0 so no retry will be scheduled after clearQueue
completes.
---
Outside diff comments:
In `@src/channels/telegram.ts`:
- Around line 1-447: The file fails CI due to Prettier formatting; run your
project's formatter (e.g. npx prettier --write src/channels/telegram.ts or npm
run format) to apply consistent formatting across the TelegramChannel class and
its methods (connect, sendMessage, sendDocument, setTyping, etc.), then re-stage
and push the formatted file so CI can pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 943a136d-af66-4f20-92db-6fb5ecb51264
📒 Files selected for processing (5)
CHANGELOG.mdpackage.jsonsrc/channels/telegram.tssrc/group-queue.tssrc/index.ts
Comment on lines
+68
to
+70
| this.opts.onReset?.(chatJid); | ||
| ctx.reply('Reset. Agent killed and queue cleared — send me something to start fresh.'); | ||
| }); |
There was a problem hiding this comment.
/reset response is misleading when nothing was running.
onReset returns kill status, but the reply always says “Agent killed”. This can misreport state.
💡 Proposed fix
- this.opts.onReset?.(chatJid);
- ctx.reply('Reset. Agent killed and queue cleared — send me something to start fresh.');
+ const killed = this.opts.onReset?.(chatJid) ?? false;
+ ctx.reply(
+ killed
+ ? 'Reset. Agent killed and queue cleared — send me something to start fresh.'
+ : 'Reset. Queue cleared. No running agent found — send me something to start fresh.',
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.opts.onReset?.(chatJid); | |
| ctx.reply('Reset. Agent killed and queue cleared — send me something to start fresh.'); | |
| }); | |
| const killed = this.opts.onReset?.(chatJid) ?? false; | |
| ctx.reply( | |
| killed | |
| ? 'Reset. Agent killed and queue cleared — send me something to start fresh.' | |
| : 'Reset. Queue cleared. No running agent found — send me something to start fresh.', | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/channels/telegram.ts` around lines 68 - 70, The /reset handler always
replies "Agent killed..." regardless of whether anything was running; change it
to inspect the return value of this.opts.onReset?.(chatJid) (e.g., store it in a
variable like const result = await this.opts.onReset?.(chatJid)) and then send a
conditional reply via ctx.reply: if result indicates an agent was killed, keep
"Agent killed and queue cleared — send me something to start fresh.", otherwise
reply with "No agent was running; queue cleared — send me something to start
fresh." Ensure you await the onReset call if it returns a promise and only call
ctx.reply once with the appropriate message.
Comment on lines
+73
to
97
| this.bot.command('update', async (ctx) => { | ||
| const chatJid = `tg:${ctx.chat.id}`; | ||
| const group = this.opts.registeredGroups()[chatJid]; | ||
| if (!group) { | ||
| ctx.reply('Not a registered chat.'); | ||
| return; | ||
| } | ||
|
|
||
| await ctx.reply('Pulling latest code...'); | ||
| const cwd = process.cwd(); | ||
|
|
||
| try { | ||
| const pullOut = execSync('git pull', { cwd, encoding: 'utf-8' }); | ||
| await ctx.reply(`git pull: ${pullOut.trim()}`); | ||
|
|
||
| await ctx.reply('Building...'); | ||
| execSync('npm run build', { cwd, encoding: 'utf-8', timeout: 120_000 }); | ||
|
|
||
| await ctx.reply('Done. Restarting — back in a moment.'); | ||
| setTimeout(() => process.exit(0), 500); | ||
| } catch (err) { | ||
| const msg = err instanceof Error ? err.message : String(err); | ||
| await ctx.reply(`Update failed:\n${msg.slice(0, 500)}`); | ||
| logger.error({ err }, '/update command failed'); | ||
| } |
There was a problem hiding this comment.
/update needs stricter authorization.
This command can execute repo update/build/restart, but it’s currently available to any registered chat. That’s a high-impact control path and should be restricted to an admin channel/allowlist.
🔐 Proposed fix (restrict to main/admin group)
-import { ASSISTANT_NAME, TRIGGER_PATTERN } from '../config.js';
+import { ASSISTANT_NAME, MAIN_GROUP_FOLDER, TRIGGER_PATTERN } from '../config.js';
this.bot.command('update', async (ctx) => {
const chatJid = `tg:${ctx.chat.id}`;
const group = this.opts.registeredGroups()[chatJid];
if (!group) {
ctx.reply('Not a registered chat.');
return;
}
+ if (group.folder !== MAIN_GROUP_FOLDER) {
+ await ctx.reply('Not authorized to run /update from this chat.');
+ return;
+ }
await ctx.reply('Pulling latest code...');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/channels/telegram.ts` around lines 73 - 97, The /update handler
(this.bot.command('update')) currently allows any registered chat (using
registeredGroups() and chatJid) to pull/build/restart; restrict it by checking
the caller against an admin/allowlist before performing actions. Add an explicit
authorization check at the top of the handler that verifies the chatJid is in a
configured admin set (e.g., this.opts.adminGroups or a single
this.opts.adminGroup/mainGroup value) or that group has an admin flag (e.g.,
group.isAdmin); if the check fails reply with an "unauthorized" message and
return. Ensure you reference the same symbols (chatJid, registeredGroups(),
this.opts) so the check integrates with existing config and log unauthorized
attempts via logger before returning.
Comment on lines
+205
to
+219
| /** | ||
| * Clear all pending tasks and messages for a group. | ||
| * Call alongside killAgent() on /reset so the queue doesn't keep draining. | ||
| */ | ||
| clearQueue(groupJid: string): void { | ||
| const state = this.getGroup(groupJid); | ||
| const taskCount = state.pendingTasks.length; | ||
| state.pendingTasks = []; | ||
| state.pendingMessages = false; | ||
| // Remove from waiting list too | ||
| this.waitingGroups = this.waitingGroups.filter((jid) => jid !== groupJid); | ||
| if (taskCount > 0) { | ||
| logger.info({ groupJid, taskCount }, 'Queue cleared on reset'); | ||
| } | ||
| } |
There was a problem hiding this comment.
clearQueue() should also cancel retry backoff state.
A previously scheduled retry can still fire after reset and restart processing for the same group. That makes /reset less deterministic as a hard stop.
🛠️ Proposed fix
interface GroupState {
active: boolean;
idleWaiting: boolean;
isTaskContainer: boolean;
pendingMessages: boolean;
pendingTasks: QueuedTask[];
process: ChildProcess | null;
containerName: string | null;
groupFolder: string | null;
retryCount: number;
+ retryTimer: NodeJS.Timeout | null;
}
// in getGroup() initializer
state = {
active: false,
idleWaiting: false,
isTaskContainer: false,
pendingMessages: false,
pendingTasks: [],
process: null,
containerName: null,
groupFolder: null,
retryCount: 0,
+ retryTimer: null,
};
clearQueue(groupJid: string): void {
const state = this.getGroup(groupJid);
const taskCount = state.pendingTasks.length;
state.pendingTasks = [];
state.pendingMessages = false;
+ state.retryCount = 0;
+ if (state.retryTimer) {
+ clearTimeout(state.retryTimer);
+ state.retryTimer = null;
+ }
this.waitingGroups = this.waitingGroups.filter((jid) => jid !== groupJid);
if (taskCount > 0) {
logger.info({ groupJid, taskCount }, 'Queue cleared on reset');
}
}
private scheduleRetry(groupJid: string, state: GroupState): void {
state.retryCount++;
if (state.retryCount > MAX_RETRIES) {
...
return;
}
const delayMs = BASE_RETRY_MS * Math.pow(2, state.retryCount - 1);
+ if (state.retryTimer) clearTimeout(state.retryTimer);
- setTimeout(() => {
+ state.retryTimer = setTimeout(() => {
+ state.retryTimer = null;
if (!this.shuttingDown) {
this.enqueueMessageCheck(groupJid);
}
}, delayMs);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/group-queue.ts` around lines 205 - 219, The clearQueue method currently
empties pendingTasks and removes the group from waitingGroups but does not
cancel any scheduled retry/backoff for that group; update clearQueue (and
related state on the group object returned by getGroup) to cancel and clear any
retry timer or backoff state so retries cannot fire after reset — specifically:
if the group state has a timer id (e.g., state.retryTimer or
state.retryTimeoutId) call clearTimeout/clearInterval and delete/null that
property, and if there is a centralized backoff map (e.g., this.retryBackoffs or
this.backoffTimers keyed by groupJid) clear and delete its entry and reset any
backoff counters (e.g., state.backoffCount) to 0 so no retry will be scheduled
after clearQueue completes.
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.
What
Two small but important fixes for remote control when you can't SSH in.
/resetnow actually resetsPreviously
/resetkilled the current agent but leftpendingTasksandpendingMessagesin the queue.drainGroup()would immediately start the next item, making it impossible to stop a runaway queue via Telegram. Now/resetcallsclearQueue()first, so it's a genuine hard stop./update— remote updates over TelegramPulls latest code, rebuilds, and restarts via launchd — no SSH needed:
Bootstrap note: requires one manual
git pull && npm run build && launchctl kickstart -k gui/501/com.ghostclawto get onto a running instance. After that, all future updates can be sent via/updatefrom Telegram.Files changed
src/group-queue.ts— addedclearQueue()methodsrc/index.ts—onResetnow callsclearQueue()beforekillAgent()src/channels/telegram.ts— added/updatecommand, updated/resetreplypackage.json— bumped to v0.5.5CHANGELOG.md— v0.5.5 entryTest plan
/resetwhile agent is running — confirm queue doesn't drain/resetwith no agent running — confirm "Nothing running" behaviour (note: now always confirms kill+clear regardless)/updateon a repo with no new commits — confirm "Already up to date" message/updatewith new commits — confirm pull, build, restart cycle/pingafter restart — confirm bot comes back upSummary by CodeRabbit
New Features
/updatecommand to deploy code updates remotely via Telegram—automatically fetches latest code, rebuilds, and restarts the service.Bug Fixes
/resetcommand to fully clear pending tasks and message queues, ensuring complete system reset alongside stopping the agent.