fix: correct MIME type detection for Computer Use screenshots#262
fix: correct MIME type detection for Computer Use screenshots#262amDosion wants to merge 2 commits intoclaude-code-best:mainfrom
Conversation
…y, KAIROS activation, openclaw autonomy Squashed merge of: 1. fix/mcp-tsc-errors — 修复上游 MCP 重构后的 tsc 错误和测试失败 2. feat/pipe-mute-disconnect — Pipe IPC 逻辑断开、/lang 命令、mute 状态机 3. feat/stub-recovery-all — 实现全部 stub 恢复 (task 001-012) 4. feat/kairos-activation — KAIROS 激活解除阻塞 + 工具实现 5. codex/openclaw-autonomy-pr — 自治权限系统、运行记录、managed flows Conflicts resolved: - src/commands/assistant/assistant.tsx (stub-recovery + kairos) - src/services/api/openai/__tests__/queryModelOpenAI.test.ts (mcp-fix + autonomy) Tested: bun test (2695 pass, 0 fail)
The original detectMimeFromBase64() compared raw byte magic numbers against charCodeAt(0) of a base64-encoded string. Base64 encoding transforms byte values, so no condition ever matched — the function always returned "image/png". On Windows the Python bridge produces JPEG screenshots, causing API 400 errors. Fix: decode the first 12 raw bytes from base64 and check standard magic byte signatures directly: - PNG: 89 50 4E 47 (4-byte signature) - JPEG: FF D8 FF (3-byte, covers all marker variants) - WebP: RIFF + WEBP dual check (bytes 0-3 and 8-11) - GIF: 47 49 46 (covers GIF87a and GIF89a)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive autonomy system with daemon state management, background sessions, template jobs, language support, and assistant session handling. It adds new build features ( Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Process
participant State as Daemon State File
participant Monitor as Status Command
participant Stop as Stop Command
Note over CLI: Daemon starts
CLI->>State: writeDaemonState(pid, cwd, ...)
State-->>CLI: State persisted
Note over CLI: User requests daemon status
Monitor->>State: readDaemonState()
State-->>Monitor: {pid, ...}
Monitor->>Monitor: queryDaemonStatus()<br/>Check if PID alive
alt Process running
Monitor-->>CLI: "running"
else Process dead
Monitor->>State: removeDaemonState()
State-->>Monitor: Cleaned
Monitor-->>CLI: "stale"
end
Note over CLI: User requests daemon stop
Stop->>State: readDaemonState()
State-->>Stop: {pid, ...}
Stop->>Stop: SIGTERM → wait → SIGKILL
alt Success
Stop->>State: removeDaemonState()
State-->>Stop: Cleaned
Stop-->>CLI: "Daemon stopped"
end
sequenceDiagram
participant Proactive as Proactive Tick
participant Authority as Autonomy Authority
participant Runs as Autonomy Runs
participant Queue as Command Queue
participant REPL as REPL Handler
Proactive->>Authority: loadAutonomyAuthority()
Authority-->>Proactive: agents+heartbeat content
Proactive->>Runs: createProactiveAutonomyCommands(basePrompt)
Runs->>Runs: createAutonomyQueuedPrompt()<br/>for each due heartbeat task
Runs->>Runs: Persist runs + flow records
Runs-->>Proactive: [QueuedCommand, ...]
loop For each command
Proactive->>Queue: enqueue(command)
Queue-->>Proactive: UUID assigned
end
Proactive->>REPL: run()
REPL->>REPL: markAutonomyRunRunning(runId)
REPL->>REPL: ask(prompt, context)
activate REPL
Note over REPL: LLM processes
deactivate REPL
REPL->>REPL: finalizeAutonomyRunCompleted(runId)
alt Has nextCommands
REPL->>Queue: enqueue(nextCommand)
end
sequenceDiagram
participant Master as Master Process
participant Slave as Slave Process
participant MuteState as Pipe Mute State
Note over Master: Route mode changes to local
Master->>MuteState: setMasterMutedPipes(unmuted_set)
MuteState-->>Master: Updated muted set
loop Muted slave
Master->>MuteState: Send relay_mute
Master->>Slave: {type: 'relay_mute'}
Slave->>Slave: setRelayMuted(true)
Slave->>Slave: Drop business messages
end
Note over Master: User unmutes slave via /send
Master->>MuteState: addSendOverride(slaveName)
Master->>MuteState: Send relay_unmute
Master->>Slave: {type: 'relay_unmute'}
Slave->>Slave: setRelayMuted(false)
Note over Master: Slave turn completes
Slave->>Master: {type: 'done'}
Master->>MuteState: removeSendOverride(slaveName)
Master->>Slave: {type: 'relay_mute'} (re-mute)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/proactive/useProactive.ts (1)
103-108:⚠️ Potential issue | 🟡 MinorFunction calls in useEffect dependency array is an anti-pattern.
Calling
isProactiveActive(),isProactivePaused(), andisContextBlocked()directly in the dependency array evaluates them during render. While this may work if they return stable primitives, it violates React's expectation that dependencies should be values captured in the closure, not function invocations.♻️ Suggested fix: capture values before the effect
+ const proactiveActive = isProactiveActive() + const proactivePaused = isProactivePaused() + const contextBlocked = isContextBlocked() + useEffect(() => { - if (!isProactiveActive()) return + if (!proactiveActive) return // ... rest of effect }, [ - isProactiveActive(), - isProactivePaused(), - isContextBlocked(), + proactiveActive, + proactivePaused, + contextBlocked, ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proactive/useProactive.ts` around lines 103 - 108, The useEffect currently invokes isProactiveActive(), isProactivePaused(), and isContextBlocked() directly in its dependency array which evaluates them during render; instead call those functions once at the top of the hook/component to capture their returned values (e.g., const proactiveActive = isProactiveActive(); const proactivePaused = isProactivePaused(); const contextBlocked = isContextBlocked();) and then replace the function calls in the useEffect dependency array with those captured variables so useEffect depends on values, not function invocations; update the useEffect invocation in useProactive.ts to reference the new locals.src/cli/print.ts (1)
2166-2292:⚠️ Potential issue | 🟠 MajorGuard the
runningtransition with the same failure path.
markAutonomyRunRunning()sits outside the newtry. If it rejects, none of thefinalizeAutonomyRunFailed()calls run, so the autonomy run can be left stranded in queued/running state.💡 Suggested fix
- for (const runId of autonomyRunIds) { - await markAutonomyRunRunning(runId) - } try { + for (const runId of autonomyRunIds) { + await markAutonomyRunRunning(runId) + } await runWithWorkload(cmd.workload ?? options.workload, async () => { for await (const message of ask({ commands: uniqBy(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/print.ts` around lines 2166 - 2292, The calls to markAutonomyRunRunning(runId) are currently executed before the try block so if they reject the catch path (finalizeAutonomyRunFailed) never runs; move the loop that awaits markAutonomyRunRunning for each runId into the try block (immediately before runWithWorkload) so any rejection will be caught by the existing catch and will invoke finalizeAutonomyRunFailed for each autonomyRunIds, keeping the same ordering and awaiting behavior for markAutonomyRunRunning, and ensure autonomyRunIds, markAutonomyRunRunning, runWithWorkload, and finalizeAutonomyRunFailed are the referenced symbols you adjust.
🟡 Minor comments (12)
packages/builtin-tools/src/tools/PushNotificationTool/PushNotificationTool.ts-117-131 (1)
117-131:⚠️ Potential issue | 🟡 MinorDon't collapse bridge delivery failures into the "not configured" fallback.
A 4xx response or thrown bridge error currently ends up returning
No Remote Control bridge configured, even when a bridge is configured and the request was actually rejected. That makes auth/session/runtime failures indistinguishable from the true no-bridge case.💡 Suggested fix
+ let bridgeError: string | undefined + // Try bridge delivery first (for remote/mobile viewers) if (appState.replBridgeEnabled) { if (feature('BRIDGE_MODE')) { try { @@ if (response.status >= 200 && response.status < 300) { logForDebugging(`[PushNotification] delivered via bridge session=${sessionId}`) return { data: { sent: true } } } - logForDebugging(`[PushNotification] bridge delivery failed: status=${response.status}`) + bridgeError = `Bridge delivery failed with status ${response.status}.` + logForDebugging(`[PushNotification] ${bridgeError}`) } } catch (e) { + bridgeError = 'Bridge delivery failed before the notification could be sent.' logForDebugging(`[PushNotification] bridge delivery error: ${e}`) } } } @@ - return { data: { sent: false, error: 'No Remote Control bridge configured. Notification not delivered.' } } + return { + data: { + sent: false, + error: bridgeError ?? 'No Remote Control bridge configured. Notification not delivered.', + }, + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/builtin-tools/src/tools/PushNotificationTool/PushNotificationTool.ts` around lines 117 - 131, The current logic in PushNotificationTool (in PushNotificationTool.ts) collapses bridge delivery failures into the "No Remote Control bridge configured" fallback; update the delivery branch inside the block that calls the bridge (the try/catch around the HTTP call where `response`, `sessionId`, and `logForDebugging` are used) so that if a bridge was attempted but returns a non-2xx status you immediately return { data: { sent: false, error: 'Bridge delivery failed', status: response.status, sessionId } } (or similar descriptive error), and in the catch handler return { data: { sent: false, error: 'Bridge delivery error', details: e, sessionId } }; only keep the "No Remote Control bridge configured" message when no bridge was attempted at all. Ensure the returned shape stays consistent with other branches.src/cli/handlers/ant.ts-198-216 (1)
198-216:⚠️ Potential issue | 🟡 Minor
completionHandlerhas identical branches —opts.outputis unused.Both the
if (opts.output)andelsebranches execute the same code. Theshellparameter is also only used in the log message but doesn't affect behavior. This appears to be an incomplete implementation.Potential fix if output file writing was intended
export async function completionHandler( shell: string, opts: { output?: string }, _program: Command, ): Promise<void> { const { regenerateCompletionCache } = await import( '../../utils/completionCache.js' ) + const completionScript = await regenerateCompletionCache(shell) if (opts.output) { - // Generate and write to file - await regenerateCompletionCache() - console.log(`Completion cache regenerated for ${shell}.`) + const { writeFile } = await import('fs/promises') + await writeFile(opts.output, completionScript, 'utf-8') + console.log(`Completion script written to ${opts.output}`) } else { - // Regenerate and output to stdout - await regenerateCompletionCache() - console.log(`Completion cache regenerated for ${shell}.`) + console.log(completionScript) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/handlers/ant.ts` around lines 198 - 216, The completionHandler currently ignores opts.output and has identical branches; update it so when opts.output is provided it writes the regenerated completion data to the given file path, and otherwise prints the completion data to stdout. Specifically, call regenerateCompletionCache to obtain the completion text (or pass shell into regenerateCompletionCache if that helper accepts a shell argument), then if opts.output is set use fs/promises.writeFile (or writeFileSync) to write that returned string to opts.output and log a confirmation, otherwise console.log the returned string; ensure you reference completionHandler, opts.output, shell and regenerateCompletionCache when making these changes.docs/task/task-001-daemon-status-stop.md-6-6 (1)
6-6:⚠️ Potential issue | 🟡 MinorStatus/checklist are inconsistent.
Line 6 says
DONE, but Lines 64-68 are still unchecked. Please either mark completed checks as[x]or set status to an in-progress state to avoid ambiguity.Also applies to: 64-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/task/task-001-daemon-status-stop.md` at line 6, The document's status header "状态: DONE" conflicts with the unchecked checklist later in the file; either update the status header (e.g., change "状态: DONE" to "状态: IN-PROGRESS" or "状态: TODO") or mark the pending checklist items as completed by converting their unchecked boxes to checked (change "[ ]" to "[x]") so the header and the checklist remain consistent (update the "状态: ..." line or the checklist block accordingly).src/utils/taskSummary.ts-34-34 (1)
34-34:⚠️ Potential issue | 🟡 MinorRate-limit timestamp updated before early return.
lastSummaryTimeis set at line 34 before the messages check at line 44. IfforkContextMessagesis empty, the function returns early but still consumes the rate-limit window. Consider moving the timestamp update after validation.♻️ Proposed fix: update timestamp only on actual work
export function maybeGenerateTaskSummary( options: Record<string, unknown>, ): void { - lastSummaryTime = Date.now() - try { const messages = options.forkContextMessages as | Array<{ type: string message?: { content?: unknown } }> | undefined if (!messages || messages.length === 0) return + + lastSummaryTime = Date.now()Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/taskSummary.ts` at line 34, The rate-limit timestamp lastSummaryTime is being updated before validation which causes the window to be consumed even when no work is done; move the assignment of lastSummaryTime so it only runs after you confirm forkContextMessages is non-empty (or after the function proceeds past the early return) in the task summary function (e.g., the function that references lastSummaryTime and forkContextMessages in src/utils/taskSummary.ts) so the timestamp is set only when actual summarization work occurs.src/proactive/useProactive.ts-75-87 (1)
75-87:⚠️ Potential issue | 🟡 MinorMissing error handling in async IIFE.
If
createProactiveAutonomyCommandsthrows or rejects, the error becomes an unhandled promise rejection. Consider wrapping in try/catch to log or gracefully handle failures.🛡️ Proposed fix: add error handling
void (async () => { + try { const commands = await createProactiveAutonomyCommands({ basePrompt: `<${TICK_TAG}>${new Date().toLocaleTimeString()}</${TICK_TAG}>`, currentDir: getCwd(), }) for (const command of commands) { optsRef.current.onQueueTick(command) } + } catch (err) { + // Log but don't crash the tick scheduler + console.error('[proactive] command generation failed:', err) + } })()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proactive/useProactive.ts` around lines 75 - 87, The async IIFE invoking createProactiveAutonomyCommands lacks error handling and can cause unhandled promise rejections; wrap the body of the IIFE in a try/catch around the await createProactiveAutonomyCommands(...) and the for-loop that calls optsRef.current.onQueueTick(command), and in the catch log the error (including context such as TICK_TAG and currentDir via getCwd()) or call a graceful failure handler (e.g., an existing error dispatch or optsRef.current.onQueueError) so failures are recorded and do not leak as unhandled rejections.src/hooks/useScheduledTasks.ts-115-122 (1)
115-122:⚠️ Potential issue | 🟡 MinorUse a type guard instead of direct type assertion for
command.value.While
createAutonomyQueuedPromptalways returns aQueuedCommandwith a string value (derived fromPreparedAutonomyTurn.prompt), the directas stringassertion bypasses TypeScript's type checking. Per coding guidelines, use a type guard to narrow union types rather than casting. Consider narrowing the type with a check liketypeof command.value === 'string'before use, or update the type signature if this context always guarantees a string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScheduledTasks.ts` around lines 115 - 122, The code uses a direct cast (command.value as string) when calling injectUserMessageToTeammate; replace this with a proper type guard to narrow the union type instead of asserting—check typeof command.value === 'string' (or otherwise guard for string) before invoking injectUserMessageToTeammate so you only pass a real string, and handle the non-string branch (log, skip, or convert) accordingly; alternatively, if createAutonomyQueuedPrompt/QueuedCommand truly always yields a string here, update the QueuedCommand type/signature so callers like useScheduledTasks.ts can rely on a string without an assertion. Ensure references include command.value, injectUserMessageToTeammate, setAppState, and command.autonomy?.runId/origin when applying the guard.docs/test-plans/openclaw-autonomy-baseline.md-84-87 (1)
84-87:⚠️ Potential issue | 🟡 MinorThe acceptance count is off by one.
This document lists five added test files above, but the acceptance criteria still says "four new test files". Please make those counts match.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/test-plans/openclaw-autonomy-baseline.md` around lines 84 - 87, Update the acceptance criterion that currently reads "The four new test files pass." to match the list of five added tests by changing it to "The five new test files pass." (or alternatively remove the hard-coded number and use "all new test files pass.") Ensure the sentence replacement targets the exact string "The four new test files pass." so the counts in the document match.docs/test-plans/openclaw-autonomy-baseline.md-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorFix the repository name typo.
Claude-code-bastreads like a typo and will undermine the doc if copied elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/test-plans/openclaw-autonomy-baseline.md` at line 11, Replace the typo string "Claude-code-bast" with the correct repository name "Claude-code-base" wherever it appears in the document; search for the exact token "Claude-code-bast" and update it to "Claude-code-base" to ensure the doc references the correct repo name.src/jobs/state.ts-58-61 (1)
58-61:⚠️ Potential issue | 🟡 MinorValidate
state.jsonbefore treating it asJobState.
JSON.parse(raw) as JobStatetrusts on-disk data without checking the required fields. A partial or corrupt file will flow through as a valid state object and then get rewritten byappendJobReply(). Parse intoRecord<string, unknown>and guard the shape first. As per coding guidelines, "Use type guards to narrow union types rather than type casting".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/state.ts` around lines 58 - 61, The readJobState function currently trusts disk contents by doing JSON.parse(raw) as JobState; change it to parse into a general Record<string, unknown> and validate the object shape with a type guard before returning a JobState (check required fields and their types for JobState), returning null if validation fails or the file is malformed; update readJobState to use getJobDir and state.json as before, and ensure callers such as appendJobReply handle a null return (do not rely on unchecked casting).docs/features/stub-recovery-design-1-4.md-18-24 (1)
18-24:⚠️ Potential issue | 🟡 MinorReplace machine-local links with repo-relative paths.
These markdown links point to
e:/Source_code/..., so they'll be broken for everyone except the authoring machine. Use repo-relative paths instead, or plain inline code if the reference is only illustrative.Also applies to: 55-57, 80-89, 125-130, 153-161, 188-202, 225-235, 275-282
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/stub-recovery-design-1-4.md` around lines 18 - 24, The markdown contains machine-local file links like `e:/Source_code/Claude-code-bast/src/daemon/main.ts` that must be replaced with repo-relative paths or inline code; update the links shown (references to `src/daemon/main.ts`, `src/daemon/workerRegistry.ts`, and `src/commands/remoteControlServer/remoteControlServer.tsx`) to use repo-relative links (e.g., `src/daemon/main.ts`) or plain inline code and apply the same fix to the other ranges called out (lines 55-57, 80-89, 125-130, 153-161, 188-202, 225-235, 275-282) so no links point to local drive paths.src/utils/autonomyFlows.ts-960-984 (1)
960-984:⚠️ Potential issue | 🟡 MinorMinor:
loststatus is counted but not included in output.The
countsobject trackslost: 0but the status output doesn't include a line for it. This is inconsistent.🔧 Proposed fix
return [ `Autonomy flows: ${flows.length}`, `Queued: ${counts.queued}`, `Running: ${counts.running}`, `Waiting: ${counts.waiting}`, `Blocked: ${counts.blocked}`, `Succeeded: ${counts.succeeded}`, `Failed: ${counts.failed}`, `Cancelled: ${counts.cancelled}`, + `Lost: ${counts.lost}`, ].join('\n')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/autonomyFlows.ts` around lines 960 - 984, The function formatAutonomyFlowsStatus builds a counts object including a lost property but never prints it; update formatAutonomyFlowsStatus to include the lost count in the returned status lines (add a `Lost: ${counts.lost}` entry) so the counted `lost` status is reflected in the output; locate the counts declaration and the array of status strings in formatAutonomyFlowsStatus to insert the missing line.src/cli/bg.ts-297-303 (1)
297-303:⚠️ Potential issue | 🟡 MinorLog directory may not exist when session starts.
The
logPathis constructed but the parent directory (sessions/logs/) is never created. When the background session attempts to write logs, it may fail if the directory doesn't exist.🛠️ Proposed fix
+import { mkdir } from 'fs/promises' + export async function handleBgFlag(args: string[]): Promise<void> { // Check tmux availability const { code: tmuxCode } = await execFileNoThrow('tmux', ['-V']) if (tmuxCode !== 0) { console.error( 'tmux is required for --bg. Install tmux to use background sessions.', ) console.error(getTmuxHint()) process.exitCode = 1 return } const sessionName = `claude-bg-${randomUUID().slice(0, 8)}` - const logPath = join( + const logsDir = join( + getClaudeConfigHomeDir(), + 'sessions', + 'logs', + ) + await mkdir(logsDir, { recursive: true }) + const logPath = join( + logsDir, - getClaudeConfigHomeDir(), - 'sessions', - 'logs', `${sessionName}.log`, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/bg.ts` around lines 297 - 303, The code builds logPath using sessionName and getClaudeConfigHomeDir() but never ensures its parent directory exists, which can cause writes to fail; before using logPath (e.g., prior to starting the background session or writing logs) create the parent directory (use path.dirname(logPath)) with recursive=true (fs.mkdir or fs.promises.mkdir) and handle/report any errors; update the surrounding logic in the function that defines sessionName/logPath so the directory is created synchronously or awaited before opening/streaming the log file.
🧹 Nitpick comments (21)
tsconfig.json (1)
24-25: Expandincludecoverage for newly aliased packages.Line 24 includes only
packages/mcp-client/src/**/*.ts. Consider addingmcp-clientTSX andagent-toolsTS/TSX to keeptsc --noEmitcoverage comprehensive.♻️ Proposed update
- "include": ["src/**/*.ts", "src/**/*.tsx", "packages/builtin-tools/src/**/*.ts", "packages/builtin-tools/src/**/*.tsx", "packages/mcp-client/src/**/*.ts"], + "include": [ + "src/**/*.ts", + "src/**/*.tsx", + "packages/builtin-tools/src/**/*.ts", + "packages/builtin-tools/src/**/*.tsx", + "packages/mcp-client/src/**/*.ts", + "packages/mcp-client/src/**/*.tsx", + "packages/agent-tools/src/**/*.ts", + "packages/agent-tools/src/**/*.tsx" + ],Based on learnings: "Applies to **/*.{ts,tsx} : TypeScript strict mode must pass with zero errors via
bunx tsc --noEmit."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` around lines 24 - 25, Update the tsconfig "include" array to also cover the newly aliased packages by adding patterns for TSX and the agent-tools package: include "packages/mcp-client/src/**/*.tsx" and both "packages/agent-tools/src/**/*.ts" and "packages/agent-tools/src/**/*.tsx" (or a single "packages/agent-tools/src/**/*.{ts,tsx}" pattern) so tsc --noEmit will type-check TS and TSX files in mcp-client and agent-tools in addition to existing entries.src/services/api/openai/__tests__/streamAdapter.test.ts (1)
12-18: Unused import — the module-leveladaptOpenAIStreamToAnthropicbinding is never used.The
collectEvents()helper (line 53) performs its own dynamic import with a unique URL on each call, so the binding from line 18 is dead code. Consider removing lines 12-18 entirely if the temp-file approach is no longer needed at module scope.♻️ Proposed fix
-const _testDir = dirname(fileURLToPath(import.meta.url)) -const _realSource = readFileSync(join(_testDir, '..', 'streamAdapter.ts'), 'utf-8') -const _tempDir = join(tmpdir(), `stream-adapter-test-${Date.now()}`) -mkdirSync(_tempDir, { recursive: true }) -const _tempFile = join(_tempDir, 'streamAdapter.ts') -writeFileSync(_tempFile, _realSource, 'utf-8') -const { adaptOpenAIStreamToAnthropic } = await import(_tempFile)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/openai/__tests__/streamAdapter.test.ts` around lines 12 - 18, Remove the unused module-level temp-file creation and import that defines the adaptOpenAIStreamToAnthropic binding (the block creating _tempDir/_tempFile and the await import(_tempFile)); the test helper collectEvents already does its own dynamic import with a unique URL per call, so delete the lines that create and write the temp file and the top-level const { adaptOpenAIStreamToAnthropic } = await import(_tempFile) binding to eliminate the dead code.src/utils/pipeMuteState.ts (2)
42-47: Listener exceptions could break notification to subsequent subscribers.If a listener throws in
emitSendOverrideChanged, the iteration stops and remaining listeners won't be notified. Consider wrapping listener calls.🛡️ Defensive listener invocation
function emitSendOverrideChanged(): void { _sendOverrideVersion += 1 for (const listener of _sendOverrideListeners) { - listener() + try { + listener() + } catch { + // Listener errors should not break notification chain + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/pipeMuteState.ts` around lines 42 - 47, The emitSendOverrideChanged function currently increments _sendOverrideVersion and invokes each listener directly, so if any listener throws the loop aborts and remaining subscribers are not notified; update emitSendOverrideChanged to call each listener inside a try/catch (iterating _sendOverrideListeners) and handle exceptions per-listener (e.g., log or swallow) so one failing listener cannot stop notification to others while still preserving the version increment semantics.
49-52:addSendOverrideemits change notification even when the override already exists.Unlike
removeSendOverridewhich checks for actual change,addSendOverridealways increments version and notifies listeners. This may cause unnecessary re-renders or work in subscribers.♻️ Only emit on actual change
export function addSendOverride(name: string): void { - _sendOverrides.add(name) - emitSendOverrideChanged() + if (!_sendOverrides.has(name)) { + _sendOverrides.add(name) + emitSendOverrideChanged() + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/pipeMuteState.ts` around lines 49 - 52, addSendOverride unconditionally calls emitSendOverrideChanged causing redundant notifications; change it to first check whether _sendOverrides already contains the name (use _sendOverrides.has(name)), only add the name and call emitSendOverrideChanged when it was not present, mirroring the behavior of removeSendOverride so subscribers are only notified on real state changes; update the function addSendOverride to perform this presence check before mutating and emitting.src/__tests__/context.baseline.test.ts (2)
77-90: Conditional assertion weakens test coverage.The
if ('cacheBreaker' in second)branch means this test might not verify the primary assertion (cacheBreakercontains the injection string) if the implementation changes. Consider making the expectation deterministic.💡 Suggested fix
If
cacheBreakeris expected after injection, assert it directly:const second = await getSystemContext() - if ('cacheBreaker' in second) { - expect(second.cacheBreaker).toContain('baseline-cache-break') - } else { - expect(second.gitStatus).toBeUndefined() - } + expect(second.cacheBreaker).toContain('baseline-cache-break')If the field is truly optional based on implementation details, add a comment explaining the expected behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/context.baseline.test.ts` around lines 77 - 90, The test "getSystemContext reflects system prompt injection after cache invalidation" uses a conditional 'if ("cacheBreaker" in second)' which weakens the assertion; update the test to deterministically assert the expected behavior after calling setSystemPromptInjection('baseline-cache-break') — either directly expect second.cacheBreaker to contain 'baseline-cache-break' (using getSystemContext and cacheBreaker) or, if cacheBreaker may legitimately be absent, add an explicit comment and assert the alternative (gitStatus remains undefined) so the test documents the contract of getSystemContext and setSystemPromptInjection rather than allowing a silent pass via the conditional.
28-29: Optional chaining on.cache.clearsuggests fragile coupling to memoization internals.Accessing
getUserContext.cache.clear?.()relies on implementation details of the memoization. If the memoization strategy changes, these calls silently become no-ops. Consider exposing explicit test reset functions from the module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/context.baseline.test.ts` around lines 28 - 29, The test is relying on memoization internals via getUserContext.cache.clear?.() and getSystemContext.cache.clear?.(), which is fragile; add explicit reset functions in the module (e.g., export function resetUserContext() and export function resetSystemContext() or add clearCache methods on the exported functions) that reliably clear whatever memoization is used, update the tests to call resetUserContext() and resetSystemContext() instead of using optional chaining on .cache.clear, and ensure the new reset functions encapsulate the current memoization logic so future changes won't break the tests.src/utils/__tests__/cronTasks.baseline.test.ts (1)
194-202: Test name doesn't match assertion behavior.The test is named "returns the exact next fire time when no second match exists" but only asserts
>= fromMs. If the intent is to verify deterministic behavior for rare cron schedules, consider asserting equality with the non-jittered result.💡 Align test name with assertions or strengthen assertions
Option 1 - Rename to match behavior:
-test('jitteredNextCronRunMs returns the exact next fire time when no second match exists in range', () => { +test('jitteredNextCronRunMs returns a valid time at or after fromMs for rare schedules', () => {Option 2 - Assert exact equality if that's the expected behavior:
expect(jittered).not.toBeNull() - expect(jittered!).toBeGreaterThanOrEqual(fromMs) + expect(jittered).toBe(exact)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/__tests__/cronTasks.baseline.test.ts` around lines 194 - 202, Test name and assertions disagree: update the test in src/utils/__tests__/cronTasks.baseline.test.ts to either rename it to reflect that jittered result need only be >= fromMs, or strengthen assertions to require exact equality with the deterministic cron result; specifically, compute exact = nextCronRunMs('0 0 29 2 *', fromMs) and replace expect(jittered!).toBeGreaterThanOrEqual(fromMs) with expect(jittered).toEqual(exact) if you intend deterministic behavior for oneShotJitteredNextCronRunMs, otherwise change the test title to say "returns a next fire time >= fromMs" to match the current assertion.src/cli/rollback.ts (2)
63-65: Handlenullstatus from signal-terminated processes.When a process is killed by a signal,
result.statusisnullandresult.signalcontains the signal name. The current code handles this with?? 1, but the error message would show "exit code null".♻️ Improved error message
if (result.status !== 0) { - console.error(`Rollback failed with exit code ${result.status}`) + if (result.signal) { + console.error(`Rollback terminated by signal ${result.signal}`) + } else { + console.error(`Rollback failed with exit code ${result.status}`) + } process.exitCode = result.status ?? 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/rollback.ts` around lines 63 - 65, The current rollback exit handling prints "exit code null" when the child was killed by a signal; update the if (result.status !== 0) branch that checks result to detect result.status === null and, in that case, log a message using result.signal (e.g., "Rollback terminated by signal SIG...") and set process.exitCode to a non-zero value (or 1); otherwise keep the existing message for numeric statuses and set process.exitCode to result.status. Target the block in rollback.ts where result.status is inspected (the if (result.status !== 0) branch) and adjust the logging and exitCode assignment accordingly.
56-61: Consider validating thetargetversion format.While
spawnSyncwith array arguments avoids shell injection, validatingtargetagainst a semantic version pattern would provide defense-in-depth and better user feedback for typos.💡 Optional validation
+const VERSION_PATTERN = /^\d+\.\d+\.\d+(-[\w.]+)?$/ + console.log(`Rolling back to version ${target}...`) if (options?.dryRun) { console.log(` (dry run — would install ${target})`) return } +if (!VERSION_PATTERN.test(target)) { + console.error(`Invalid version format: ${target}`) + console.error('Expected format: X.Y.Z (e.g., 2.1.880)') + process.exitCode = 1 + return +} + // Version rollback via npm/bun const { spawnSync } = await import('child_process')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/rollback.ts` around lines 56 - 61, Validate the target string before calling spawnSync: ensure the variable target in rollback.ts matches a semantic version pattern (e.g., MAJOR.MINOR.PATCH with optional prerelease/build) and if it fails, print a clear error and exit instead of invoking spawnSync; implement validation either by using the semver package (semver.valid/semver.coerce) or a strict regex, and keep the existing spawnSync invocation unchanged to preserve safety.src/cli/up.ts (1)
70-94: Code fence stripping may not handle multiple code blocks correctly.If the
# claude upsection contains multiple fenced code blocks, the regex strips only the outermost markers, potentially leaving intermediate fence syntax in the executed script.💡 Consider documenting expected format
If multiple code blocks aren't supported, consider adding a comment or updating the usage message to clarify that only a single code block is expected:
console.log( 'No "# claude up" section found in CLAUDE.md.\n' + 'Add a section like:\n\n' + ' # claude up\n' + ' ```bash\n' + ' npm install\n' + ' npm run build\n' + - ' ```', + ' ```\n\n' + + 'Note: Only a single code block is supported.', )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/up.ts` around lines 70 - 94, The current extractUpSection function only strips the first/last fenced code markers which leaves intermediate ``` markers if there are multiple code blocks; update extractUpSection to either (A) robustly extract and concatenate the contents of all fenced code blocks inside the section by scanning sectionLines for fence boundaries and joining each fenced block's inner lines into text, or (B) validate and reject multiple fenced blocks (return null or surface an error) if only a single block is supported — implement this logic using the existing sectionLines/text variables in extractUpSection so all ```...``` fenced blocks are handled deterministically (or a clear validation message is returned).src/jobs/__tests__/classifier.test.ts (1)
31-44: Consider usingRecord<string, unknown>for theextraparameter.While
as anyis allowed in tests for mock data per coding guidelines, usingRecord<string, unknown>for theextraparameter would be more consistent with production code patterns.♻️ Suggested improvement
function makeAssistantMessage( content: any[], - extra: Record<string, any> = {}, + extra: Record<string, unknown> = {}, ): AssistantMessage {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/__tests__/classifier.test.ts` around lines 31 - 44, The test helper function makeAssistantMessage currently types the extra parameter as Record<string, any>; change its type to Record<string, unknown> to match production patterns and stronger typing, and update any call sites in the tests if needed to satisfy the new type while keeping the function return typing (AssistantMessage) and allowed test use of as any unchanged.src/daemon/main.ts (2)
135-136: Same non-null assertion concern applies here.Line 135 uses
result.state!.pidwith the same assumption. Consider consolidating the status check pattern to ensure type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/daemon/main.ts` around lines 135 - 136, The console.log call uses a non-null assertion on result.state!.pid; replace this with a proper guard like the earlier status-check pattern so we only access result.state.pid when result.state and result.state.pid are defined (and otherwise handle the missing state gracefully), e.g. check result.state and result.state.pid before logging and before calling stopDaemonByPid to ensure type safety and avoid runtime errors; update the code around result.state, result.state.pid and stopDaemonByPid accordingly.
100-108: Non-null assertion onresult.statecould be avoided with type narrowing.The
result.state!assertion at lines 102 and 107 relies on the assumption thatstateis always present whenstatus === 'running'. IfqueryDaemonStatusreturns a discriminated union type, TypeScript should narrow automatically without needing!.♻️ Safer pattern using type guard or destructuring
case 'running': { - const s = result.state! + const { state: s } = result + if (!s) break // defensive; type should guarantee this console.log(`daemon status: running`) console.log(` PID: ${s.pid}`)Alternatively, ensure
queryDaemonStatusreturns a discriminated union wherestatus: 'running'guaranteesstateis defined, eliminating the need for assertions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/daemon/main.ts` around lines 100 - 108, Remove the non-null assertion on result.state in the 'running' switch branch and let TypeScript narrow the type (or add an explicit type guard) instead: in the case 'running' block of the switch over queryDaemonStatus()'s return, replace "const s = result.state!" with a safe binding such as "const s = result.state" (or destructure: "const { state: s } = result") and, if necessary, add an early check (e.g., if (!s) throw new Error(...)) or implement a type guard so the compiler knows that when result.status === 'running' the state is defined; reference symbols: result, state, queryDaemonStatus, and the 'running' switch case.src/utils/handlePromptSubmit.ts (1)
29-34: Unused import:markAutonomyRunFailed.
markAutonomyRunFailedis imported but never used in this file. The failure path usesfinalizeAutonomyRunFailedinstead.🧹 Remove unused import
import { finalizeAutonomyRunCompleted, finalizeAutonomyRunFailed, - markAutonomyRunFailed, markAutonomyRunRunning, } from './autonomyRuns.js'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/handlePromptSubmit.ts` around lines 29 - 34, The import list in handlePromptSubmit.ts includes an unused symbol markAutonomyRunFailed; remove markAutonomyRunFailed from the import statement that currently brings in finalizeAutonomyRunCompleted, finalizeAutonomyRunFailed, markAutonomyRunFailed, markAutonomyRunRunning so only the actually used functions (e.g., finalizeAutonomyRunCompleted, finalizeAutonomyRunFailed, markAutonomyRunRunning) remain imported; no other changes to logic are required.src/jobs/templates.ts (2)
81-87: Type assertion forfrontmatter.descriptionmay coerce non-string values.If
frontmatter.descriptionis a number, boolean, or other non-string type, theas stringassertion won't convert it—it will pass through as-is, potentially causing unexpected behavior in the description output.💡 Safer type handling
const description = - (frontmatter.description as string) || + (typeof frontmatter.description === 'string' ? frontmatter.description : null) || content .split('\n') .find(l => l.trim().length > 0) ?.trim() || 'No description'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/templates.ts` around lines 81 - 87, The current assignment for description uses a type assertion "(frontmatter.description as string)" which can incorrectly pass through non-string values; replace the assertion with a safe check and conversion: test typeof frontmatter.description === 'string' and use it, otherwise coerce non-null values with String(frontmatter.description) (or ignore null/undefined and fall back to the content-first line or 'No description'), removing the "as string" assertion to ensure description is always a proper string; apply this change where description is computed (frontmatter.description, content).
102-105:loadTemplatere-discovers all templates on every call.
loadTemplatecallslistTemplates()which performs full filesystem traversal and file parsing each time. This is acceptable for occasional CLI usage but could be optimized with caching if called frequently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/templates.ts` around lines 102 - 105, The loadTemplate function repeatedly calls listTemplates() which re-traverses and parses the filesystem on every invocation; change loadTemplate to use a cached in-memory template index: add a module-level cache (e.g., cachedTemplates or templatesCache) populated by a single call to listTemplates() and used by loadTemplate (and expose a refreshTemplates or invalidateTemplates function to refresh the cache when templates change). Update loadTemplate to read from that cache (falling back to populate the cache on first access) rather than calling listTemplates() on every call; reference function names loadTemplate, listTemplates, and the new cache/refresh helpers when making the change.src/daemon/__tests__/state.test.ts (2)
17-27: Memoize cache clearing relies on lodash internals.The cache clearing pattern
(getClaudeConfigHomeDir as any).cache.clear?.()depends on lodash's memoize implementation details, which could change between versions.This is acceptable for tests but worth noting as a maintenance concern if lodash is upgraded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/daemon/__tests__/state.test.ts` around lines 17 - 27, The test's current cache-clearing uses lodash internals via getClaudeConfigHomeDir.cache.clear?.(), which is brittle across lodash versions; change the test to avoid relying on internals by either (A) calling a dedicated exported helper to clear the memoize cache (e.g., export and call clearClaudeConfigHomeDirCache that invokes getClaudeConfigHomeDir.cache.clear()), or (B) resetting module state via jest.resetModules()/delete require.cache and re-requiring the module before each test; update the test to use the chosen approach and reference getClaudeConfigHomeDir when implementing the helper or module reset so cache clearing is stable and not dependent on lodash internals.
170-184: Hardcoded PID 999999 could theoretically be a live process.While PID 999999 is unlikely to exist on most systems, it's not guaranteed to be dead. On some configurations (especially containers or systems with high PID wrap-around), this could cause flaky tests.
Consider using a more robust approach to get a guaranteed-dead PID.
💡 Suggested improvement
test('returns stale when PID is dead and cleans up', () => { + // Use a negative PID or spawn-and-kill a process to guarantee a dead PID + // Negative PIDs are invalid on all POSIX systems writeDaemonState( { - pid: 999999, + pid: -1, cwd: '/', startedAt: '', workerKinds: [], lastStatus: 'running', }, 'stale-test', )Alternatively, spawn a subprocess and wait for it to exit, then use its PID.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/daemon/__tests__/state.test.ts` around lines 170 - 184, The test currently uses a hardcoded PID (999999) which might be valid on some systems; instead, spawn a short-lived subprocess, capture its PID, wait for it to exit so you have a guaranteed-dead PID, then call writeDaemonState with that PID and assert queryDaemonStatus('stale-test') returns 'stale' and the state file is removed (use getDaemonStateFilePath to check). Update the test to use the spawned process PID and ensure you await the child's exit before invoking queryDaemonStatus; keep references to writeDaemonState, queryDaemonStatus, and getDaemonStateFilePath so the change is localized.src/cli/handlers/templateJobs.ts (1)
2-8: Use the projectsrc/alias for these imports.This new file mixes in relative internal imports instead of the repo-standard alias form.
♻️ Suggested diff
-import { listTemplates, loadTemplate } from '../../jobs/templates.js' +import { listTemplates, loadTemplate } from 'src/jobs/templates.js' import { createJob, readJobState, appendJobReply, getJobDir, -} from '../../jobs/state.js' +} from 'src/jobs/state.js'As per coding guidelines, "Use
src/path alias for imports; valid paths likeimport { ... } from 'src/utils/...'".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/handlers/templateJobs.ts` around lines 2 - 8, Imports in this file use relative paths; replace them with the repo-standard src/ alias by changing the two import statements that bring in listTemplates and loadTemplate and the imports for createJob, readJobState, appendJobReply, and getJobDir so they import from 'src/jobs/templates' and 'src/jobs/state' respectively; update only the module specifiers (not the imported symbol names) to use the src/ path alias.src/commands/autonomy.ts (1)
30-34: Consider lazy-loading runs/flows based on subcommand.Pre-fetching both
runsandflowson every invocation is inefficient. Most subcommands only need one dataset (e.g.,runssubcommand doesn't need flows,flow <id>doesn't need the full runs list). Consider fetching only what's needed per subcommand.♻️ Suggested refactor
const call: LocalCommandCall = async (args: string) => { const [subcommand = 'status', arg1, arg2] = args.trim().split(/\s+/, 3) - const runs = await listAutonomyRuns() - const flows = await listAutonomyFlows() if (subcommand === 'runs') { + const runs = await listAutonomyRuns() return { type: 'text', value: formatAutonomyRunsList(runs, parseRunsLimit(arg1)), } } if (subcommand === 'flows') { + const flows = await listAutonomyFlows() return { type: 'text', value: formatAutonomyFlowsList(flows, parseRunsLimit(arg1)), } } // ... similar changes for other subcommands + + // Only fetch both for 'status' + const runs = await listAutonomyRuns() + const flows = await listAutonomyFlows() return { type: 'text', value: [formatAutonomyRunsStatus(runs), formatAutonomyFlowsStatus(flows)].join('\n'), } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/autonomy.ts` around lines 30 - 34, The handler function call currently eagerly awaits listAutonomyRuns() and listAutonomyFlows() for every invocation; instead, inspect the parsed subcommand (variable subcommand and args arg1/arg2) and only call await listAutonomyRuns() when handling subcommands that require runs (e.g., 'runs', 'status', 'run <id>') and only call await listAutonomyFlows() when handling flow-related subcommands (e.g., 'flows', 'flow <id>'). Move the awaits into the respective case branches or conditional blocks so the code fetches runs or flows lazily based on subcommand, leaving the parsing of subcommand/arg1/arg2 intact.src/utils/autonomyAuthority.ts (1)
136-305: Hand-rolled YAML parser is fragile.This custom YAML-like parser works for the expected format but is brittle. It doesn't handle:
- Quoted strings with colons
- Multi-line strings
- Comments
- Complex YAML features
Consider using a proper YAML parser library (e.g.,
js-yaml) if the format may evolve, or at minimum document the supported subset explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/autonomyAuthority.ts` around lines 136 - 305, The hand-rolled YAML parser in parseHeartbeatAuthorityTasks is brittle; replace it with a proper YAML parse and validation using a library like js-yaml (e.g., yaml.load/safeLoad) inside parseHeartbeatAuthorityTasks: import the YAML parser, parse the content into an object, validate the expected shape (presence of tasks array and each task having name, interval, prompt, optional steps with name/prompt/waitFor), and map that object to HeartbeatAuthorityTask[] (returning [] or throwing a clear error on parse/validation failure); ensure you remove or bypass the internal parseSteps logic and add try/catch around yaml parsing to log/handle syntax errors gracefully.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74355c2d-6d97-47e6-8666-b259b1392ef0
📒 Files selected for processing (89)
.gitignore02-kairos (1).mdbuild.tsdocs/features/stub-recovery-design-1-4.mddocs/task/task-001-daemon-status-stop.mddocs/task/task-002-bg-sessions-ps-logs-kill.mddocs/task/task-003-templates-job-mvp.mddocs/task/task-004-assistant-session-attach.mddocs/test-plans/openclaw-autonomy-baseline.mdpackages/@ant/computer-use-mcp/src/toolCalls.tspackages/builtin-tools/src/tools/PushNotificationTool/PushNotificationTool.tspackages/builtin-tools/src/tools/SendUserFileTool/SendUserFileTool.tsscripts/dev.tssrc/__tests__/context.baseline.test.tssrc/assistant/AssistantSessionChooser.tssrc/assistant/AssistantSessionChooser.tsxsrc/assistant/gate.tssrc/assistant/index.tssrc/assistant/sessionDiscovery.tssrc/cli/bg.tssrc/cli/handlers/ant.tssrc/cli/handlers/templateJobs.tssrc/cli/print.tssrc/cli/rollback.tssrc/cli/up.tssrc/commands.tssrc/commands/__tests__/autonomy.test.tssrc/commands/__tests__/proactive.baseline.test.tssrc/commands/assistant/assistant.tssrc/commands/assistant/assistant.tsxsrc/commands/assistant/gate.tssrc/commands/autonomy.tssrc/commands/init.tssrc/commands/lang/index.tssrc/commands/lang/lang.tssrc/commands/send/send.tssrc/commands/torch.tssrc/daemon/__tests__/state.test.tssrc/daemon/main.tssrc/daemon/state.tssrc/hooks/useAwaySummary.tssrc/hooks/useMasterMonitor.tssrc/hooks/usePipeIpc.tssrc/hooks/usePipeMuteSync.tssrc/hooks/usePipePermissionForward.tssrc/hooks/usePipeRelay.tssrc/hooks/useScheduledTasks.tssrc/jobs/__tests__/classifier.test.tssrc/jobs/__tests__/state.test.tssrc/jobs/__tests__/templates.test.tssrc/jobs/classifier.tssrc/jobs/state.tssrc/jobs/templates.tssrc/main.tsxsrc/proactive/__tests__/state.baseline.test.tssrc/proactive/useProactive.tssrc/screens/REPL.tsxsrc/services/analytics/growthbook.tssrc/services/api/openai/__tests__/queryModelOpenAI.isolated.tssrc/services/api/openai/__tests__/queryModelOpenAI.test.tssrc/services/api/openai/__tests__/streamAdapter.test.tssrc/services/awaySummary.tssrc/services/langfuse/__tests__/langfuse.isolated.tssrc/services/langfuse/__tests__/langfuse.test.tssrc/tasks/InProcessTeammateTask/InProcessTeammateTask.tsxsrc/tasks/InProcessTeammateTask/types.tssrc/types/textInputTypes.tssrc/utils/__tests__/autonomyAuthority.test.tssrc/utils/__tests__/autonomyRuns.test.tssrc/utils/__tests__/cronScheduler.baseline.test.tssrc/utils/__tests__/cronTasks.baseline.test.tssrc/utils/__tests__/language.test.tssrc/utils/__tests__/pipeMuteState.test.tssrc/utils/__tests__/taskSummary.test.tssrc/utils/autonomyAuthority.tssrc/utils/autonomyFlows.tssrc/utils/autonomyPersistence.tssrc/utils/autonomyRuns.tssrc/utils/config.tssrc/utils/handlePromptSubmit.tssrc/utils/language.tssrc/utils/pipeMuteState.tssrc/utils/pipePermissionRelay.tssrc/utils/pipeTransport.tssrc/utils/swarm/inProcessRunner.tssrc/utils/swarm/spawnInProcess.tssrc/utils/taskSummary.tstests/mocks/file-system.tstsconfig.json
💤 Files with no reviewable changes (3)
- src/assistant/AssistantSessionChooser.ts
- src/commands/assistant/assistant.ts
- src/hooks/useAwaySummary.ts
| async call(input: SendUserFileInput, context) { | ||
| const { file_path } = input | ||
| const { stat } = await import('fs/promises') | ||
|
|
||
| // Verify file exists and is readable | ||
| let fileSize: number | ||
| try { | ||
| const fileStat = await stat(file_path) | ||
| if (!fileStat.isFile()) { | ||
| return { | ||
| data: { sent: false, file_path, error: 'Path is not a file.' }, | ||
| } | ||
| } | ||
| fileSize = fileStat.size | ||
| } catch { | ||
| return { | ||
| data: { sent: false, file_path, error: 'File does not exist or is not readable.' }, | ||
| } |
There was a problem hiding this comment.
Reject relative paths before calling stat().
The schema/prompt says file_path must be absolute, but this accepts relative paths and resolves them from the current working directory. That makes the target file process-state-dependent and can send the wrong file.
💡 Suggested fix
async call(input: SendUserFileInput, context) {
const { file_path } = input
+ const { isAbsolute } = await import('node:path')
const { stat } = await import('fs/promises')
+
+ if (!isAbsolute(file_path)) {
+ return {
+ data: { sent: false, file_path, error: 'File path must be absolute.' },
+ }
+ }
// Verify file exists and is readable
let fileSize: number🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/builtin-tools/src/tools/SendUserFileTool/SendUserFileTool.ts` around
lines 73 - 90, In SendUserFileTool.call, reject non-absolute paths before
invoking stat: import and use path.isAbsolute to validate the input.file_path
and immediately return the existing error-shaped response (e.g., data: { sent:
false, file_path, error: '...'} ) when file_path is not absolute; do this check
at the top of the call function (before calling stat) so the function never
resolves relative paths from process.cwd(). Ensure you reference the same
variable name file_path and preserve the existing error response shape used
elsewhere in call.
| // Attempt bridge upload if available (so web viewers can download) | ||
| const appState = context.getAppState() | ||
| let fileUuid: string | undefined | ||
| if (appState.replBridgeEnabled) { | ||
| try { | ||
| const { uploadBriefAttachment } = await import( | ||
| '@claude-code-best/builtin-tools/tools/BriefTool/upload.js' | ||
| ) | ||
| fileUuid = await uploadBriefAttachment(file_path, fileSize, { | ||
| replBridgeEnabled: true, | ||
| signal: context.abortController.signal, | ||
| }) | ||
| } catch { | ||
| // Best-effort upload — local path is always available | ||
| } | ||
| } | ||
|
|
||
| const delivered = !appState.replBridgeEnabled || Boolean(fileUuid) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the SendUserFileTool.ts file to see the actual code and any existing feature() usage
fd -t f "SendUserFileTool.ts" --exec cat -n {} \; 2>/dev/null | head -150Repository: claude-code-best/claude-code
Length of output: 4307
🏁 Script executed:
# Also check for the import statements at the top of SendUserFileTool.ts
fd -t f "SendUserFileTool.ts" --exec head -30 {} \;Repository: claude-code-best/claude-code
Length of output: 992
🏁 Script executed:
# Search for PushNotificationTool.ts mentioned in the scratchpad as reference
fd -t f "PushNotificationTool.ts" --exec cat -n {} \; 2>/dev/nullRepository: claude-code-best/claude-code
Length of output: 5324
🏁 Script executed:
# Search for feature('BRIDGE_MODE') usage across the codebase
rg "feature\('BRIDGE_MODE'\)" --type ts --type tsx -B2 -A2Repository: claude-code-best/claude-code
Length of output: 99
Gate the bridge upload path with feature('BRIDGE_MODE').
This branch is controlled only by appState.replBridgeEnabled, so bridge-only upload code can still run in builds where the bridge feature is disabled. The delivered logic should also key off the same feature flag condition.
💡 Suggested fix
+import { feature } from 'bun:bundle'
import { z } from 'zod/v4'
@@
- if (appState.replBridgeEnabled) {
+ if (appState.replBridgeEnabled && feature('BRIDGE_MODE')) {
try {
const { uploadBriefAttachment } = await import(
'@claude-code-best/builtin-tools/tools/BriefTool/upload.js'
@@
- const delivered = !appState.replBridgeEnabled || Boolean(fileUuid)
+ const delivered =
+ appState.replBridgeEnabled && feature('BRIDGE_MODE')
+ ? Boolean(fileUuid)
+ : truePer coding guidelines for **/*.{ts,tsx}: import and use feature() from bun:bundle to control feature flags at runtime. Feature flags default to disabled and must be explicitly enabled via environment variables. The PushNotificationTool.ts file implements the correct pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/builtin-tools/src/tools/SendUserFileTool/SendUserFileTool.ts` around
lines 93 - 110, Wrap the repl bridge upload and the delivered calculation behind
the runtime feature flag by importing feature from 'bun:bundle' and checking
feature('BRIDGE_MODE') in addition to appState.replBridgeEnabled; i.e., only
attempt the upload (the block that imports uploadBriefAttachment and sets
fileUuid) when feature('BRIDGE_MODE') && appState.replBridgeEnabled, and set
delivered to !feature('BRIDGE_MODE') || !appState.replBridgeEnabled ||
Boolean(fileUuid) (or equivalent) so the delivered boolean mirrors the gated
behavior; update references to appState.replBridgeEnabled, fileUuid,
uploadBriefAttachment and the delivered computation accordingly.
| useKeybindings( | ||
| { | ||
| 'select:next': () => setFocusIndex(i => (i + 1) % sessions.length), | ||
| 'select:previous': () => setFocusIndex(i => (i - 1 + sessions.length) % sessions.length), | ||
| 'select:accept': () => onSelect(sessions[focusIndex]!.id), | ||
| }, | ||
| { context: 'Select' }, | ||
| ); |
There was a problem hiding this comment.
Guard against empty sessions array to prevent runtime errors.
If sessions is empty, line 31's sessions[focusIndex]! will be undefined, and the modulo operations on lines 29-30 will produce NaN (from % 0). This could cause a crash when the user presses Enter on an empty list.
🛡️ Proposed fix
useKeybindings(
{
- 'select:next': () => setFocusIndex(i => (i + 1) % sessions.length),
- 'select:previous': () => setFocusIndex(i => (i - 1 + sessions.length) % sessions.length),
- 'select:accept': () => onSelect(sessions[focusIndex]!.id),
+ 'select:next': () => {
+ if (sessions.length > 0) setFocusIndex(i => (i + 1) % sessions.length);
+ },
+ 'select:previous': () => {
+ if (sessions.length > 0) setFocusIndex(i => (i - 1 + sessions.length) % sessions.length);
+ },
+ 'select:accept': () => {
+ if (sessions.length > 0) onSelect(sessions[focusIndex]!.id);
+ },
},
{ context: 'Select' },
);Alternatively, if the parent component guarantees sessions.length > 0, consider adding a runtime assertion or early return with a message indicating no sessions are available.
📝 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.
| useKeybindings( | |
| { | |
| 'select:next': () => setFocusIndex(i => (i + 1) % sessions.length), | |
| 'select:previous': () => setFocusIndex(i => (i - 1 + sessions.length) % sessions.length), | |
| 'select:accept': () => onSelect(sessions[focusIndex]!.id), | |
| }, | |
| { context: 'Select' }, | |
| ); | |
| useKeybindings( | |
| { | |
| 'select:next': () => { | |
| if (sessions.length > 0) setFocusIndex(i => (i + 1) % sessions.length); | |
| }, | |
| 'select:previous': () => { | |
| if (sessions.length > 0) setFocusIndex(i => (i - 1 + sessions.length) % sessions.length); | |
| }, | |
| 'select:accept': () => { | |
| if (sessions.length > 0) onSelect(sessions[focusIndex]!.id); | |
| }, | |
| }, | |
| { context: 'Select' }, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assistant/AssistantSessionChooser.tsx` around lines 27 - 34, The
keybinding handlers in useKeybindings (the 'select:next', 'select:previous', and
'select:accept' callbacks referencing setFocusIndex and onSelect using
sessions[focusIndex]) assume sessions.length > 0 and will crash or produce NaN
when sessions is empty; fix by guarding registration or logic: only
register/return handlers when sessions.length > 0 (or early-return from the
component) and update the setFocusIndex callbacks to compute the next index only
when sessions.length > 0 (e.g., skip modulo ops if length is zero), and make
'select:accept' no-op unless sessions[focusIndex] is defined before calling
onSelect; locate useKeybindings, setFocusIndex, focusIndex, and onSelect to
implement this guard.
| void (async () => { | ||
| const commands = await createProactiveAutonomyCommands({ | ||
| basePrompt: `<${TICK_TAG}>${new Date().toLocaleTimeString()}</${TICK_TAG}>`, | ||
| currentDir: cwd(), | ||
| shouldCreate: () => !inputClosed, | ||
| }) | ||
| for (const command of commands) { | ||
| if (inputClosed) { | ||
| return | ||
| } | ||
| enqueue({ | ||
| ...command, | ||
| uuid: randomUUID(), | ||
| }) | ||
| } | ||
| void run() | ||
| })() |
There was a problem hiding this comment.
Catch proactive tick failures explicitly.
If createProactiveAutonomyCommands() throws here, this detached promise rejects without any handling. In the headless loop that turns a recoverable scheduling failure into an unhandled rejection.
💡 Suggested fix
- void (async () => {
+ void (async () => {
const commands = await createProactiveAutonomyCommands({
basePrompt: `<${TICK_TAG}>${new Date().toLocaleTimeString()}</${TICK_TAG}>`,
currentDir: cwd(),
shouldCreate: () => !inputClosed,
})
for (const command of commands) {
if (inputClosed) {
return
}
enqueue({
...command,
uuid: randomUUID(),
})
}
void run()
- })()
+ })().catch(error => {
+ logError(error)
+ })📝 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.
| void (async () => { | |
| const commands = await createProactiveAutonomyCommands({ | |
| basePrompt: `<${TICK_TAG}>${new Date().toLocaleTimeString()}</${TICK_TAG}>`, | |
| currentDir: cwd(), | |
| shouldCreate: () => !inputClosed, | |
| }) | |
| for (const command of commands) { | |
| if (inputClosed) { | |
| return | |
| } | |
| enqueue({ | |
| ...command, | |
| uuid: randomUUID(), | |
| }) | |
| } | |
| void run() | |
| })() | |
| void (async () => { | |
| const commands = await createProactiveAutonomyCommands({ | |
| basePrompt: `<${TICK_TAG}>${new Date().toLocaleTimeString()}</${TICK_TAG}>`, | |
| currentDir: cwd(), | |
| shouldCreate: () => !inputClosed, | |
| }) | |
| for (const command of commands) { | |
| if (inputClosed) { | |
| return | |
| } | |
| enqueue({ | |
| ...command, | |
| uuid: randomUUID(), | |
| }) | |
| } | |
| void run() | |
| })().catch(error => { | |
| logError(error) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/print.ts` around lines 1853 - 1869, Wrap the detached async IIFE body
in a try/catch so any rejection from createProactiveAutonomyCommands (and
subsequent logic) is handled; specifically, inside the anonymous async function
that calls createProactiveAutonomyCommands, add a try/catch around the await
createProactiveAutonomyCommands(...) and the for-loop that enqueues commands
(referencing createProactiveAutonomyCommands, enqueue, randomUUID, inputClosed,
and run), and in the catch block handle the failure (e.g., log the error and/or
enqueue a recoverable-failure event) to avoid unhandled promise rejections and
ensure run() is not invoked when creation fails.
| void (async () => { | ||
| const prepared = await prepareAutonomyTurnPrompt({ | ||
| basePrompt: prompt, | ||
| trigger: 'scheduled-task', | ||
| currentDir: cwd(), | ||
| }) | ||
| if (inputClosed) return | ||
| const command = await commitAutonomyQueuedPrompt({ | ||
| prepared, | ||
| currentDir: cwd(), | ||
| workload: WORKLOAD_CRON, | ||
| }) | ||
| if (inputClosed) return | ||
| enqueue({ | ||
| ...command, | ||
| uuid: randomUUID(), | ||
| }) | ||
| void run() | ||
| })() | ||
| }, | ||
| onFireTask: task => { | ||
| if (inputClosed) return | ||
| void (async () => { | ||
| if (task.agentId) { | ||
| const prepared = await prepareAutonomyTurnPrompt({ | ||
| basePrompt: task.prompt, | ||
| trigger: 'scheduled-task', | ||
| currentDir: cwd(), | ||
| }) | ||
| if (inputClosed) return | ||
| const command = await commitAutonomyQueuedPrompt({ | ||
| prepared, | ||
| currentDir: cwd(), | ||
| sourceId: task.id, | ||
| sourceLabel: task.prompt, | ||
| workload: WORKLOAD_CRON, | ||
| }) | ||
| await markAutonomyRunFailed( | ||
| command.autonomy!.runId, | ||
| `No teammate runtime available for scheduled task owner ${task.agentId} in headless mode.`, | ||
| ) | ||
| return | ||
| } | ||
| const prepared = await prepareAutonomyTurnPrompt({ | ||
| basePrompt: task.prompt, | ||
| trigger: 'scheduled-task', | ||
| currentDir: cwd(), | ||
| }) | ||
| if (inputClosed) return | ||
| const command = await commitAutonomyQueuedPrompt({ | ||
| prepared, | ||
| currentDir: cwd(), | ||
| sourceId: task.id, | ||
| sourceLabel: task.prompt, | ||
| workload: WORKLOAD_CRON, | ||
| }) | ||
| if (inputClosed) return | ||
| enqueue({ | ||
| ...command, | ||
| uuid: randomUUID(), | ||
| }) | ||
| void run() | ||
| })() |
There was a problem hiding this comment.
Handle cron callback rejections instead of dropping them.
Both scheduler callbacks spin off async work with void (async () => ...)(). Any failure in prepareAutonomyTurnPrompt() / commitAutonomyQueuedPrompt() becomes an unhandled rejection instead of a logged scheduler error.
💡 Suggested fix
- void (async () => {
+ void (async () => {
const prepared = await prepareAutonomyTurnPrompt({
basePrompt: prompt,
trigger: 'scheduled-task',
currentDir: cwd(),
})
if (inputClosed) return
const command = await commitAutonomyQueuedPrompt({
prepared,
currentDir: cwd(),
workload: WORKLOAD_CRON,
})
if (inputClosed) return
enqueue({
...command,
uuid: randomUUID(),
})
void run()
- })()
+ })().catch(error => {
+ logError(error)
+ })Apply the same pattern to onFireTask.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/print.ts` around lines 2758 - 2820, The onFireTask callback currently
spawns an async IIFE with void (async () => { ... })() causing any thrown errors
in prepareAutonomyTurnPrompt/commitAutonomyQueuedPrompt to become unhandled
rejections; change the IIFE invocation so its returned Promise is observed and
add a catch handler that logs the error (same pattern used elsewhere), e.g.
invoke the IIFE without void and append .catch(...) or await it in a try/catch;
ensure both branches inside onFireTask (the agentId branch that calls
markAutonomyRunFailed and the normal branch that enqueues the command) are
covered by this rejection handler and reference prepareAutonomyTurnPrompt,
commitAutonomyQueuedPrompt, onFireTask, markAutonomyRunFailed and enqueue when
locating and updating the code.
| void onQuery([userMessage], newAbortController, true, [], mainLoopModel) | ||
| .then(() => { | ||
| if (autonomyRunId) { | ||
| void finalizeAutonomyRunCompleted({ | ||
| runId: autonomyRunId, | ||
| currentDir: getCwd(), | ||
| priority: 'later', | ||
| }).then(nextCommands => { | ||
| for (const command of nextCommands) { | ||
| enqueue(command); | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Don't finalize aborted autonomy runs as completed.
This then branch runs on any fulfilled onQuery(), including interrupted turns that were aborted but didn't reject. In that case the run gets marked completed and nextCommands are enqueued even though the turn was canceled. Gate the completion path on !newAbortController.signal.aborted and finalize the run as failed/canceled otherwise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/REPL.tsx` around lines 4854 - 4866, The current then-branch after
calling onQuery(...) always finalizes the autonomy run as completed and enqueues
nextCommands even when the turn was aborted; change the logic in the
onQuery(...).then(...) handler to check newAbortController.signal.aborted: if
not aborted (i.e. !newAbortController.signal.aborted) proceed to call
finalizeAutonomyRunCompleted({ runId: autonomyRunId, currentDir: getCwd(),
priority: 'later' }) and enqueue returned commands via enqueue(...); otherwise,
mark the run as failed or canceled (invoke the appropriate failure/cancel path
instead of finalizeAutonomyRunCompleted) so aborted turns do not get treated as
completed or enqueue nextCommands (use the existing autonomyRunId,
finalizeAutonomyRunCompleted, enqueue, and newAbortController references to
locate where to change the behavior).
| void finalizeAutonomyRunCompleted({ | ||
| runId: autonomyRunId, | ||
| currentDir: getCwd(), | ||
| priority: 'later', | ||
| }).then(nextCommands => { | ||
| for (const command of nextCommands) { | ||
| enqueue(command); | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
| .catch((error: unknown) => { | ||
| if (autonomyRunId) { | ||
| void finalizeAutonomyRunFailed({ | ||
| runId: autonomyRunId, | ||
| error: String(error), | ||
| }) | ||
| } | ||
| logError(toError(error)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the REPL.tsx file
fd -t f REPL.tsxRepository: claude-code-best/claude-code
Length of output: 92
🏁 Script executed:
# Check the imports at the top of REPL.tsx
head -100 src/screens/REPL.tsx | cat -nRepository: claude-code-best/claude-code
Length of output: 4536
🏁 Script executed:
# Search for getCwd and toError usage in the file
rg "getCwd|toError" src/screens/REPL.tsx -nRepository: claude-code-best/claude-code
Length of output: 152
🏁 Script executed:
# Search for getOriginalCwd definition in the codebase
rg "getOriginalCwd|export.*getCwd|export.*toError" -t ts -t tsxRepository: claude-code-best/claude-code
Length of output: 99
🏁 Script executed:
# Search for all imports in REPL.tsx to find getCwd, toError, logError
rg "import.*\{.*getCwd|import.*toError|import.*logError" src/screens/REPL.tsxRepository: claude-code-best/claude-code
Length of output: 115
🏁 Script executed:
# Check if getCwd, toError, logError are defined anywhere in the codebase
rg "export.*getCwd|export.*toError|export.*logError" --type tsRepository: claude-code-best/claude-code
Length of output: 1495
🏁 Script executed:
# Let's search the broader imports section of REPL.tsx
sed -n '1,150p' src/screens/REPL.tsx | cat -nRepository: claude-code-best/claude-code
Length of output: 7754
🏁 Script executed:
# Search entire REPL.tsx for any imports of getCwd or toError
rg "getCwd|toError" src/screens/REPL.tsx | grep -i importRepository: claude-code-best/claude-code
Length of output: 54
🏁 Script executed:
# Also look for where these functions are actually used, with context
rg "getCwd\(\)|toError\(" src/screens/REPL.tsx -B 5 -A 5Repository: claude-code-best/claude-code
Length of output: 757
🏁 Script executed:
# Let's check the full import statements - get lines 1-200 to see all imports
sed -n '1,200p' src/screens/REPL.tsx | tail -50 | cat -nRepository: claude-code-best/claude-code
Length of output: 3571
🏁 Script executed:
# Run TypeScript type checking to see actual errors
cd src/screens && bunx tsc REPL.tsx --noEmit 2>&1 | head -50Repository: claude-code-best/claude-code
Length of output: 114
🏁 Script executed:
# Check the getCwd and getOriginalCwd definitions
cat src/utils/cwd.tsRepository: claude-code-best/claude-code
Length of output: 1056
🏁 Script executed:
# Also check getOriginalCwd in state.ts
rg "getOriginalCwd" src/bootstrap/state.ts -A 5Repository: claude-code-best/claude-code
Length of output: 553
🏁 Script executed:
# Check the toError function definition
cat src/utils/errors.tsRepository: claude-code-best/claude-code
Length of output: 7702
Add missing imports for getCwd and toError to fix TypeScript strict mode errors.
getCwd() on line 4859 and toError() on line 4875 are not imported. Both exist and should be imported directly:
import { getCwd } from '../utils/cwd.js'import { toError } from '../utils/errors.js'
Do not replace getCwd() with getOriginalCwd(). The getCwd() function handles async context overrides via cwdOverrideStorage, which getOriginalCwd() does not. For autonomy runs, the override-aware version is more correct. Similarly, import toError() rather than inlining the equivalent logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/REPL.tsx` around lines 4857 - 4875, The code is missing imports
for getCwd and toError used in the autonomy run handling; add an import for
getCwd from '../utils/cwd.js' and an import for toError from
'../utils/errors.js' so calls to getCwd() (used when finalizing runs via
finalizeAutonomyRunCompleted) and toError(error) (used in the catch block when
logging errors) resolve under TypeScript strict mode—do not swap getCwd() for
getOriginalCwd(), keep getCwd() as-is.
| if (pending.autonomyRunId) { | ||
| await markAutonomyRunRunning(pending.autonomyRunId) | ||
| } | ||
| return { | ||
| type: 'new_message', | ||
| message, | ||
| message: pending.message, | ||
| autonomyRunId: pending.autonomyRunId, | ||
| from: 'user', |
There was a problem hiding this comment.
Let the caller own the autonomy-run state transition.
If markAutonomyRunRunning() throws here, runInProcessTeammate() has not assigned currentAutonomyRunId yet, so the outer catch can't mark that run failed. Move the running transition to the new_message branch after the ID is captured.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/swarm/inProcessRunner.ts` around lines 740 - 747, The call to
markAutonomyRunRunning() can throw before runInProcessTeammate() sets
currentAutonomyRunId so the outer catch cannot mark the run failed; move the
transition into the new_message branch after the autonomyRunId is captured.
Specifically, in runInProcessTeammate, remove or defer calling
markAutonomyRunRunning() where it currently executes, and instead invoke
markAutonomyRunRunning(pending.autonomyRunId) immediately after you set/return
the new_message payload that contains pending.autonomyRunId (i.e., once
currentAutonomyRunId is assigned), ensuring you still return type:'new_message'
with message and autonomyRunId and call markAutonomyRunRunning only after the ID
is owned by the caller.
| const pendingAutonomyRunIds = teammateTask.pendingUserMessages | ||
| .map(message => message.autonomyRunId) | ||
| .filter((runId): runId is string => Boolean(runId)) | ||
| for (const runId of pendingAutonomyRunIds) { | ||
| void markAutonomyRunFailed( | ||
| runId, | ||
| `Teammate ${agentId ?? taskId} was stopped before it could consume the queued autonomy prompt.`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
teammateTask is out of scope — this code will fail at runtime.
The variable teammateTask is defined inside the setAppState callback (line 244), but this code block is outside that callback. When killed is true, accessing teammateTask.pendingUserMessages will throw a ReferenceError.
You need to capture the pending messages inside the callback before it returns:
🐛 Proposed fix: capture pending messages inside the callback
let killed = false
let teamName: string | null = null
let agentId: string | null = null
let toolUseId: string | undefined
let description: string | undefined
+ let pendingAutonomyRunIds: string[] = []
setAppState((prev: AppState) => {
const task = prev.tasks[taskId]
if (!task || task.type !== 'in_process_teammate') {
return prev
}
const teammateTask = task as InProcessTeammateTaskState
if (teammateTask.status !== 'running') {
return prev
}
// Capture identity for cleanup after state update
teamName = teammateTask.identity.teamName
agentId = teammateTask.identity.agentId
toolUseId = teammateTask.toolUseId
description = teammateTask.description
+ pendingAutonomyRunIds = teammateTask.pendingUserMessages
+ .map(message => message.autonomyRunId)
+ .filter((runId): runId is string => Boolean(runId))
// ... rest of callback
})
// ... after setAppState
if (killed) {
- const pendingAutonomyRunIds = teammateTask.pendingUserMessages
- .map(message => message.autonomyRunId)
- .filter((runId): runId is string => Boolean(runId))
for (const runId of pendingAutonomyRunIds) {
void markAutonomyRunFailed(
runId,
`Teammate ${agentId ?? taskId} was stopped before it could consume the queued autonomy prompt.`,
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/swarm/spawnInProcess.ts` around lines 308 - 316, The code accesses
teammateTask.pendingUserMessages outside the setAppState callback where
teammateTask is declared, causing a runtime ReferenceError; capture the pending
messages inside the setAppState callback (e.g., read
teammateTask.pendingUserMessages into a local array like pendingAutonomyMessages
or pendingAutonomyRunIds inside the callback) and then use that captured array
in the outer killed handling block to call markAutonomyRunFailed(runId,
`Teammate ${agentId ?? taskId} was stopped...`), ensuring you reference the same
runId extraction logic (map -> filter runId is string) but only after it's
safely captured.
| .reverse() | ||
| .find(m => m.type === 'assistant') | ||
|
|
||
| let status = 'working' |
There was a problem hiding this comment.
Type mismatch: 'working' is cast to 'busy' | 'idle'.
Line 51 assigns status = 'working', but line 70 casts it to 'busy' | 'idle'. The value 'working' is not part of the union type, which means the API contract for updateSessionActivity may be violated.
🐛 Proposed fix: align status values with the expected type
- let status = 'working'
+ let status: 'busy' | 'idle' = 'idle'
let waitingFor: string | undefined
if (lastAssistant?.message?.content) {
@@ -69,7 +69,7 @@
// Fire-and-forget update to session registry
void updateSessionActivity({
- status: status as 'busy' | 'idle',
+ status,
waitingFor,
}).catch(err => {If 'working' is a valid status, update the type union to include it; otherwise, use 'idle' as the default.
Also applies to: 69-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/taskSummary.ts` at line 51, The variable status is initialized to
'working' but later cast to the union 'busy' | 'idle', causing a type mismatch;
locate the status variable in taskSummary.ts and either change its initial value
from 'working' to 'idle' (or 'busy' as appropriate) or expand the allowed union
type where updateSessionActivity is defined/used to include 'working' (and
update any callers/types accordingly), ensuring consistency between the status
variable and the type expected by updateSessionActivity.
1 participant
Summary
detectMimeFromBase64()compared raw byte magic numbers against base64-encoded characters — no condition ever matched, always returningimage/pngformat='JPEG', quality=75), causing API 400: "image was specified using image/png but appears to be image/jpeg"Changes
packages/@ant/computer-use-mcp/src/toolCalls.ts:89 50 4E 47(4-byte signature)FF D8 FF(3-byte, all JFIF/EXIF/DQT variants)RIFF(bytes 0-3) +WEBP(bytes 8-11) dual check47 49 46(covers GIF87a and GIF89a)Verification
tsc --noEmitzero new errorsTest plan
image/jpegSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests