feat: integrate 5 feature branches + MIME detection fix#266
feat: integrate 5 feature branches + MIME detection fix#266amDosion wants to merge 1 commit intoclaude-code-best:lint/previewfrom
Conversation
Features: - MCP tsc error fixes (43 pre-existing TypeScript errors) - Pipe mute sync, /lang command, mute state machine - Stub recovery: daemon state, job system, bg engine, assistant session - KAIROS activation + tool implementation - Openclaw autonomy: permission system, run records, managed flows - Daemon/job command hierarchy (subcommand architecture) - Cross-platform background engine (detached/tmux) MIME detection fix (packages/@ant/computer-use-mcp/src/toolCalls.ts): - detectMimeFromBase64(): decode raw bytes from base64 instead of broken charCodeAt comparison - Fixes API 400 on Windows (JPEG) and macOS (PNG) screenshots - Verified by Codex (GPT-5.4) via Buffer computation Other: - Remote-control-server logger abstraction - InProcessTeammateTask type extensions - GrowthBook gate enhancements - 30+ new/refactored test files with mock isolation Verified: tsc 0 errors, bun test 2758 pass / 0 fail
📝 WalkthroughWalkthroughA major integration batch adds multiple new features and subsystems: daemon/background session management with tmux/detached engine support, template-based job workflow system, autonomy run/flow orchestration for automatic task handling, assistant mode UI improvements, language preference support, centralized logging infrastructure, and pipe mute state management for slave output control. Includes base64 MIME type detection fix and extensive test coverage. 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}
There was a problem hiding this comment.
Actionable comments posted: 1
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 (3)
src/proactive/useProactive.ts (1)
75-91:⚠️ Potential issue | 🟡 MinorAsync tick execution races with next tick scheduling.
The async IIFE on lines 75-87 runs concurrently with
scheduleTick()on line 90. IfcreateProactiveAutonomyCommandsis slow or if the user starts typing during the async operation, commands could be queued after the next tick is already scheduled, potentially causing ordering issues.Consider awaiting the async work before scheduling the next tick, or explicitly handle the case where conditions change during the async operation:
🔧 Proposed restructure
- void (async () => { - const commands = await createProactiveAutonomyCommands({ - basePrompt: `<${TICK_TAG}>${new Date().toLocaleTimeString()}</${TICK_TAG}>`, - currentDir: getCwd(), - }) - for (const command of commands) { - // Always queue proactive turns. This avoids races where the prompt - // is built asynchronously, a user turn starts meanwhile, and a - // direct-submit path would silently drop the autonomy turn after - // consuming its heartbeat due-state. - optsRef.current.onQueueTick(command) - } - })() - - // Schedule next tick - scheduleTick() + 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) + } + } finally { + // Schedule next tick after async work completes + scheduleTick() + } + })()🤖 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 - 91, The async IIFE that calls createProactiveAutonomyCommands races with scheduleTick() — move the scheduleTick() call so the next tick is scheduled only after the async work completes (i.e., await createProactiveAutonomyCommands and enqueue via optsRef.current.onQueueTick for each command before calling scheduleTick()), or alternatively add a cancel/version check inside the IIFE to drop/ignore stale results if conditions changed; update references around createProactiveAutonomyCommands, optsRef.current.onQueueTick, scheduleTick and TICK_INTERVAL_MS to ensure ordering is preserved.src/entrypoints/cli.tsx (1)
205-223: 🛠️ Refactor suggestion | 🟠 MajorSplit these
ifconditions to keepfeature()as the direct guard, per coding guidelines.These code branches use
feature()inside&&chains at lines 206, 221, 236–240, 257, and 269–270. The coding guideline requires: "Usefeature()only directly inifstatements or ternary expressions; do not assign to variables, place in arrow functions, or use in&&chains."Refactor each branch to nest the remaining conditions:
if (feature('DAEMON') || feature('BG_SESSIONS')) { if (args[0] === 'daemon') { // existing body } }instead of combining with
&&.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/cli.tsx` around lines 205 - 223, The guarded branches currently use feature(...) inside && chains; change each to first check feature(...) directly and nest the other conditions inside (e.g., if (feature('DAEMON') || feature('BG_SESSIONS')) { if (args[0] === 'daemon') { ... } }), and apply the same pattern for the BG_SESSIONS/--bg/--background fast-path and the other occurrences mentioned; keep existing bodies intact (profileCheckpoint, enableConfigs(), initSinks(), daemonMain(args.slice(1)), etc.) while only refactoring the if conditions to call feature() as the direct guard and nest the remaining checks.src/screens/REPL.tsx (1)
4307-4312:⚠️ Potential issue | 🟠 MajorPreserve the draft when teammate injection is rejected.
injectUserMessageToTeammate()now reportsfalsefor terminal teammates, but this branch still clears the prompt. If the teammate exits between render and Enter, the user's message is silently lost.Suggested fix
- injectUserMessageToTeammate(task.id, input, undefined, setAppState); + if (!injectUserMessageToTeammate(task.id, input, undefined, setAppState)) { + return; + } } setInputValue('');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/REPL.tsx` around lines 4307 - 4312, injectUserMessageToTeammate can return false for terminal teammates, but the code always clears the draft after the call; change the call to capture its result and only clear the draft when injection succeeded. Specifically, await or handle the boolean return from injectUserMessageToTeammate(task.id, input, undefined, setAppState) into a variable (e.g., injected), and only call setInputValue(''), helpers.setCursorOffset(0), and helpers.clearBuffer() when injected is truthy; if injected is false, leave the input and cursor/buffer intact so the user's draft is preserved.
🟠 Major comments (30)
src/cli/rollback.ts-22-30 (1)
22-30:⚠️ Potential issue | 🟠 Major
--safecurrently returns success without performing rollbackThis branch is a no-op for non-dry-run and exits with code 0, which can mislead users/automation into thinking rollback succeeded.
Proposed fix
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.') + console.error('Safe version pinning requires access to the release API.') + console.error('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 22 - 30, The current options?.safe branch prints messages and returns success without performing a rollback; change it so that when options.safe is true and not a dry run (options.dryRun is false) the CLI exits with a non-zero status (e.g., process.exit(1) or throw new Error) after printing the explanatory messages, rather than returning normally. Update the branch in src/cli/rollback.ts that checks options?.safe (and references options.dryRun) to terminate with an error status so automation and users see the failure when the safe rollback cannot be performed.src/cli/rollback.ts-57-67 (1)
57-67:⚠️ Potential issue | 🟠 MajorImprove subprocess error handling for signal and launch failures
The current code only checks
status, missing error conditions and signal termination. Checkresult.errorfor launch failures (e.g., ENOENT on Windows if npm is unavailable), thenresult.signalfor signal termination, before checkingstatus.Avoid the Windows-specific
npm.cmdconditional — it causes EINVAL errors withshell=false(CVE-2024-27980). Using'npm'directly resolves via PATH correctly on all platforms.Suggested fix
// Version rollback via npm/bun const { spawnSync } = await import('child_process') const result = spawnSync( 'npm', ['install', '-g', `@anthropic-ai/claude-code@${target}`], { stdio: 'inherit' }, ) + if (result.error) { + console.error(`Rollback failed: ${result.error.message}`) + process.exitCode = 1 + return + } + if (result.signal) { + console.error(`Rollback terminated by signal ${result.signal}`) + process.exitCode = 1 + return + } if (result.status !== 0) { console.error(`Rollback failed with exit code ${result.status}`) process.exitCode = result.status ?? 1 } else { console.log(`Rolled back to ${target} successfully.`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/rollback.ts` around lines 57 - 67, The subprocess error handling around spawnSync('npm', ['install', '-g', `@anthropic-ai/claude-code@${target}`]) is incomplete: update the logic after calling spawnSync (see spawnSync and the result variable) to first check result.error and log a clear launch-failure message and set process.exitCode (e.g., 1) for ENOENT/launch errors, then check result.signal and log that the process was terminated by a signal and set process.exitCode accordingly, and only then check result.status for a non-zero exit code; also remove any platform-specific use of 'npm.cmd' and always invoke 'npm' so PATH resolution is used (avoid shell=true) to prevent EINVAL/CVE issues.src/cli/up.ts-15-17 (1)
15-17:⚠️ Potential issue | 🟠 MajorThe resolver can pick one
CLAUDE.mdand run it from another directory context.This only probes
gitRootandcwd, and it checksgitRootfirst. Then the child process always runs with the originalcwd. In a nested repo that can both select the wrongCLAUDE.mdand execute its relative commands from the wrong directory. Walk parent directories upward fromcwd, remember the matched directory, and pass that directory tospawnSync.Also applies to: 21-28, 52-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/up.ts` around lines 15 - 17, The current resolver only probes gitRoot then cwd (cwd, gitRoot, searchDirs) and always runs child processes from the original cwd, which can pick a CLAUDE.md from a parent repo but execute commands in the wrong directory; instead, walk parent directories upward from cwd to find the nearest directory that contains CLAUDE.md (record matchedDir), construct searchDirs to prefer that matchedDir (and include gitRoot if needed), and when invoking spawnSync pass matchedDir as the cwd option so the child executes in the directory where the CLAUDE.md was found; apply the same change to the other lookup/exec sites referenced around the same blocks.src/cli/up.ts-75-92 (1)
75-92:⚠️ Potential issue | 🟠 MajorMake the section parser fence-aware.
Once
inSectionis true, any line starting with#ends the section. A normal shell comment inside a fencedbashblock will truncate the script here, and the common one-comment case collapses tonull. Track code-fence state before treating#as the next markdown heading.🐛 Suggested fix
function extractUpSection(markdown: string): string | null { const lines = markdown.split('\n') let inSection = false + let inFence = false const sectionLines: string[] = [] for (const line of lines) { if (/^#\s+claude\s+up\b/i.test(line)) { inSection = true continue } - if (inSection && /^#\s/.test(line)) { - break - } if (inSection) { + if (/^```/.test(line.trimStart())) { + inFence = !inFence + } + if (!inFence && /^#\s/.test(line)) { + break + } sectionLines.push(line) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/up.ts` around lines 75 - 92, The parser currently ends a section when it sees any line matching /^#\s/ while inSection is true, which incorrectly stops inside fenced code blocks; add an inFence boolean and update the loop in the block that fills sectionLines so that before checking /^#\s/ you toggle inFence when a fence marker is encountered (e.g., line.trimStart().startsWith('```') or /^```\w*/.test(line.trimStart())), and only treat /^#\s/ as a section end when inFence is false; ensure you initialize inFence = false alongside inSection and keep pushing the line into sectionLines after handling the fence/toggle logic.src/services/langfuse/__tests__/langfuse.isolated.ts-217-229 (1)
217-229:⚠️ Potential issue | 🟠 MajorThe init happy-path and idempotence cases never hit the initialized path.
The
"returns true when keys are configured"case only checksisLangfuseEnabled(), and the idempotence case callsinitLangfuse()without configuring keys first. A broken init path or double-init bug would still pass this block.Configure
LANGFUSE_PUBLIC_KEY/LANGFUSE_SECRET_KEYin both tests, callinitLangfuse(), and assert the init-side effect only happens once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/langfuse/__tests__/langfuse.isolated.ts` around lines 217 - 229, The tests currently never exercise the initialized path: update both tests to set LANGFUSE_PUBLIC_KEY and LANGFUSE_SECRET_KEY before importing/using the module, then call initLangfuse() and verify initialization occurred; for the first test call isLangfuseEnabled() after setting the env vars and after calling initLangfuse(), and for the "idempotent" test set the env vars, call initLangfuse() twice and assert the init-side effect (e.g., whatever state or client is created by initLangfuse()) only happens once to prove idempotence of initLangfuse().src/services/langfuse/__tests__/langfuse.isolated.ts-24-28 (1)
24-28:⚠️ Potential issue | 🟠 MajorUse the child-span mocks here.
mockChildUpdate/mockChildEndare defined, butstartObservation()still returnsmockRootUpdate/mockRootEnd. That makes the generation/tool assertions unable to detect whether the code updated the child observation or the root trace.Suggested fix
-const mockStartObservation = mock(() => ({ - id: 'test-span-id', - traceId: 'test-trace-id', - type: 'span', - otelSpan: { - spanContext: () => mockSpanContext, - setAttribute: mockSetAttribute, - }, - update: mockRootUpdate, - end: mockRootEnd, -})) +const mockStartObservation = mock( + (_name?: string, _params?: unknown, options?: { asType?: string }) => ({ + id: 'test-span-id', + traceId: 'test-trace-id', + type: 'span', + otelSpan: { + spanContext: () => mockSpanContext, + setAttribute: mockSetAttribute, + }, + update: options?.asType === 'agent' ? mockRootUpdate : mockChildUpdate, + end: options?.asType === 'agent' ? mockRootEnd : mockChildEnd, + }), +)Then switch the child-observation expectations to
mockChildUpdate/mockChildEnd.Also applies to: 49-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/langfuse/__tests__/langfuse.isolated.ts` around lines 24 - 28, The test defines mockChildUpdate and mockChildEnd but startObservation currently returns mockRootUpdate/mockRootEnd; update the startObservation call(s) in this file (the one used to create the child observation / span) to return the pair [mockChildUpdate, mockChildEnd] instead of the root mocks, and then change the child-observation assertions to expect mockChildUpdate and mockChildEnd (also update the other startObservation usage later in the file that mirrors lines 49-59 to return and assert against the child mocks as well). Ensure you only switch the child span usages — leave root span startObservation usages returning mockRootUpdate/mockRootEnd.src/services/langfuse/__tests__/langfuse.isolated.ts-637-656 (1)
637-656:⚠️ Potential issue | 🟠 MajorThis test is tautological and does not exercise the trace reuse decision logic.
The test only creates a subagent trace and manually assigns hardcoded values to local variables, then asserts those same values match themselves. It never imports or calls the
query()function where the actual reuse decision occurs (lines 237–248 insrc/query.ts):const ownsTrace = !params.toolUseContext.langfuseTrace const langfuseTrace = params.toolUseContext.langfuseTrace ?? (isLangfuseEnabled() ? createTrace(...) : null)The real conditional logic—whether to reuse an existing trace or create a new one—can regress without this test catching it. Either import and invoke
query()with a pre-populatedlangfuseTraceintoolUseContextto test the actual branch, or remove this placeholder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/langfuse/__tests__/langfuse.isolated.ts` around lines 637 - 656, The test currently only constructs a subagent trace and asserts local variables against themselves; instead, import and call the actual query(...) function with params.toolUseContext.langfuseTrace set to the subTrace created by createSubagentTrace(...) so the query code path that computes ownsTrace and langfuseTrace is exercised; verify that query reuses the passed trace by asserting the returned trace (or the trace used inside query) equals the original subTrace and/or spy/mock createTrace/createSubagentTrace (or isLangfuseEnabled) to ensure no new trace was created.packages/remote-control-server/src/logger.ts-1-10 (1)
1-10:⚠️ Potential issue | 🟠 MajorDon't send debug traffic through the always-on logger.
This wrapper is only silent in tests, so every
[RC-DEBUG]call routed through it still prints in normal environments. In this PR that includes potentially sensitive request payloads and per-event SSE traffic. Please add a default-offdebug()path, or equivalent env gating, and move the debug call sites there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/remote-control-server/src/logger.ts` around lines 1 - 10, Current logger always prints debug-style messages via log() in non-test environments; add a default-off debug channel gated by an environment flag and update call sites to use it. Introduce a new exported debug(...args: unknown[]) function (and a config boolean like const isDebug = process.env.RC_DEBUG === "true") that only calls console.log when isDebug is true (keeping existing isTest suppression logic), export it alongside log() and error(), and replace any [RC-DEBUG] call sites to use debug() so sensitive payloads are only emitted when RC_DEBUG is explicitly enabled.packages/builtin-tools/src/tools/SendUserFileTool/SendUserFileTool.ts-75-91 (1)
75-91:⚠️ Potential issue | 🟠 Major
stat()doesn't prove the file is readable.A path can exist and pass
isFile()but still fail on read due to permission restrictions. If the bridge is disabled (orBRIDGE_MODEfeature is off), the file is never actually read, so this tool can returnsent: trueeven though the file is unreadable. Add a permission check (access()withR_OK) before declaring success.🔧 Suggested change
- const { stat } = await import('fs/promises') + const { access, stat } = await import('fs/promises') + const { constants } = await import('fs') @@ let fileSize: number try { + await access(file_path, constants.R_OK) const fileStat = await stat(file_path) if (!fileStat.isFile()) {🤖 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 75 - 91, The current check uses stat(file_path) and isFile() but doesn't verify read permissions, so update SendUserFileTool to call access(file_path, constants.R_OK) (import access and constants from 'fs/promises' or 'fs') after stat succeeds and before declaring success; if access throws, return { data: { sent: false, file_path, error: 'File is not readable.' } } so the tool doesn't report sent: true for unreadable files (ensure you reference the existing file_path, stat, and the tool's return shape).packages/remote-control-server/src/transport/ws-handler.ts-134-135 (1)
134-135:⚠️ Potential issue | 🟠 MajorRemove raw message content from WS debug logs.
Line 134 and Line 219 currently log message bodies/previews. These can contain user content, secrets, or other sensitive data and should not be persisted in server logs.
🔧 Proposed fix
- log(`[RC-DEBUG] [WS] -> bridge (outbound): type=${event.type} len=${sdkMsg.length} msg=${sdkMsg.slice(0, 300)}`); + log(`[RC-DEBUG] [WS] -> bridge (outbound): type=${event.type} bytes=${sdkMsg.length}`); @@ - log(`[RC-DEBUG] [WS] <- bridge (inbound): sessionId=${sessionId} type=${eventType}${msg.uuid ? ` uuid=${msg.uuid}` : ""} msg=${JSON.stringify(msg).slice(0, 300)}`); + log( + `[RC-DEBUG] [WS] <- bridge (inbound): sessionId=${sessionId} type=${eventType}${msg.uuid ? ` uuid=${msg.uuid}` : ""} keys=${Object.keys(msg).slice(0, 8).join(",")}`, + );Also applies to: 219-219
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/remote-control-server/src/transport/ws-handler.ts` around lines 134 - 135, The debug log currently prints raw message content via the log call that references event.type and sdkMsg before calling ws.send(sdkMsg); remove the message body from logs to avoid leaking sensitive data — change the log entry to only include safe metadata (e.g., event.type and sdkMsg.length) and drop sdkMsg or any slice of it. Update the other debug logging site that also logs sdkMsg (the second log near the ws.send usage) to the same metadata-only format so no message content is ever written to logs.src/hooks/usePipeIpc.ts-249-256 (1)
249-256:⚠️ Potential issue | 🟠 MajorReset relay mute state on detach/cleanup to prevent sticky mute across sessions.
If
relay_muteis received and the session detaches beforerelay_unmute, mute can persist and silently drop relays after reconnect.🔧 Proposed fix
server.onMessage((msg: PipeMessage, _reply) => { if (msg.type !== 'detach') return @@ clearPendingPipePermissions('Pipe detached before permission was resolved.') pp().setPipeRelay(null) + pp().setRelayMuted(false) @@ store.setState((prev: any) => ({ @@ })) }) @@ if (pipeServer) { void pipeServer.close().catch(() => {}) pipeServer = null } pp().setPipeRelay(null) + pp().setRelayMuted(false) } }, []) // eslint-disable-line react-hooks/exhaustive-deps }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/usePipeIpc.ts` around lines 249 - 256, The relay mute state can stick if a 'relay_mute' arrives and the session detaches before a 'relay_unmute'—update the cleanup/detach logic to explicitly reset the mute flag: in the same module where server.onMessage handles 'relay_mute'/'relay_unmute' (reference server.onMessage and pp().setRelayMuted), add a call to pp().setRelayMuted(false) during the detach/cleanup path (e.g., in the effect cleanup, disconnect handler, or server.close/stop callback) so relay mute is cleared when the session ends.src/services/analytics/growthbook.ts-469-470 (1)
469-470:⚠️ Potential issue | 🟠 MajorThis turns
LOCAL_GATE_DEFAULTSinto a hard override, not a fallback.With
tengu_kairos_assistantadded here and_CACHED_MAY_BE_STALE()now returninglocalDefaultbefore the in-memory/disk caches, any gate listed inLOCAL_GATE_DEFAULTSstops seeing server-sidefalseor config changes on the cached path.getFeatureValueInternal()still honors the remote answer, so sync and blocking callers can now disagree, and the runtime kill switch thatisKairosEnabled()expects no longer works.If the goal is only “enable locally when GB is absent,” keep local defaults as the final fallback. If you need a true hard override for forks/self-hosted builds, it should be a separate opt-in helper rather than changing the shared cached getter.
💡 Keep cached reads cache-first
- // LOCAL_GATE_DEFAULTS take priority over remote values and disk cache. - // In fork/self-hosted deployments, the GrowthBook server may push false - // for gates we intentionally enable. Local defaults represent the - // project's intentional configuration and override everything except - // env/config overrides (which are explicit user intent). - const localDefault = getLocalGateDefault(feature) - if (localDefault !== undefined) { - return localDefault as T - } - // Log experiment exposure if data is available, otherwise defer until after init if (experimentDataByFeature.has(feature)) { logExposureForFeature(feature) } else { pendingExposures.add(feature) @@ try { const cached = getGlobalConfig().cachedGrowthBookFeatures?.[feature] if (cached !== undefined) { return cached as T } } catch { // Config not yet initialized — fall through to defaultValue } + const localDefault = getLocalGateDefault(feature) + if (localDefault !== undefined) { + return localDefault as T + } return defaultValueAlso 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, LOCAL_GATE_DEFAULTS being read before in-memory/disk caches in _CACHED_MAY_BE_STALE turns local defaults into a hard override; change _CACHED_MAY_BE_STALE so it checks the in-memory cache and disk cache first and only falls back to LOCAL_GATE_DEFAULTS when no cached value exists, ensuring getFeatureValueInternal and isKairosEnabled still reflect remote/ cached server answers; update the logic references in _CACHED_MAY_BE_STALE and any similar code paths (also adjust the duplicate behavior noted around the block affecting lines 834-842) so LOCAL_GATE_DEFAULTS remains the final fallback rather than the first source.src/jobs/__tests__/state.test.ts-65-71 (1)
65-71:⚠️ Potential issue | 🟠 MajorTest may fail due to module caching from previous test.
The
readJobStatetest creates'job-2'and reads it back. However, if the module was loaded with a differentCLAUDE_CONFIG_DIRthan whatbeforeEachset for this test, the job will be created in a different directory than wherereadJobStatelooks.This depends on the fix for the import timing issue above. Once imports are moved inside tests, this should work correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/__tests__/state.test.ts` around lines 65 - 71, The failing test is caused by module caching using a different CLAUDE_CONFIG_DIR at import time; to fix, ensure the module that defines createJob and readJobState is imported after setting up CLAUDE_CONFIG_DIR (or clear Node's module cache) inside the test setup—move the import of the module that exports createJob/readJobState into the test (or beforeEach) after setting process.env.CLAUDE_CONFIG_DIR, or call jest.resetModules() before importing so createJob and readJobState use the correct directory for this test.src/jobs/__tests__/state.test.ts-33-35 (1)
33-35:⚠️ Potential issue | 🟠 MajorTop-level import executes before
beforeEachsetsCLAUDE_CONFIG_DIR.The
await import('../state.js')runs once at module load time, before anybeforeEachhook. Ifstate.tsreadsCLAUDE_CONFIG_DIRduring module initialization (e.g., to compute a base path), it will capture an undefined or stale value.Move the import inside each test or use a fresh import per test to ensure proper isolation:
🔧 Proposed fix
-// ─── import ───────────────────────────────────────────────────────────────── - -const { createJob, readJobState, appendJobReply, getJobDir } = await import( - '../state.js' -) - // ─── tests ────────────────────────────────────────────────────────────────── describe('createJob', () => { test('creates job directory and writes state, template, and input files', async () => { + const { createJob } = await import('../state.js') const dir = createJob('job-1', 'my-template', '# Template', 'hello', [Based on learnings: "Use
mock.module()with inlineawait import()in test files for testing modules with heavy dependencies"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jobs/__tests__/state.test.ts` around lines 33 - 35, The tests currently perform a top-level await import('../state.js') which executes module initialization before beforeEach sets CLAUDE_CONFIG_DIR, causing stale/undefined config; change to import the module inside each test or inside beforeEach to force a fresh evaluation (e.g., use dynamic await import('../state.js') per test or use your test framework's mock.module() to create an isolated module instance) and then reference the exported functions createJob, readJobState, appendJobReply, and getJobDir from that fresh import so state.js reads the correct CLAUDE_CONFIG_DIR at test time.src/cli/handlers/templateJobs.ts-113-115 (1)
113-115:⚠️ Potential issue | 🟠 MajorDon't hand-roll YAML frontmatter serialization.
${k}: ${v}breaks for values containing:, newlines, quotes, arrays, or nested objects. Jobs created from richer templates will end up with invalid frontmatter or lossy data. Please serializetemplate.frontmatterthrough a YAML encoder before constructingrawContent.🤖 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 113 - 115, The frontmatter is being hand-serialized into rawContent which will corrupt values with colons, newlines, quotes or complex types; replace the manual Object.entries mapping with a proper YAML serializer (e.g., yaml.dump or YAML.stringify) to serialize template.frontmatter, import the YAML library at the top of templateJobs.ts, produce the frontmatter string via the serializer and then build rawContent as `---\n${serializedFrontmatter}---\n${template.content}` (ensure serializer options produce a plain block document without extra document markers if needed).src/utils/handlePromptSubmit.ts-609-629 (1)
609-629:⚠️ Potential issue | 🟠 MajorKeep autonomy completion bookkeeping out of the failure fallback.
This
catchnow covers both the turn execution and thefinalizeAutonomyRunCompleted()loop. If completion bookkeeping throws after the turn already succeeded, every collectedrunIdis sent throughfinalizeAutonomyRunFailed(), including runs that may already be marked completed. That can regress successful runs and duplicate follow-up enqueueing. Narrow the failurecatchto the execution phase, or track which runs were already completed before failing the remainder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/handlePromptSubmit.ts` around lines 609 - 629, The catch currently wraps both turn execution and the finalizeAutonomyRunCompleted loop, causing successful runs to be marked failed if the completion bookkeeping throws; narrow the failure handling so only execution errors mark runs failed or track completed runs before marking failures. Specifically, either move the try/catch so it only surrounds the execution phase (leaving the finalizeAutonomyRunCompleted loop outside) or collect completed runIds returned by finalizeAutonomyRunCompleted into a Set (use autonomyRunIds, finalizeAutonomyRunCompleted, enqueue) and in the catch only call finalizeAutonomyRunFailed for runIds not present in that completed Set; ensure you await finalizeAutonomyRunCompleted calls and only call finalizeAutonomyRunFailed for genuinely uncompleted runs.src/hooks/usePipeMuteSync.ts-91-103 (1)
91-103:⚠️ Potential issue | 🟠 MajorMove side effects outside the functional state updater.
The
setToolUseConfirmQueueupdater at lines 91–103 callsonAbort()inside the updater function. React may invoke updaters multiple times in Strict Mode, causing side effects to fire more than once. Compute the next queue state inside the updater (which must remain pure), then invoke the abort callbacks after the state change is committed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/usePipeMuteSync.ts` around lines 91 - 103, The updater passed to setToolUseConfirmQueue in usePipeMuteSync currently invokes onAbort() inside the functional state updater causing side effects during state calculation; change it so the updater remains pure by computing and returning the new queue (filter out items where item.pipeName === name) and also collect the toAbort callbacks (items where item.pipeName === name) inside the updater but do NOT call them there; after calling setToolUseConfirmQueue, iterate the collected callbacks and invoke each onAbort in a try/catch (preserving the existing ignore-on-error behavior). Ensure you update the code locations referencing setToolUseConfirmQueue and onAbort accordingly so the state update is pure and callbacks run only once after state commit.src/cli/bg/engines/detached.ts-17-33 (1)
17-33:⚠️ Potential issue | 🟠 MajorWait for spawn outcome before returning a detached session.
In Node.js,
spawn()returns immediately but the child process launch can still fail asynchronously. Spawn failures are reported via the child'serrorevent, not propagated to the returnedChildProcess. Currently the function returns as soon asspawn()succeeds, which means callers can record a session with a validpideven though the actual launch failed. Additionally,logFdwill leak ifspawn()throws before reachingcloseSync(logFd).Wrap
closeSync(logFd)in afinallyblock and wait for either thespawnevent (launch succeeded) orerrorevent (launch failed) before returning. This ensures callers only record a session once the process has actually started.🤖 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 17 - 33, The code currently opens logFd, calls spawn(...) and returns immediately which can leak logFd and allow recording a PID for a process that later fails; fix by wrapping the spawn call and closeSync(logFd) in a try/finally so logFd is always closed, and await the child's startup by returning a Promise that listens for the child's 'spawn' (resolve) and 'error' (reject) events before proceeding; ensure you call child.unref() only after spawn success and propagate the error if the child emits 'error' so callers don't record a failed session. Reference: openSync, spawn, child, closeSync, logFd, child.unref(), opts, entrypoint.src/assistant/AssistantSessionChooser.tsx-23-31 (1)
23-31:⚠️ Potential issue | 🟠 MajorGuard empty or shrinking
sessionsbefore usingfocusIndex.The modulo math and the non-null assertion both assume
sessions.length > 0and that the old index stays valid. If the list ever arrives empty, or shrinks while the chooser is open,focusIndexbecomes invalid andsessions[focusIndex]!.idwill throw. Clamp the index whensessions.lengthchanges and no-op/cancel when the array is empty.Possible fix
export function AssistantSessionChooser({ sessions, onSelect, onCancel }: Props): React.ReactNode { useRegisterOverlay('assistant-session-chooser'); const [focusIndex, setFocusIndex] = useState(0); + + React.useEffect(() => { + if (sessions.length === 0) { + onCancel(); + return; + } + setFocusIndex(i => Math.min(i, sessions.length - 1)); + }, [sessions.length, onCancel]); 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': () => { + const session = sessions[focusIndex]; + if (session) onSelect(session.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 23 - 31, AssistantSessionChooser currently assumes sessions.length > 0 when using focusIndex and sessions[focusIndex]!.id inside the useKeybindings handlers; clamp and guard the index whenever sessions changes and avoid accessing sessions when empty: add an effect that watches sessions.length and calls setFocusIndex(i => Math.min(i, Math.max(0, sessions.length - 1))) (or resets to 0 when empty), and update the 'select:accept' handler to first check if sessions.length === 0 (call onCancel or no-op) before calling onSelect(sessions[focusIndex].id); ensure the 'select:next'/'select:previous' handlers also no-op when sessions.length === 0 to avoid modulo-by-zero.src/commands/daemon/daemon.tsx-44-55 (1)
44-55:⚠️ Potential issue | 🟠 MajorAvoid process-wide console monkey-patching here.
Patching
console.logandconsole.errorglobally creates two problems: (1) unrelated async work running during the handler can leak logs intolines, and (2) concurrent/daemoncommands can race and corrupt each other's output if the REPL allows parallel execution. Instead, pass a logger/output sink parameter intohandleBgStart()anddaemonMain()so they write to a captured stream without mutating global state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/daemon/daemon.tsx` around lines 44 - 55, captureConsole is monkey-patching process-wide console (console.log/console.error) which can leak unrelated async logs and race between concurrent /daemon invocations; instead remove the global patching and add a local output sink/logger parameter that you pass into handleBgStart() and daemonMain() so they write to that sink directly. Update captureConsole to create/own a per-invocation string buffer or writable stream and call handleBgStart({ logger }) / daemonMain({ logger }) (or add logger args to their signatures), refactor any console.log/error usages inside handleBgStart and daemonMain to use the injected logger/output sink, and update all call sites to pass a fresh sink to avoid global state and races.src/hooks/useScheduledTasks.ts-74-85 (1)
74-85:⚠️ Potential issue | 🟠 MajorCatch these fire-and-forget scheduler branches.
Both new paths are launched with
voidand no terminal.catch(). IfcreateAutonomyQueuedPrompt()ormarkAutonomyRunFailed()rejects, the scheduler tick turns into an unhandled rejection.Suggested fix
onFire: prompt => { - void enqueueForLead(prompt) + void enqueueForLead(prompt).catch(error => { + logForDebugging( + `[ScheduledTasks] failed to enqueue lead cron: ${String(error)}`, + ) + }) }, @@ onFireTask: task => { void (async () => { @@ - })() + })().catch(error => { + logForDebugging( + `[ScheduledTasks] failed to process cron ${task.id}: ${String(error)}`, + ) + }) },Also applies to: 92-159
🤖 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 - 85, The scheduler is launching fire-and-forget promises without handling rejections (e.g., in enqueueForLead calling createAutonomyQueuedPrompt and enqueuePendingNotification), causing unhandled rejections; update these branches (including other similar spots between the 92-159 area) to always handle errors by either awaiting the async call inside try/catch or appending a .catch(...) to the promise chain, and on error call markAutonomyRunFailed(...) and/or log the error so failures are surfaced; specifically modify enqueueForLead, the places that call createAutonomyQueuedPrompt, and any void-started flows to ensure .catch or try/catch handles rejected promises and invokes markAutonomyRunFailed with the caught error.src/cli/bg/engines/tmux.ts-18-29 (1)
18-29:⚠️ Potential issue | 🟠 MajorPass session metadata to tmux via
-eflag, not only through client env.When
new-sessionconnects to an existing tmux server, custom environment variables from the client process do not automatically propagate into the new session. TheCLAUDE_CODE_*variables will be missing from the bg child process unless explicitly passed totmuxwith-e VAR=value. Construct both the spawn process env (fromprocess.env+ session vars) and the tmux args separately.Suggested fix
- const tmuxEnv: Record<string, string | undefined> = { + const sessionEnv: Record<string, string | undefined> = { ...opts.env, CLAUDE_CODE_SESSION_KIND: 'bg', CLAUDE_CODE_SESSION_NAME: opts.sessionName, CLAUDE_CODE_SESSION_LOG: opts.logPath, CLAUDE_CODE_TMUX_SESSION: opts.sessionName, } + const spawnEnv: Record<string, string | undefined> = { + ...process.env, + ...sessionEnv, + } + const envArgs = Object.entries(sessionEnv).flatMap(([key, value]) => + value === undefined ? [] : ['-e', `${key}=${value}`], + ) const result = spawnSync( 'tmux', - ['new-session', '-d', '-s', opts.sessionName, cmd], - { stdio: 'inherit', env: tmuxEnv }, + ['new-session', '-d', '-s', opts.sessionName, ...envArgs, cmd], + { stdio: 'inherit', env: spawnEnv }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/bg/engines/tmux.ts` around lines 18 - 29, The tmux session env vars are only being set in tmuxEnv (used as the child process env) but not passed to tmux itself, so when tmux connects to an existing server the CLAUDE_CODE_* vars are missing; update the spawn call that uses spawnSync('tmux', ['new-session', ...], { env: tmuxEnv }) to (1) build tmuxEnv by spreading process.env and then overlaying the session-specific keys (use tmuxEnv = { ...process.env, ...opts.env, CLAUDE_CODE_SESSION_KIND: 'bg', CLAUDE_CODE_SESSION_NAME: opts.sessionName, CLAUDE_CODE_SESSION_LOG: opts.logPath, CLAUDE_CODE_TMUX_SESSION: opts.sessionName }), and (2) append -e 'VAR=value' arguments for each CLAUDE_CODE_* (and any opts.env entries) to the tmux args array (the arguments passed alongside opts.sessionName and cmd) so tmux receives the variables even when reusing an existing server; keep using spawnSync with the updated args and the constructed tmuxEnv for the child process.src/hooks/useMasterMonitor.ts-121-140 (1)
121-140:⚠️ Potential issue | 🟠 MajorLet muted slaves still report terminal events.
With
'done'and'error'inMUTED_DROPPABLE_TYPES, muting a slave mid-turn makes both listeners return before the master updates that slave out ofbusy. The session can stay stuck until disconnect/unmute, and/statusstops reflecting reality. If you still want those messages hidden from history, special-case them after the state update instead of filtering them out here.Smallest safe change
const MUTED_DROPPABLE_TYPES = new Set([ 'prompt_ack', 'stream', 'tool_start', 'tool_result', - 'done', - 'error', 'permission_request', 'permission_cancel', ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useMasterMonitor.ts` around lines 121 - 140, The current mute gate in MUTED_DROPPABLE_TYPES causes terminal events to be dropped so update it to not include 'done' and 'error' (i.e., remove those two entries from the MUTED_DROPPABLE_TYPES set) and rely on post-state-update special-casing for hiding terminal events from history; modify the constant and leave shouldDropMutedMessage(slaveName, msgType) unchanged so terminal events still pass through the gate and can update slave busy/status before any history-filtering logic runs.src/cli/print.ts-1862-1878 (1)
1862-1878:⚠️ Potential issue | 🟠 MajorHandle failures inside these detached autonomy tasks.
These callbacks launch async IIFEs with
voidand no localcatch. IfcreateProactiveAutonomyCommands,prepareAutonomyTurnPrompt,commitAutonomyQueuedPrompt, ormarkAutonomyRunFailedrejects, the scheduler callback has nowhere to report it and the rejection escapes unhandled. Wrap each body intry/catchand log/fail the affected run explicitly.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 detached async IIFEs launched with void (async () => { ... }) currently lack error handling and can produce unhandled rejections; update each such IIFE (the one that calls createProactiveAutonomyCommands and the similar block around lines 2787-2849) to wrap the entire body in try/catch, catching any rejection from createProactiveAutonomyCommands, prepareAutonomyTurnPrompt, commitAutonomyQueuedPrompt, markAutonomyRunFailed, run, or enqueue; in the catch, log the error (using the same logger used elsewhere) and explicitly mark the affected run failed by calling markAutonomyRunFailed or otherwise recording the failure (include the run/command UUID if available), and ensure the catch prevents the rejection from escaping the scheduler callback.src/commands/assistant/assistant.tsx-78-87 (1)
78-87:⚠️ Potential issue | 🟠 MajorCancel the delayed success callback on abort paths.
onInstalled(dir)is always scheduled here, so a latererrorevent still leaves the success path armed. The same timer also survives a user cancel whilestartingis true. Keep the timeout handle in a ref and clear it from every failure/cancel path before reporting success.🤖 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 78 - 87, The scheduled setTimeout that calls onInstalled(dir) is never cleared, so later error paths or a user cancel can still trigger the success callback; store the timer ID (from setTimeout) in a variable or ref (e.g., installTimeoutRef) when you schedule it in the start flow, then clearTimeout(installTimeoutRef) from every failure/abort path (e.g., inside the child.on('error' handler and any user cancel/abort handler when starting is true) before calling onError or performing cancellation, and ensure you null out the ref after clearing to avoid double-clear and accidental success callbacks.src/cli/print.ts-2794-2804 (1)
2794-2804:⚠️ Potential issue | 🟠 MajorDon't strand committed cron runs when shutdown wins the race.
commitAutonomyQueuedPrompt()has already persisted the run at this point. IfinputClosedflips beforeenqueue(...), these branches just return, so the run is never executed and never marked failed. Mirror thetask.agentIdbranch and fail the committed run before returning.Suggested change
const command = await commitAutonomyQueuedPrompt({ prepared, currentDir: cwd(), sourceId: task.id, sourceLabel: task.prompt, workload: WORKLOAD_CRON, }) - if (inputClosed) return + if (inputClosed) { + await markAutonomyRunFailed( + command.autonomy!.runId, + 'Headless input closed before the queued autonomy run could be enqueued.', + ) + return + } enqueue({ ...command, uuid: randomUUID(), })Also applies to: 2836-2848
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/print.ts` around lines 2794 - 2804, The code calls commitAutonomyQueuedPrompt(...) which persists a cron run, then checks inputClosed and returns before enqueue(...), risking a stranded persisted run; change the early-return to mirror the existing task.agentId branch by invoking the same failure path used there to mark the committed run as failed (i.e., call the function or code used in the task.agentId branch to mark the persisted run failed and log/error accordingly) before returning, and apply the same fix to the analogous block later in the file (the similar commitAutonomyQueuedPrompt -> inputClosed -> enqueue sequence).src/cli/print.ts-2175-2177 (1)
2175-2177:⚠️ Potential issue | 🟠 MajorKeep
markAutonomyRunRunning()inside the same fail/finalize envelope.If this transition throws, control jumps straight to the outer
run()catch and none of these autonomy runs are finalized as failed. That leaves persisted run state stranded beforeask()even starts.Suggested change
- for (const runId of autonomyRunIds) { - await markAutonomyRunRunning(runId) - } let lastResultIsError = false try { + for (const runId of autonomyRunIds) { + await markAutonomyRunRunning(runId) + } await runWithWorkload(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/print.ts` around lines 2175 - 2177, The loop calling markAutonomyRunRunning(runId) is outside the fail/finalize envelope, so if markAutonomyRunRunning throws we skip the per-run failure/finalization logic; move the call for each autonomyRunId into the same try/catch/finally (or the existing per-run failure handling block) that surrounds ask()/run() so any exception from markAutonomyRunRunning triggers the same finalize-as-failed path for that runId; update the autonomyRunIds loop to call markAutonomyRunRunning inside that envelope (or wrap it in its own try that forwards errors into the existing finalize/fail logic) so persisted run state is never left stranded before ask() starts.src/screens/REPL.tsx-4843-4859 (1)
4843-4859:⚠️ Potential issue | 🟠 MajorDon't finalize aborted autonomy runs as successful low-priority completions.
onQuery()can resolve withnewAbortController.signal.aborted === true, and this success path still callsfinalizeAutonomyRunCompleted()with a hardcoded'later'. That means an interrupted"now"step can be recorded as a successful completion, advance the flow incorrectly, and suppress the next-step interrupt behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/REPL.tsx` around lines 4843 - 4859, The code currently finalizes autonomy runs regardless of whether the onQuery call completed due to an abort; update the success path after onQuery (where autonomyRunId is used) to first check newAbortController.signal.aborted and skip calling finalizeAutonomyRunCompleted (and enqueueing next commands) if aborted; ensure this check is applied around the finalizeAutonomyRunCompleted(...) call that uses autonomyRunId, getCwd(), and enqueue so only genuinely completed runs are marked as completed and progressed.src/tasks/InProcessTeammateTask/InProcessTeammateTask.tsx-126-140 (1)
126-140:⚠️ Potential issue | 🟠 MajorPrefer live idle teammates over terminal fallback entries.
This helper only special-cases
running, so if an old killed/completed task is encountered before the current idle task for the sameagentId, it returns the dead entry. That breaks lookups for teammates that are waiting for input.Suggested fix
- if (!fallback) { + if ( + !fallback || + (isTerminalTaskStatus(fallback.status) && !isTerminalTaskStatus(task.status)) + ) { fallback = task; }🤖 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 lookup loop in InProcessTeammateTask (using isInProcessTeammateTask and comparing task.identity.agentId to agentId) only prioritizes status === 'running' and otherwise returns the first match, which can return a terminal/killed task before a live 'idle' teammate; change the selection logic to prefer running, then prefer 'idle' over terminal states: track an idleCandidate and a terminalFallback while iterating, return the running task immediately, if none return idleCandidate if set, otherwise return terminalFallback (or undefined) to ensure waiting teammates are found over old terminal entries.src/utils/autonomyPersistence.ts-19-22 (1)
19-22:⚠️ Potential issue | 🟠 MajorFix lock-map cleanup comparison to prevent key leakage.
At Line 19–22 the map stores
previous.then(() => current), but Line 44 compares againstcurrent, so the delete condition never succeeds. This leaves stale entries perrootDir.Suggested fix
- persistenceLocks.set( - key, - previous.then(() => current), - ) + const queued = previous.then(() => current) + persistenceLocks.set(key, queued) @@ - if (persistenceLocks.get(key) === current) { + if (persistenceLocks.get(key) === queued) { persistenceLocks.delete(key) }Also applies to: 44-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/autonomyPersistence.ts` around lines 19 - 22, The lock-map stores previous.then(() => current) but later compares against current, so the delete check never matches and keys leak; fix by creating a single chained promise variable (e.g., const chained = previous.then(() => current)), store that chained promise in persistenceLocks via persistenceLocks.set(key, chained), and then use that same chained reference in the cleanup comparison (instead of comparing to current) so the delete condition can succeed; update both the persistenceLocks.set usage and the corresponding delete check where persistenceLocks.get(key) is compared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4167f518-dca0-4456-874e-cdacbd1dc013
📒 Files selected for processing (112)
.gitignoreDEV-LOG.mdbuild.tspackages/@ant/computer-use-mcp/src/toolCalls.tspackages/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/assistant/AssistantSessionChooser.ts
- src/commands/assistant/assistant.ts
| feature("KAIROS") && | ||
| assistantModule?.isAssistantMode() && | ||
| assistantModule && | ||
| (assistantModule.isAssistantForced() || | ||
| (options as Record<string, unknown>).assistant === true) && |
There was a problem hiding this comment.
Don't drop the persisted assistant-mode path.
After markAssistantForced() runs above, this predicate only admits forced/CLI sessions. In an already-trusted repo, .claude/settings.json assistant: true no longer reaches the activation block, even though Line 1799 still says isAssistantMode() runs below. If the goal was to avoid touching project settings before trust, defer assistantModule.isAssistantMode() until after the trust check instead of removing it from the flow.
Suggested fix
if (
feature("KAIROS") &&
assistantModule &&
- (assistantModule.isAssistantForced() ||
- (options as Record<string, unknown>).assistant === true) &&
// Spawned teammates share the leader's cwd + settings.json, so
// the flag is true for them too. --agent-id being set
// means we ARE a spawned teammate (extractTeammateOptions runs
// ~170 lines later so check the raw commander option) — don't
// re-init the team or override teammateMode/proactive/brief.
!(options as { agentId?: unknown }).agentId &&
kairosGate
) {
- if (!checkHasTrustDialogAccepted()) {
+ const trustAccepted = checkHasTrustDialogAccepted();
+ if (!trustAccepted) {
// biome-ignore lint/suspicious/noConsole:: intentional console output
console.warn(
chalk.yellow(
"Assistant mode disabled: directory is not trusted. Accept the trust dialog and restart.",
),
);
- } else {
+ } else if (assistantModule.isAssistantMode()) {
// Blocking gate check — returns cached `true` instantly; if disk
// cache is false/missing, lazily inits GrowthBook and fetches fresh
// (max ~5s). --assistant skips the gate entirely (daemon is🤖 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 current predicate using
assistantModule.isAssistantForced() and the CLI/options check prevents the
persisted assistant-mode path from activating; restore that by including
assistantModule.isAssistantMode() in the activation flow but only evaluate it
after the trust check (i.e., do not call isAssistantMode() before
markAssistantForced()/trust gating). Concretely: adjust the condition around
feature("KAIROS") and assistantModule so that forced/CLI sessions remain handled
as now, but move or add a branch that checks assistantModule.isAssistantMode()
after the repository trust check (after markAssistantForced()) so persisted
`.claude/settings.json` with assistant: true still reaches the activation block.
|
Content already merged into lint/preview branch via direct push. |
1 participant
Summary
Features
/langcommand, mute state machineMIME Detection Fix
packages/@ant/computer-use-mcp/src/toolCalls.ts:detectMimeFromBase64()decoded raw bytes from base64 instead of brokencharCodeAtcomparisonOther
Verification
bunx tsc --noEmit: 0 errorsbun test: 2758 pass, 0 failTest plan
bunx tsc --noEmitzero errorsbun test2758 pass / 0 failSummary by CodeRabbit
New Features
/daemoncommand supporting session tracking, logs, and lifecycle control/jobcommand for creating and managing template-based workflows/langcommand supporting English, Chinese, and auto-detectionBug Fixes
Tests
Refactor