feat: integrate 5 feature branches + daemon/job 命令层级化 + 跨平台后台引擎#259
Closed
amDosion wants to merge 1 commit intoclaude-code-best:mainfrom
Closed
feat: integrate 5 feature branches + daemon/job 命令层级化 + 跨平台后台引擎#259amDosion wants to merge 1 commit intoclaude-code-best:mainfrom
amDosion wants to merge 1 commit intoclaude-code-best:mainfrom
Conversation
…eScript 错误修复 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 Additional: 6. daemon/job 命令层级化重构 (subcommand 架构) 7. 跨平台后台引擎抽象 (detached/tmux engines) 8. 修复 src/ 中 43 个预存在的 TypeScript 类型错误 9. 修复 langfuse isolated test mock 完整性 10. 修复 CodeRabbit 审查的 Critical/Major/Minor 问题 11. remote-control-server logger 抽象 (测试 stderr 静默化) 12. /simplify 审查修复 (代码复用、质量、效率)
Contributor
📝 WalkthroughWalkthroughThis PR implements stub recovery for the daemon/background-session system, template jobs, assistant mode activation, and comprehensive autonomy infrastructure including run tracking, managed flows, authority loading, and language support. It adds cross-platform background-session engine abstraction, restructures daemon commands into a unified Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical 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 (5)
packages/builtin-tools/src/tools/SendUserFileTool/SendUserFileTool.ts (1)
21-21:⚠️ Potential issue | 🟡 MinorReturn type is incomplete.
The
SendUserFileOutputtype only defines{ sent: boolean; file_path: string }, but the actual return includessize,file_uuid, anderrorproperties. Consider extending the type to match the runtime shape.🛠️ Suggested fix
-type SendUserFileOutput = { sent: boolean; file_path: string } +type SendUserFileOutput = { + sent: boolean + file_path: string + size?: number + file_uuid?: string + error?: string +}Also applies to: 112-118
🤖 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` at line 21, The declared type SendUserFileOutput is missing fields returned at runtime (size, file_uuid, and error); update the SendUserFileOutput type to include size: number, file_uuid?: string, and error?: string (or make fields optional/nullable as appropriate) so it matches the actual return shape used by the SendUserFileTool implementation and any callers; ensure any code referencing SendUserFileOutput (e.g., the send/execute function in SendUserFileTool) is still type-correct after adding these fields.packages/builtin-tools/src/tools/PushNotificationTool/PushNotificationTool.ts (1)
27-27:⚠️ Potential issue | 🟡 MinorReturn type does not include
errorproperty.The
PushOutputtype at Line 27 is{ sent: boolean }, but Line 131 returns an object with an additionalerrorproperty. While this works at runtime due to structural typing, it creates a type inconsistency.🛠️ Suggested fix
Update the type definition to include the optional error field:
-type PushOutput = { sent: boolean } +type PushOutput = { sent: boolean; error?: string }Also applies to: 131-131
🤖 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` at line 27, The PushOutput type is missing the optional error property but code returns { sent, error } at runtime; update the PushOutput type definition to include an optional error field (e.g., change type PushOutput = { sent: boolean; error?: string } or error?: any) and ensure any return sites (such as the function/method that returns at the spot referenced around line 131) conform to the updated shape.packages/remote-control-server/src/services/work-dispatch.ts (1)
83-83:⚠️ Potential issue | 🟡 MinorAvoid
as anyin production code.Per coding guidelines,
as anyis prohibited in production code. The comment indicates this is just to bumpupdatedAt, but the type bypass may mask issues.🛠️ Suggested fix
Consider updating the
storeUpdateWorkItemsignature to accept an empty update, or use a more specific type:- storeUpdateWorkItem(workId, {} as any); // just bump updatedAt + storeUpdateWorkItem(workId, {} as Record<string, never>); // just bump updatedAtOr better, if
storeUpdateWorkItemrequires at least one field, add a dedicatedtouchWorkItem(workId)function that explicitly bumpsupdatedAt.As per coding guidelines: "Prohibit
as anyin production code; test files may useas anyfor mock data."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/remote-control-server/src/services/work-dispatch.ts` at line 83, Replace the prohibited `as any` usage by adding an explicit “touch” API or relaxing the update signature: either add a new function touchWorkItem(workId) that calls storeUpdateWorkItem(workId, { updatedAt: <current-timestamp> }) to explicitly bump updatedAt, or change storeUpdateWorkItem’s parameter type to accept an empty/partial update (e.g., Partial<WorkUpdate>) so you can call storeUpdateWorkItem(workId, {}) without type-casting; update call sites to use touchWorkItem or the new signature and remove the `as any` cast.src/utils/handlePromptSubmit.ts (1)
469-486:⚠️ Potential issue | 🟠 MajorDeduplicate autonomy run IDs before marking/finalizing them.
If this batch contains multiple commands for the same run, the same
runIdis appended multiple times andfinalizeAutonomyRunCompleted()will run multiple times, which can enqueue duplicate follow-up commands.Suggested fix
- let autonomyRunIds: string[] | undefined + let autonomyRunIds: Set<string> | undefined ... if (cmd.autonomy?.runId) { - ;(autonomyRunIds ??= []).push(cmd.autonomy.runId) - await markAutonomyRunRunning(cmd.autonomy.runId) + const runId = cmd.autonomy.runId + const seen = (autonomyRunIds ??= new Set()).has(runId) + autonomyRunIds.add(runId) + if (!seen) { + await markAutonomyRunRunning(runId) + } } ... - if (autonomyRunIds?.length) { + if (autonomyRunIds?.size) { for (const runId of autonomyRunIds) { const nextCommands = await finalizeAutonomyRunCompleted({ runId, priority: 'later', workload: turnWorkload,Also applies to: 609-619
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/handlePromptSubmit.ts` around lines 469 - 486, The batch may push duplicate autonomy run IDs into autonomyRunIds causing markAutonomyRunRunning and later finalizeAutonomyRunCompleted to run multiple times; change collection to deduplicate run IDs (e.g., use a Set or check membership) when adding inside the runWithWorkload loop (refer to autonomyRunIds and the loop iterating commands) and when finalizing (where finalizeAutonomyRunCompleted is invoked) iterate only the unique run IDs so each runId is marked/finalized exactly once; apply the same deduplication logic to the other occurrence around lines 609-619.src/daemon/main.ts (1)
257-267:⚠️ Potential issue | 🟠 MajorRemove the state file after shutdown completes, not when it starts.
removeDaemonState()now runs as soon as SIGTERM/SIGINT is received, soclaude daemon statuscan report "stopped" while the supervisor is still draining workers. That creates a race where another CLI process can start a second daemon before the first one has actually exited.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/daemon/main.ts` around lines 257 - 267, The shutdown function currently calls removeDaemonState() immediately; instead, delay removing the state file until after the supervisor and all workers have fully exited. Modify shutdown (the function that calls controller.abort() and iterates over workers) to first signal termination (controller.abort(), send SIGTERM to each w.process), then await worker termination (e.g., Promise.all of each worker.process 'exit' or a waitForExit helper on the workers collection) and only after all workers have exited and the supervisor is drained call removeDaemonState(); ensure removeDaemonState() is not invoked synchronously before waiting and that any existing helper like workers or their process event handlers are used to detect real exit.
🟠 Major comments (22)
src/services/analytics/growthbook.ts-469-470 (1)
469-470:⚠️ Potential issue | 🟠 MajorAlign local-gate precedence across cached and blocking readers.
getFeatureValue_CACHED_MAY_BE_STALE()now hard-overrides remote/disk values withLOCAL_GATE_DEFAULTS, butgetFeatureValueInternal()andcheckGate_CACHED_OR_BLOCKING()still only use those defaults when GrowthBook is unavailable. That makes the same gate resolve differently by call path; for example,tengu_kairos_assistantwill readtruein sync startup code while blocking paths can still return the server value. Either keep local defaults as fallback-only here, or mirror the same precedence in the blocking helpers too.Also applies to: 834-842
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/analytics/growthbook.ts` around lines 469 - 470, Cached reader getFeatureValue_CACHED_MAY_BE_STALE currently hard-overrides remote/disk gate values with LOCAL_GATE_DEFAULTS while getFeatureValueInternal() and checkGate_CACHED_OR_BLOCKING() only apply LOCAL_GATE_DEFAULTS as a fallback, causing inconsistent gate resolution by call path; update getFeatureValueInternal and checkGate_CACHED_OR_BLOCKING to mirror the cached helper’s precedence by applying LOCAL_GATE_DEFAULTS as an override of remote/disk values (i.e., merge/apply LOCAL_GATE_DEFAULTS onto the fetched value before returning) so all resolution paths (including blocking helpers) return the same value, and apply the same change to the other occurrence referenced around the block noted (the similar logic at the other location).src/cli/handlers/ant.ts-198-215 (1)
198-215:⚠️ Potential issue | 🟠 MajorHonor
opts.outputor remove the option.Both branches do the same thing, and neither writes anything to the requested path. As written,
--outputis accepted but ignored.🤖 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 - 215, The completionHandler currently ignores opts.output; update it so when opts.output is provided it writes the regenerated completion cache to that path instead of just logging. Call regenerateCompletionCache() to obtain the cache content (or pass the path to it if the helper supports that), then write the content to the file path from opts.output using the Node fs API (ensuring directories exist or failing with a clear error) and log a message including the target path; when opts.output is not provided, keep the current behavior of regenerating and printing to stdout. Ensure you modify the completionHandler function and reference opts.output and regenerateCompletionCache accordingly.src/cli/handlers/ant.ts-147-160 (1)
147-160:⚠️ Potential issue | 🟠 Major
errorHandler()never narrows to erroring sessions.Right now it prints the first
Nentries fromgetRecentActivity()regardless of whether those sessions actually failed, soclaude errorbehaves like a renamed recent-sessions command.🤖 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 147 - 160, errorHandler currently lists the first N recent sessions instead of only those that errored; change it to filter the array returned by getRecentActivity() for error sessions (check properties such as log.error, log.errors?.length > 0, or log.status === 'failed' on each log object) before taking the first count, then iterate over that filtered list and print sessionId and modified as before; also handle the case where the filtered list is empty by printing a "no error sessions" message. Ensure you update references to logs, log.modified and log.sessionId inside errorHandler accordingly.src/cli/rollback.ts-15-30 (1)
15-30:⚠️ Potential issue | 🟠 MajorDon't return success for the stubbed
--list/--safepaths.Both branches advertise a supported mode, make no change, and still exit 0. That makes callers treat a no-op as a successful rollback/list operation.
Proposed minimal guard
if (options?.list) { console.log('Recent versions:') console.log(' (version listing requires access to the release registry)') console.log(' Use `claude update --list` for available versions.') + process.exitCode = 1 return } @@ if (options?.safe) { console.log('Safe rollback: would install the server-pinned safe version.') if (options.dryRun) { console.log(' (dry run — no changes made)') return } console.log(' Safe version pinning requires access to the release API.') console.log(' Contact oncall for the current safe version.') + process.exitCode = 1 return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/rollback.ts` around lines 15 - 30, The stubbed branches for options?.list and options?.safe currently print messages then return success (bare return), which makes callers treat no-op as success; update both branches (the blocks around the options?.list and options?.safe checks and the nested options.dryRun check) to abort with a non-zero exit or throw a CLI error instead of returning (e.g., call process.exit with a non-zero code or throw a descriptive Error) so callers see failure when the operation is not implemented; ensure the dry-run path also exits non-zero rather than returning success.src/commands/send/send.ts-4-8 (1)
4-8:⚠️ Potential issue | 🟠 MajorDon't clear the persistent master mute for a one-off
/send.
addSendOverride()already gives this turn temporary visibility.removeMasterPipeMute()makes a user-set mute permanent, and the error path only rolls back the transient override.Proposed fix
import { addSendOverride, removeSendOverride, - removeMasterPipeMute, } from '../../utils/pipeMuteState.js'- addSendOverride(targetName) - removeMasterPipeMute(targetName) + addSendOverride(targetName) client.send({ type: 'relay_unmute' })Also applies to: 55-61, 102-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/send/send.ts` around lines 4 - 8, The send command is incorrectly calling removeMasterPipeMute (which clears a user-set persistent master mute) for a one-off /send; instead only manage the transient visibility provided by addSendOverride. Remove any calls to removeMasterPipeMute in the send flow and ensure the error and completion paths only call removeSendOverride to roll back the temporary override added by addSendOverride (and leave persistent mute state untouched).src/hooks/usePipePermissionForward.ts-92-94 (1)
92-94:⚠️ Potential issue | 🟠 MajorNamespace the queued permission ID with
pipeName.
pipeNameis now stored on the queue item, but the actual queue key is still onlypipe:${requestId}. If two slaves emit the samerequestId, a laterpermission_cancelwill remove both prompts because the cancel path still filters bytoolUseIDalone.Proposed fix
- toolUseID: `pipe:${payload.requestId}`, + toolUseID: `pipe:${pipeName}:${payload.requestId}`, pipeName,- (item: any) => item.toolUseID !== `pipe:${payload.requestId}`, + (item: any) => + item.toolUseID !== `pipe:${pipeName}:${payload.requestId}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/usePipePermissionForward.ts` around lines 92 - 94, The queued permission ID currently uses only requestId and should be namespaced with pipeName to avoid collisions; update the creation of the queue item in usePipePermissionForward so toolUseID includes pipeName (e.g., `pipe:${pipeName}:${payload.requestId}`) and update any cancellation/filtering logic that checks toolUseID (the permission_cancel handling) to use the same namespaced format so cancel only targets the intended pipe's prompt; ensure both insert and remove code paths (the code that sets toolUseID and the code that filters/removes by toolUseID) are changed consistently.src/main.tsx-1804-1807 (1)
1804-1807:⚠️ Potential issue | 🟠 MajorThis drops settings-driven assistant activation.
The new guard only honors
markAssistantForced()and the hidden--assistantflag. Sessions relying on.claude/settings.jsonwithassistant: truenow skipkairosEnabledandinitializeAssistantTeam(), which no longer matches the surrounding flow.Proposed fix
if ( feature("KAIROS") && assistantModule && - (assistantModule.isAssistantForced() || - (options as Record<string, unknown>).assistant === true) && + (assistantModule.isAssistantForced() || + assistantModule.isAssistantMode() || + (options as Record<string, unknown>).assistant === true) && !(options as { agentId?: unknown }).agentId && kairosGate ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.tsx` around lines 1804 - 1807, The new guard for feature("KAIROS") currently only checks assistantModule.isAssistantForced() and the CLI/flag (options.assistant), which drops activation driven by persisted settings (the assistant:true in .claude/settings.json); restore that behavior by extending the condition to also honor the stored settings flag (e.g., check whatever API reads settings, such as settingsManager.get("assistant") or a helper like assistantModule.isAssistantEnabledFromSettings()), so that kairosEnabled and initializeAssistantTeam() still run when the persisted assistant setting is true in addition to isAssistantForced() or options.assistant.src/commands/job/job.tsx-19-30 (1)
19-30:⚠️ Potential issue | 🟠 MajorAvoid process-wide console monkey-patching in async command execution
Lines 21-23 temporarily replace global console methods while awaiting async code; this can capture unrelated logs from other concurrent work and produce nondeterministic REPL output. Please route output through an injected logger/output sink on
templatesMaininstead of mutating global console state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/job/job.tsx` around lines 19 - 30, The code is mutating process-wide console.log/console.error to capture output into the local lines array while awaiting templatesMain, which can intercept unrelated logs; change templatesMain invocation to accept an injected logger/output sink instead of monkey-patching globals: add a lightweight logger object (e.g., { log: (msg)=>lines.push(msg), error: (msg)=>lines.push(msg) }) and pass it into templatesMain (or into the imported handler call) so templatesMain writes to that sink rather than relying on global console; update the templatesMain signature and all callers to accept and use this logger to avoid global console mutation.src/cli/bg/tail.ts-53-55 (1)
53-55:⚠️ Potential issue | 🟠 MajorHandle log truncation/rotation to avoid permanently stalled tailing
Line 54 returns when size shrinks, but
positionis not reset. After truncate/rotate, new logs can be missed indefinitely.🐛 Suggested fix for truncation handling
- if (stat.size <= position) return + if (stat.size < position) { + // File truncated/rotated; restart reading from beginning + position = 0 + } + if (stat.size === position) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/bg/tail.ts` around lines 53 - 55, The current tail loop uses statSync(logPath) and returns when stat.size <= position, which leaves position stale after truncation/rotation; change this behavior so if stat.size < position (file shrank/rotated) you reset position to 0 (or to stat.size if you prefer starting at end) and continue reading, and only skip when stat.size === position; update the logic around the statSync(logPath) check where position is referenced to handle shrink/rotation by resetting position instead of returning.src/utils/handlePromptSubmit.ts-479-631 (1)
479-631:⚠️ Potential issue | 🟠 MajorKeep completion-finalization errors out of the failure path.
The
catchcurrently wraps both the turn execution and the laterfinalizeAutonomyRunCompleted()loop. If completion finalization throws for one run, every tracked run is then marked failed even though the turn itself already succeeded.Suggested fix
- try { - await runWithWorkload(turnWorkload, async () => { + try { + await runWithWorkload(turnWorkload, async () => { ... }) // end runWithWorkload — ALS context naturally scoped, no finally needed - if (autonomyRunIds?.length) { - for (const runId of autonomyRunIds) { - const nextCommands = await finalizeAutonomyRunCompleted({ - runId, - priority: 'later', - workload: turnWorkload, - }) - for (const nextCommand of nextCommands) { - enqueue(nextCommand) - } - } - } } catch (error) { - if (autonomyRunIds?.length) { + if (autonomyRunIds?.size) { for (const runId of autonomyRunIds) { await finalizeAutonomyRunFailed({ runId, error: String(error), }) } } throw error } + + if (autonomyRunIds?.size) { + for (const runId of autonomyRunIds) { + const nextCommands = await finalizeAutonomyRunCompleted({ + runId, + priority: 'later', + workload: turnWorkload, + }) + for (const nextCommand of nextCommands) { + enqueue(nextCommand) + } + } + }src/tasks/InProcessTeammateTask/InProcessTeammateTask.tsx-126-140 (1)
126-140:⚠️ Potential issue | 🟠 MajorPrefer live idle teammates over historical terminal ones.
Now that message injection is allowed for idle teammates, this lookup can still return an older completed/killed task if that record is encountered before the current idle task. That sends callers to stale state.
Suggested fix
export function findTeammateTaskByAgentId( agentId: string, tasks: Record<string, TaskStateBase>, ): InProcessTeammateTaskState | undefined { let fallback: InProcessTeammateTaskState | undefined; for (const task of Object.values(tasks)) { if (isInProcessTeammateTask(task) && task.identity.agentId === agentId) { - // Prefer running tasks in case old killed tasks still exist in AppState - // alongside new running ones with the same agentId + // Prefer live tasks before falling back to historical terminal ones. if (task.status === 'running') { return task; } - // Keep first match as fallback in case no running task exists + if (task.status === 'idle') { + fallback = task; + continue; + } if (!fallback) { fallback = task; } } } return fallback; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/InProcessTeammateTask/InProcessTeammateTask.tsx` around lines 126 - 140, The current loop returns the first non-running match as fallback which can be an older terminal task; update the selection logic to prefer an idle teammate over terminal ones: inside the loop that iterates Object.values(tasks) and checks isInProcessTeammateTask(task) && task.identity.agentId === agentId, keep the existing immediate return for task.status === 'running', but then prefer task.status === 'idle' as the next-best match (either return it immediately or store it as a stronger fallback) before falling back to older terminal statuses (the current fallback variable). Ensure you reference the same symbols task.status, 'running', 'idle', fallback, isInProcessTeammateTask and agentId when making the change.src/cli/bg/engines/detached.ts-20-42 (1)
20-42:⚠️ Potential issue | 🟠 MajorReject PID 0 fallback when detached spawn fails.
When
spawn()fails (e.g., command not found),child.pidwill beundefined. Returning0as a fallback creates a false session record with a nonexistent process, causing subsequent operations (status,attach,kill) to fail or behave unpredictably.Suggested fix
const child = spawn(process.execPath, [entrypoint, ...opts.args], { detached: true, stdio: ['ignore', logFd, logFd], env: { ...opts.env, CLAUDE_CODE_SESSION_KIND: 'bg', CLAUDE_CODE_SESSION_NAME: opts.sessionName, CLAUDE_CODE_SESSION_LOG: opts.logPath, } as Record<string, string>, cwd: opts.cwd, }) + if (child.pid == null) { + closeSync(logFd) + throw new Error(`Failed to start detached session "${opts.sessionName}".`) + } + child.unref() closeSync(logFd) - - const pid = child.pid ?? 0 return { - pid, + pid: child.pid, sessionName: opts.sessionName, logPath: opts.logPath, engineUsed: 'detached', }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/bg/engines/detached.ts` around lines 20 - 42, The code currently returns pid = child.pid ?? 0 which creates a bogus session when spawn fails; instead, after calling spawn(...) and before returning, check if child.pid is undefined/falsey and if so closeSync(logFd) (and child.unref() if appropriate) and throw an Error (or return a rejected result) describing the spawn failure so callers don't get a PID 0 session; update the return path in this module/function to only return the object with pid when child.pid is present and otherwise propagate an error, referencing the spawned child, pid, logFd, and engineUsed ('detached') symbols to locate the change.src/commands/assistant/assistant.tsx-82-87 (1)
82-87:⚠️ Potential issue | 🟠 MajorDon't treat a fixed sleep as daemon readiness.
onInstalled()fires after 1.5s even if the detached daemon crashes immediately during startup, so the wizard can tell the user setup succeeded when nothing actually registered. Since the daemon now has a persisted status surface, this should wait for a real readiness signal instead of a blind timer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/assistant/assistant.tsx` around lines 82 - 87, The current setTimeout call that invokes onInstalled(dir) after 1.5s is unreliable; replace this blind sleep with a real readiness check that waits for the daemon's persisted status (or readiness event) to report "ready" (or failure) before calling onInstalled(dir). Locate the setTimeout in assistant.tsx and change it to either subscribe/poll the daemon status surface (or listen for a "ready" / "failed" event from the bridge) and only call onInstalled(dir) when the status is ready, otherwise surface an error if the daemon reports crashed or a configurable overall timeout is reached. Ensure you handle both success and failure paths (calling onInstalled on success, showing an error/rollback on failure) and keep dir passed through.src/utils/taskSummary.ts-51-65 (1)
51-65:⚠️ Potential issue | 🟠 MajorDefaulting to
busymakes idle turns look active.
statusstarts as'busy'and only changes when the last content block istool_use, so plain assistant text and non-array content still get reported as busy. Initialize to'idle'and only flip to'busy'when a trailingtool_useblock is actually present.Suggested fix
- let status: 'busy' | 'idle' = 'busy' + let status: 'busy' | 'idle' = 'idle' let waitingFor: string | undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/taskSummary.ts` around lines 51 - 65, The status variable is initialized to 'busy' causing non-tool assistant turns to be reported as active; change the default of status to 'idle' and only set status = 'busy' (and populate waitingFor) when you detect a trailing tool_use block: locate the block using lastAssistant.message.content and lastBlock (check Array.isArray(content) and lastBlock?.type === 'tool_use') and flip status there, leaving status as 'idle' for all other cases.src/jobs/templates.ts-55-73 (1)
55-73:⚠️ Potential issue | 🟠 MajorOnly reserve a template name after the file parses successfully.
seenNames.add(name)happens beforereadFileSync()/parseFrontmatter(). If the first copy of a template is unreadable or has invalid frontmatter, every lower-precedence fallback with the same name is skipped too.Suggested fix
if (!file.endsWith('.md')) continue const name = basename(file, '.md') if (seenNames.has(name)) continue - seenNames.add(name) const filePath = join(dir, file) try { const raw = readFileSync(filePath, 'utf-8') const { frontmatter, content } = parseFrontmatter(raw, filePath) const description = @@ templates.push({ name, description, filePath, frontmatter, content }) + seenNames.add(name) } catch { // Skip unreadable files }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/templates.ts` around lines 55 - 73, The code marks a template name as used before parsing, so if readFileSync/parseFrontmatter fails the same name is incorrectly skipped; move the seenNames.add(name) so it only runs after successful read/parse and templates.push (i.e., inside the try block after parseFrontmatter/extractDescriptionFromMarkdown) and keep the initial check if (seenNames.has(name)) continue before attempting to read the file; update the loop around files, the readFileSync/parseFrontmatter block, and the templates.push usage accordingly.src/hooks/useScheduledTasks.ts-74-159 (1)
74-159:⚠️ Potential issue | 🟠 MajorCatch async cron-enqueue failures inside the scheduler callbacks.
These paths used to be synchronous; now both
void enqueueForLead(prompt)and the async IIFE underonFireTaskcan reject viacreateAutonomyQueuedPrompt()ormarkAutonomyRunFailed(). Because the scheduler fires them fire-and-forget, those rejections will escape as unhandled promise rejections.Suggested direction
const enqueueForLead = async (prompt: string) => { - const command = await createAutonomyQueuedPrompt({ - basePrompt: prompt, - trigger: 'scheduled-task', - currentDir: getCwd(), - workload: WORKLOAD_CRON, - }) - if (!command) { - return - } - enqueuePendingNotification(command) + try { + const command = await createAutonomyQueuedPrompt({ + basePrompt: prompt, + trigger: 'scheduled-task', + currentDir: getCwd(), + workload: WORKLOAD_CRON, + }) + if (!command) { + return + } + enqueuePendingNotification(command) + } catch (err) { + logForDebugging(`[ScheduledTasks] failed to enqueue lead cron: ${err}`) + } }Apply the same pattern to the
onFireTaskasync IIFE.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScheduledTasks.ts` around lines 74 - 159, The scheduler callbacks can reject and cause unhandled promise rejections (e.g., createAutonomyQueuedPrompt and markAutonomyRunFailed used in enqueueForLead and the onFireTask IIFE); wrap the fire-and-forget calls in try/catch blocks and handle errors (log via logForDebugging/processLogger or similar) so rejections are swallowed and surfaced safely. Specifically, update enqueueForLead usage in the onFire handler and the entire async IIFE inside onFireTask to catch any thrown errors from createAutonomyQueuedPrompt, injectUserMessageToTeammate, and markAutonomyRunFailed (and await where needed inside the try) and ensure errors are logged instead of escaping as unhandled rejections.src/cli/print.ts-1862-1878 (1)
1862-1878:⚠️ Potential issue | 🟠 MajorCatch errors from the fire-and-forget autonomy schedulers.
Both callbacks launch async work with
void (async () => { ... })()and no catch. If prompt preparation or queue persistence throws, Bun gets an unhandled rejection and the proactive/cron loop can silently stop scheduling further work. Please terminate these chains with.catch(logError)or route them through a shared helper that logs failures and keeps the scheduler alive.Suggested hardening
- void (async () => { + void (async () => { // ... - })() + })().catch(logError)Also applies to: 2787-2849
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/print.ts` around lines 1862 - 1878, The anonymous fire-and-forget async IIFEs that call createProactiveAutonomyCommands (the block using TICK_TAG, randomUUID, enqueue, run and checking inputClosed) need error handling so unhandled promise rejections can't kill the scheduler; wrap those IIFEs (and the similar block around lines 2787-2849) with a `.catch(logError)` or call them via a small helper (e.g., safeFireAndForget(async () => { ... })) that catches and logs errors, ensuring any thrown errors from createProactiveAutonomyCommands, enqueue, or run are logged and do not stop the proactive/cron loop.src/jobs/state.ts-18-20 (1)
18-20:⚠️ Potential issue | 🟠 MajorBlock path traversal through
jobId.
jobIdis interpolated straight into the on-disk path. Any caller that passes../...here can escape the jobs directory and make/job statusor/job replyread/write arbitrary files under the config tree. Validate the identifier before joining, or resolve the final path and assert it stays undergetJobsDir().Suggested hardening
+function assertSafeJobId(jobId: string): string { + if (!/^[A-Za-z0-9_-]+$/.test(jobId)) { + throw new Error('Invalid job id') + } + return jobId +} + export function getJobDir(jobId: string): string { - return join(getJobsDir(), jobId) + return join(getJobsDir(), assertSafeJobId(jobId)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/state.ts` around lines 18 - 20, The getJobDir function currently concatenates jobId into the filesystem path allowing path traversal; fix getJobDir by validating/sanitizing jobId (reject or normalize inputs containing path separators or ..) or by constructing the candidate path (join(getJobsDir(), jobId)), resolving it to an absolute path and asserting the resolved path starts with the resolved getJobsDir() prefix before returning; update getJobDir to throw on invalid jobId so callers cannot escape the jobs directory.src/cli/bg.ts-165-169 (1)
165-169:⚠️ Potential issue | 🟠 MajorInclude legacy detached sessions in targetless attach selection.
This filter only keeps sessions with
tmuxSessionNameor an explicitengine === 'detached'. Older detached session files have neither field, soclaude daemon attachwithout a target skips sessions thatresolveSessionEngine()would otherwise treat as attachable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/bg.ts` around lines 165 - 169, The bgSessions filter is excluding legacy detached session files (they lack tmuxSessionName and engine) so update the filter in src/cli/bg.ts (the bgSessions variable) to treat those legacy sessions as attachable: include sessions where s.tmuxSessionName is truthy OR resolveSessionEngine(s) === 'detached' (or, if you prefer, include sessions that have no engine and no tmuxSessionName as detached). In short, change the predicate used by bgSessions to call resolveSessionEngine(session) and accept sessions whose resolved engine is 'detached' in addition to ones with a tmuxSessionName.src/utils/autonomyRuns.ts-716-743 (1)
716-743:⚠️ Potential issue | 🟠 MajorDon't consume heartbeat flow tasks before their commands exist.
commitAutonomyQueuedPrompt(prepared)marks everydueHeartbeatTaskas consumed, and only after that does this loop try to create managed-flow commands for the step-backed tasks. IfstartManagedAutonomyFlowFromHeartbeatTask()returnsnullorshouldCreate()flips false mid-loop, those heartbeat tasks are skipped until the next interval without ever enqueuing work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/autonomyRuns.ts` around lines 716 - 743, commitAutonomyQueuedPrompt(prepared) currently marks all prepared.dueHeartbeatTasks consumed before we try to create commands, causing tasks to be lost if startManagedAutonomyFlowFromHeartbeatTask returns null or params.shouldCreate flips false; fix by changing the order so you create/manage-flow commands for each prepared.dueHeartbeatTask first (call startManagedAutonomyFlowFromHeartbeatTask for each task and only mark/commit that task as consumed afterwards), or alter commitAutonomyQueuedPrompt to avoid marking heartbeat tasks consumed until a corresponding flowCommand was successfully produced; update logic that builds the commands array (commands, startManagedAutonomyFlowFromHeartbeatTask, commitAutonomyQueuedPrompt, params.shouldCreate) so tasks are only consumed after successful enqueueing.src/utils/autonomyRuns.ts-315-349 (1)
315-349:⚠️ Potential issue | 🟠 MajorRequeue queued steps when the persisted run stub is unrecoverable.
If the step is already
queuedbut itsrunIdno longer resolves to a queued/unstarted run, this returnsnulland leaves the managed flow stuck inqueuedforever. Recovery should fall back to creating a fresh queued prompt once the old stub cannot be recovered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/autonomyRuns.ts` around lines 315 - 349, The current branch returns null when step.status is 'queued' but getAutonomyRunById(step.runId) does not return a recoverable queued/unstarted run, which leaves the managed flow stuck; instead, detect that case (run is falsy or run.status !== 'queued' or run.startedAt/endedAt set) and build and return a fresh queued prompt similar to the successful branch: call buildAutonomyTurnPrompt with basePrompt from buildManagedFlowStepPrompt(flow, stepIndex) and return the same envelope (value, mode: 'prompt', priority: params.priority ?? 'later', isMeta: true, workload: params.workload) but omit run-specific fields (runId/sourceId/sourceLabel/flowStepId/etc.) in origin and autonomy so the step is requeued as a new autonomy turn; use the same unique helpers getAutonomyRunById, buildAutonomyTurnPrompt, and buildManagedFlowStepPrompt to implement this fallback.src/cli/bg.ts-47-57 (1)
47-57:⚠️ Potential issue | 🟠 MajorMatch PIDs only for purely numeric targets.
parseInt()accepts numeric prefixes, so a target like123abcresolves to PID123. That meanslogs,attach, and especiallykillcan hit the wrong session instead of treating the input as a name/ID mismatch.💡 Proposed fix
export function findSession( sessions: SessionEntry[], target: string, ): SessionEntry | undefined { - const asNum = parseInt(target, 10) + const asNum = /^\d+$/.test(target) + ? Number.parseInt(target, 10) + : Number.NaN return sessions.find( s => s.sessionId === target || - s.pid === asNum || + (!Number.isNaN(asNum) && s.pid === asNum) || (s.name && s.name === target), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/bg.ts` around lines 47 - 57, The findSession function currently uses parseInt(target, 10) which accepts numeric prefixes and can incorrectly match PIDs for inputs like "123abc"; update findSession to only treat the target as a PID when the target is purely numeric (e.g. test with /^\d+$/ or similar) before converting to a number and comparing to s.pid, otherwise fall back to matching s.sessionId or s.name; reference the findSession function and ensure s.pid comparisons only happen when the numeric-only check passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a32de47-bd88-4c79-8720-7c4687b30864
📒 Files selected for processing (123)
.gitignore02-kairos (1).mdAGENTS.mdbuild.tsdocs/features/daemon-restructure-design.mddocs/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/task/task-013-bg-engine-abstraction.mddocs/task/task-014-daemon-command-hierarchy.mddocs/task/task-015-job-command-hierarchy.mddocs/task/task-016-backward-compat-tests.mddocs/test-plans/openclaw-autonomy-baseline.mdpackages/builtin-tools/src/tools/PushNotificationTool/PushNotificationTool.tspackages/builtin-tools/src/tools/SendUserFileTool/SendUserFileTool.tspackages/remote-control-server/src/logger.tspackages/remote-control-server/src/routes/v1/session-ingress.tspackages/remote-control-server/src/routes/v1/sessions.tspackages/remote-control-server/src/routes/web/control.tspackages/remote-control-server/src/routes/web/sessions.tspackages/remote-control-server/src/services/disconnect-monitor.tspackages/remote-control-server/src/services/work-dispatch.tspackages/remote-control-server/src/transport/event-bus.tspackages/remote-control-server/src/transport/sse-writer.tspackages/remote-control-server/src/transport/ws-handler.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/bg/__tests__/detached.test.tssrc/cli/bg/__tests__/engine.test.tssrc/cli/bg/__tests__/tail.test.tssrc/cli/bg/engine.tssrc/cli/bg/engines/detached.tssrc/cli/bg/engines/index.tssrc/cli/bg/engines/tmux.tssrc/cli/bg/tail.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/daemon/__tests__/daemon.test.tssrc/commands/daemon/daemon.tsxsrc/commands/daemon/index.tssrc/commands/init.tssrc/commands/job/__tests__/job.test.tssrc/commands/job/index.tssrc/commands/job/job.tsxsrc/commands/lang/index.tssrc/commands/lang/lang.tssrc/commands/send/send.tssrc/commands/torch.tssrc/daemon/__tests__/daemonMain.test.tssrc/daemon/__tests__/state.test.tssrc/daemon/main.tssrc/daemon/state.tssrc/entrypoints/cli.tsxsrc/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__/autonomyFlows.test.tssrc/utils/__tests__/autonomyPersistence.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/integration/cli-arguments.test.tstests/mocks/file-system.tstsconfig.json
💤 Files with no reviewable changes (3)
- src/hooks/useAwaySummary.ts
- src/commands/assistant/assistant.ts
- src/assistant/AssistantSessionChooser.ts
Comment on lines
+85
to
+100
| export function queryDaemonStatus(name = 'remote-control'): { | ||
| status: DaemonStatus | ||
| state?: DaemonStateData | ||
| } { | ||
| const state = readDaemonState(name) | ||
| if (!state) { | ||
| return { status: 'stopped' } | ||
| } | ||
|
|
||
| if (isProcessAlive(state.pid)) { | ||
| return { status: 'running', state } | ||
| } | ||
|
|
||
| // Stale — process is dead but state file remains | ||
| removeDaemonState(name) | ||
| return { status: 'stale' } |
Contributor
There was a problem hiding this comment.
Don't identify or kill the daemon by PID alone.
process.kill(pid, 0) only proves that some process with that PID exists. After PID reuse, status can report a stale daemon as running and stopDaemonByPid() can send SIGTERM/SIGKILL to an unrelated process. This code also removes the state file and returns true after the SIGKILL path without re-verifying exit, so a failed kill is reported as success. Persist and verify a stronger process identity before signaling, and only delete the state file once the target is confirmed dead.
Also applies to: 109-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/daemon/state.ts` around lines 85 - 100, queryDaemonStatus and
stopDaemonByPid rely solely on PID (isProcessAlive/process.kill) which can
target a reused PID; update the logic to persist and verify a stronger process
identity (e.g., include startTime/boot-time, cmdline/exe path, or a unique nonce
in readDaemonState/DaemonStateData) and check that the live process matches
those attributes before reporting "running" or sending signals; in
stopDaemonByPid (and any kill path) perform a matching check first, send signals
only to the verified process, wait and re-check the process exit (with a
timeout/retries) before removing state with removeDaemonState, and return
failure if the kill did not actually terminate the same process. Ensure the
functions referenced (queryDaemonStatus, readDaemonState, isProcessAlive,
stopDaemonByPid, removeDaemonState) are updated together so identity is
persisted on start and re-validated on status/stop operations.
Comment on lines
+4848
to
+4860
| 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); | ||
| } | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Only finalize autonomy runs after the turn actually executes.
onQuery() also resolves in its concurrent path after it just re-enqueues the prompt (Lines 3480-3497). This then(...) therefore marks the run completed and schedules follow-up commands even when the run never ran, which can corrupt autonomy ordering/state. Please move completion/failure handling to the code path that knows whether execution really happened, or make onQuery() return an explicit outcome.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/REPL.tsx` around lines 4848 - 4860, The code calls
finalizeAutonomyRunCompleted in the generic .then of onQuery() which can run
even when onQuery only re-enqueues the prompt (i.e., no execution occurred);
update the flow so completion/failure handling only happens when the turn
actually executed by either: (A) modify onQuery(...) to return an explicit
outcome object (e.g., { executed: boolean, success: boolean, result?: ... }) and
check outcome.executed before calling finalizeAutonomyRunCompleted and
enqueueing next commands, or (B) move the finalizeAutonomyRunCompleted/enqueue
logic into the internal code path in onQuery (the branch that performs
execution) so it only runs on real execution; update callers to consume the new
outcome shape if you choose option A and ensure finalizeAutonomyRunCompleted is
invoked only when execution is confirmed.
Comment on lines
+506
to
+590
| export async function queueManagedAutonomyFlowStepRun(params: { | ||
| flowId: string | ||
| stepId: string | ||
| stepIndex: number | ||
| runId: string | ||
| rootDir?: string | ||
| nowMs?: number | ||
| }): Promise<AutonomyFlowRecord | null> { | ||
| const rootDir = resolve(params.rootDir ?? getProjectRoot()) | ||
| return updateAutonomyFlowById( | ||
| params.flowId, | ||
| current => { | ||
| const state = cloneManagedState(current.stateJson) | ||
| const step = state?.steps[params.stepIndex] | ||
| if (!state || !step || step.stepId !== params.stepId) { | ||
| return current | ||
| } | ||
| step.status = 'queued' | ||
| step.runId = params.runId | ||
| step.startedAt = undefined | ||
| step.endedAt = undefined | ||
| step.error = undefined | ||
| state.currentStepIndex = params.stepIndex | ||
| return { | ||
| ...current, | ||
| revision: current.revision + 1, | ||
| status: 'queued', | ||
| currentStep: step.name, | ||
| latestRunId: params.runId, | ||
| runCount: current.runCount + 1, | ||
| updatedAt: params.nowMs ?? Date.now(), | ||
| endedAt: undefined, | ||
| blockedRunId: undefined, | ||
| blockedSummary: undefined, | ||
| waitJson: undefined, | ||
| stateJson: state, | ||
| lastError: undefined, | ||
| } | ||
| }, | ||
| rootDir, | ||
| ) | ||
| } | ||
|
|
||
| export async function markManagedAutonomyFlowStepRunning(params: { | ||
| flowId: string | ||
| runId: string | ||
| rootDir?: string | ||
| nowMs?: number | ||
| }): Promise<AutonomyFlowRecord | null> { | ||
| const rootDir = resolve(params.rootDir ?? getProjectRoot()) | ||
| return updateAutonomyFlowById( | ||
| params.flowId, | ||
| current => { | ||
| const state = cloneManagedState(current.stateJson) | ||
| if (!state) { | ||
| return current | ||
| } | ||
| const stepIndex = state.steps.findIndex( | ||
| step => step.runId === params.runId, | ||
| ) | ||
| if (stepIndex === -1) { | ||
| return current | ||
| } | ||
| const step = state.steps[stepIndex]! | ||
| step.status = 'running' | ||
| step.startedAt = params.nowMs ?? Date.now() | ||
| state.currentStepIndex = stepIndex | ||
| return { | ||
| ...current, | ||
| revision: current.revision + 1, | ||
| status: 'running', | ||
| currentStep: step.name, | ||
| latestRunId: params.runId, | ||
| updatedAt: step.startedAt, | ||
| startedAt: current.startedAt ?? step.startedAt, | ||
| endedAt: undefined, | ||
| blockedRunId: undefined, | ||
| blockedSummary: undefined, | ||
| waitJson: undefined, | ||
| stateJson: state, | ||
| lastError: undefined, | ||
| } | ||
| }, | ||
| rootDir, | ||
| ) |
Contributor
There was a problem hiding this comment.
Reject late queue/start updates for terminal or cancelled flows.
These transitions mutate by stepId/runId alone. If a cancel lands between run creation and the follow-up queue/start update, queueManagedAutonomyFlowStepRun() or markManagedAutonomyFlowStepRunning() can reopen a cancelled flow and dispatch extra work. Please require the flow to still be active and the step to be in the expected predecessor state before applying the transition.
💡 Hardening direction
export async function queueManagedAutonomyFlowStepRun(params: {
flowId: string
stepId: string
stepIndex: number
runId: string
rootDir?: string
nowMs?: number
}): Promise<AutonomyFlowRecord | null> {
const rootDir = resolve(params.rootDir ?? getProjectRoot())
return updateAutonomyFlowById(
params.flowId,
current => {
+ if (!isManagedFlowStatusActive(current.status) || current.cancelRequestedAt) {
+ return current
+ }
const state = cloneManagedState(current.stateJson)
const step = state?.steps[params.stepIndex]
- if (!state || !step || step.stepId !== params.stepId) {
+ if (!state || !step || step.stepId !== params.stepId || step.status !== 'pending') {
return current
}
step.status = 'queued'
step.runId = params.runIdApply the same kind of predecessor-state guard to markManagedAutonomyFlowStepRunning() (queued → running only).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/autonomyFlows.ts` around lines 506 - 590, Both functions can reopen
or advance terminal/cancelled flows because they mutate by stepId/runId alone;
update queueManagedAutonomyFlowStepRun to first verify the flow is still active
(e.g., current.status not in terminal states like
'cancelled'/'completed'/'failed') and that the target step is in the expected
predecessor state (e.g., step.status === 'pending' or whatever pre-queue state
you use) before setting step.status='queued'; likewise update
markManagedAutonomyFlowStepRunning to require the flow is active and the found
step currently has status 'queued' before setting step.status='running' and
updating timestamps; make these guards inside the updater callback in
updateAutonomyFlowById so the revision/update is skipped if checks fail.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
单 commit squash merge，包含 12 项内容（123 文件，+13541/-1892）：
5 个功能分支集成：
新增功能：
6. daemon/job 命令层级化重构 (subcommand 架构)
7. 跨平台后台引擎抽象 (detached/tmux engines)
质量修复：
8. 修复 src/ 中 43 个预存在的 TypeScript 类型错误
9. 修复 langfuse isolated test mock 完整性
10. 修复 CodeRabbit 审查的 Critical/Major/Minor 问题
11. remote-control-server logger 抽象 (测试 stderr 静默化，兼容 Bun 1.3.12)
12. /simplify 审查修复 (代码复用、质量、效率改进)
Key Changes
Daemon/Job 命令层级化
/daemon合并 supervisor + bg sessions（status/start/stop）/job收纳new/list/reply跨平台后台引擎
BgEngine抽象接口：Win → DetachedEngine, macOS/Linux → TmuxEngine/DetachedEngine--bg模式因缺少 tmux 而失败自治权限系统
is_error结果不再被错误标记为 completed代码质量
getProjectDirsUpToHome和extractDescriptionFromMarkdownTest plan
bun test— 2758 pass / 0 fail / EXIT:0 (Bun 1.3.12)bunx tsc --noEmit— 零错误/daemon status正常显示/job list正常工作--bg模式在 Windows 上使用 DetachedEngineSummary by CodeRabbit
New Features
daemon ps/logs/kill)/job new/list/reply)/lang en|zh|auto)claude assistant <sessionId>)Improvements
/daemon)Backward Compatibility