Add AgentWatch tool handling for Codex long-running jobs#1088
Add AgentWatch tool handling for Codex long-running jobs#1088AliAlharmoodi wants to merge 2 commits intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL 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)
📝 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 You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
github-actions
bot
added
size:L
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }); | ||
| } | ||
|
|
||
| if (request.method === "item/tool/call") { |
There was a problem hiding this comment.
🔴 Critical src/codexAppServerManager.ts:1222
item/tool/call requests with non-agentwatch. tool names are routed to this.agentWatch.handleToolCall(), which throws Unsupported tool: ${toolName} and returns a JSON-RPC error to the client. This breaks all non-agentwatch tool calls instead of letting them fall through to the existing handling logic. Consider checking toolName.startsWith("agentwatch.") before routing to the agentWatch handler.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/codexAppServerManager.ts around line 1222:
`item/tool/call` requests with non-`agentwatch.` tool names are routed to `this.agentWatch.handleToolCall()`, which throws `Unsupported tool: ${toolName}` and returns a JSON-RPC error to the client. This breaks all non-agentwatch tool calls instead of letting them fall through to the existing handling logic. Consider checking `toolName.startsWith("agentwatch.")` before routing to the agentWatch handler.
Evidence trail:
apps/server/src/codexAppServerManager.ts lines 1222-1256 at REVIEWED_COMMIT: Shows `item/tool/call` handler routing ALL tool calls to `this.agentWatch.handleToolCall(toolName, rawArgs)` without checking if toolName starts with "agentwatch."
apps/server/src/agentWatch.ts lines 283-321 at REVIEWED_COMMIT: Shows `handleToolCall` only handles `agentwatch.start`, `agentwatch.status`, `agentwatch.poll`, `agentwatch.tail` - all other tool names cause `throw new Error(\`Unsupported tool: ${toolName}\`);`
| function isProcessAlive(pid: number): boolean { | ||
| try { | ||
| process.kill(pid, 0); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟠 High src/agentWatch.ts:56
When job.pid is -1 (set on line 149 when child.pid is undefined after spawn), calling process.kill(-1, 0) does not check if a specific process is alive. On POSIX systems, kill(-1, sig) broadcasts the signal to all processes the caller can signal, so isProcessAlive(-1) returns true even though no actual job process exists. The function should return false when pid <= 0.
+function isProcessAlive(pid: number): boolean {
+ if (pid <= 0) {
+ return false;
+ }
+ try {
+ process.kill(pid, 0);
+ return true;
+ } catch {
+ return false;
+ }
+}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/agentWatch.ts around lines 56-63:
When `job.pid` is `-1` (set on line 149 when `child.pid` is undefined after spawn), calling `process.kill(-1, 0)` does not check if a specific process is alive. On POSIX systems, `kill(-1, sig)` broadcasts the signal to all processes the caller can signal, so `isProcessAlive(-1)` returns `true` even though no actual job process exists. The function should return `false` when `pid <= 0`.
Evidence trail:
1. `apps/server/src/agentWatch.ts` lines 56-62: `isProcessAlive` function uses `process.kill(pid, 0)`
2. `apps/server/src/agentWatch.ts` line 149: `pid: child.pid ?? -1` sets pid to -1 if child.pid is undefined
3. `apps/server/src/agentWatch.ts` line 221: `if (!isProcessAlive(job.pid))` uses the function to detect finished jobs
4. Node.js documentation (https://nodejs.org/docs/latest-v22.x/api/process.html): `process.kill()` follows OS semantics, so `kill(-1, sig)` on POSIX sends to all processes caller can signal
5. POSIX `kill(2)` specification: pid=-1 means 'all processes for which the calling process has permission to send signals'
1 participant
Summary
AgentWatchruntime for monitored long-running shell jobsitem/tool/callrequests foragentwatch.start|status|poll|tailthroughCodexAppServerManagerWhy
This is a small reliability-oriented slice that lets Codex hand off long-running commands to a monitored background job instead of keeping everything tied to a single turn.
Scope
This PR intentionally avoids the later UI and orchestration follow-up work. It only adds the core runtime and Codex server plumbing.
Validation
bun fmtbun lintbun typecheckNote
Add AgentWatch tool handling for Codex long-running jobs
AgentWatch, a job supervisor that starts detached shell commands viabash -lc, tracks liveness and output staleness, and captures exit codes by appending a sentinel marker to the job's log file.agentwatch.start,agentwatch.status,agentwatch.poll, andagentwatch.tailtools via ahandleToolCallinterface.AgentWatchintoCodexAppServerManager, routingitem/tool/callJSON-RPC requests toagentWatch.handleToolCalland disposing the instance onstopAll().handleServerRequestnow handles a newitem/tool/callmethod, returning-32602for missing tool name and-32000for tool invocation failures.📊 Macroscope summarized 664fb01. 4 files reviewed, 4 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues