fixup: address CodeRabbit review on PR #386

Twelve actionable items (7 Major + 5 Minor) from the CodeRabbit review on
claude-code-best/claude-code#386:

- docs/internals/autonomy-jira.md: typo "due input close" → "due to input close".
- src/utils/autonomyRuns.ts:
  - selectPersistedAutonomyRuns no longer evicts active (queued/running) runs
    when the combined list exceeds AUTONOMY_RUNS_MAX. Active runs are kept in
    full and the inactive history is capped to the remaining budget so
    persisted ownership for live work survives.
  - isValidOwnerProcessId now allows pid <= 4_194_304 so a live run owned by
    the maximum Linux PID is not treated as stale.
- src/utils/autonomyAuthority.ts: maskCodeFencedLines tracks the active fence
  length and only closes the fence when a same-character run of equal-or-
  greater length appears with no trailing content, so a nested ```yaml inside
  an outer ```` block no longer leaks fake `tasks:` entries into the parser.
- src/cli/print.ts: late-shutdown branches in the cron and scheduled-task
  paths now call cancelQueuedAutonomyCommands({ commands: [command] }) instead
  of markAutonomyRunCancelled(...). Updating run state alone left the
  queue-side record orphaned for resume/recovery.
- src/utils/processUserInput/processSlashCommand.tsx: scheduled-task-result
  notification is enqueued before finalizeAutonomyRunCompleted (which queues
  follow-up autonomy commands) so both at priority: 'later' land in order and
  the next autonomy step can not run before the worker's output is observed.
- src/screens/REPL.tsx + src/utils/handlePromptSubmit.ts:
  - onQuery now returns Promise<boolean>: false from the concurrent-guard
    skip path, true otherwise. Other call sites use `void onQuery(...)` and
    are unaffected. handlePromptSubmit's onQuery prop type matches.
  - The autonomy-prompt callsite captures the executed flag, finalizes
    claim.claimedCommands as { type: 'completed' } only when onQuery actually
    ran, and runs the completed-finalize in its own try/catch so a failure
    there does not propagate into the outer catch and trigger a second
    finalize as { type: 'failed' } for the same commands.
  - Removed the unsafe `command.value as string` cast; createUserMessage
    already accepts `string | ContentBlockParam[]`.
  - createUserMessage mock in src/__tests__/handlePromptSubmit.test.ts now
    matches the new Promise<boolean> shape.
- packages/builtin-tools/src/tools/RemoteTriggerTool/__tests__/
  RemoteTriggerTool.test.ts:
  - Inline auth mock replaced with the shared tests/mocks/auth (added).
  - The full mock of src/constants/oauth.js is replaced by a narrow
    side-effect-only mock that overrides the env-reading helpers
    (getOauthConfig, fileSuffixForOauthConfig, MCP_CLIENT_METADATA_URL) and
    delegates pure data exports to the real module.
- tests/integration/dependency-overrides.test.ts:
  - mermaid does not export `./package.json` in its exports map, so
    require.resolve('mermaid/package.json') throws
    ERR_PACKAGE_PATH_NOT_EXPORTED in runtimes that honor exports semantics.
    The test now resolves the package entry and walks up to the package
    root via a small findPackageJson helper.
  - readFileSync from node:fs is replaced with `await Bun.file(...).text()`
    to match the project's Bun-API requirement.

Validation:
- bun run typecheck (clean).
- bun test → 3996 pass / 0 fail across 305 test files.

Targets PRs:
- amDosion/claude-code-bast#8 (fork-internal review)
- claude-code-best/claude-code#386 (upstream review, same head branch)

docs/internals/autonomy-jira.md

@@ -260,7 +260,7 @@ Acceptance criteria:
 - Proactive hook cancellation checks run both before commit and after command
   creation.
 - Headless proactive and cron paths cancel any already-created command that is
-  dropped due input close.
+  dropped due to input close.
 - REPL scheduled-task cleanup cancels already-created commands when unmounted.
 - A regression test verifies a proactive command created but dropped before
   enqueue is marked cancelled.

packages/builtin-tools/src/tools/RemoteTriggerTool/__tests__/RemoteTriggerTool.test.ts

@@ -1,4 +1,5 @@
 import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
+import { authMock } from '../../../../../../tests/mocks/auth'
 
 let requestStatus = 200
 const auditRecords: Record<string, unknown>[] = []
@@ -12,11 +13,7 @@ mock.module('axios', () => ({
   },
 }))
 
-mock.module('src/utils/auth.js', () => ({
+mock.module('src/utils/auth.js', authMock)
-  checkAndRefreshOAuthTokenIfNeeded: async () => {},
-  getClaudeAIOAuthTokens: () => ({ accessToken: 'token' }),
-  isClaudeAISubscriber: () => true,
-}))
 
 mock.module('src/services/oauth/client.js', () => ({
   getOrganizationUUID: async () => 'org',
@@ -30,17 +27,19 @@ mock.module('src/services/policyLimits/index.js', () => ({
   isPolicyAllowed: () => true,
 }))
 
-mock.module('src/constants/oauth.js', () => ({
+// Narrow mock for the side-effectful entries in `src/constants/oauth.js`.
-  ALL_OAUTH_SCOPES: ['user:profile', 'user:inference'],
+// Pure data exports (ALL_OAUTH_SCOPES, CLAUDE_AI_*_SCOPE, etc.) come from
-  CLAUDE_AI_INFERENCE_SCOPE: 'user:inference',
+// the real module and are not mocked, per the test policy that constants
-  CLAUDE_AI_OAUTH_SCOPES: ['user:profile', 'user:inference'],
+// modules without side effects should not be replaced wholesale.
-  CLAUDE_AI_PROFILE_SCOPE: 'user:profile',
+mock.module('src/constants/oauth.js', () => {
-  CONSOLE_OAUTH_SCOPES: ['org:create_api_key', 'user:profile'],
+  const actual = require('../../../../../../src/constants/oauth.js')
-  MCP_CLIENT_METADATA_URL: 'https://example.test/oauth/metadata',
+  return {
-  OAUTH_BETA_HEADER: 'oauth-test',
+    ...actual,
     fileSuffixForOauthConfig: () => '',
     getOauthConfig: () => ({ BASE_API_URL: 'https://example.test' }),
-}))
+    MCP_CLIENT_METADATA_URL: 'https://example.test/oauth/metadata',
+  }
+})
 
 mock.module('src/utils/remoteTriggerAudit.js', () => ({
   appendRemoteTriggerAuditRecord: async (

src/__tests__/handlePromptSubmit.test.ts

@@ -38,9 +38,9 @@ function createBaseParams() {
     commands: [],
     setUserInputOnProcessing: mock((_prompt?: string) => {}),
     setAbortController: mock((_abortController: AbortController | null) => {}),
-    onQuery: mock(async () => undefined) as unknown as (
+    onQuery: mock(async () => true) as unknown as (
       ...args: unknown[]
-    ) => Promise<void>,
+    ) => Promise<boolean>,
     setAppState: mock((_updater: unknown) => {}),
   }
 }

src/cli/print.ts

@@ -2839,10 +2839,7 @@ function runHeadlessStreaming(
             workload: WORKLOAD_CRON,
           })
           if (inputClosed) {
-            await markAutonomyRunCancelled(
+            await cancelQueuedAutonomyCommands({ commands: [command] })
-              command.autonomy!.runId,
-              command.autonomy!.rootDir,
-            )
             return
           }
           enqueue({
@@ -2875,10 +2872,7 @@ function runHeadlessStreaming(
             })
             if (!command) return
             if (inputClosed) {
-              await markAutonomyRunCancelled(
+              await cancelQueuedAutonomyCommands({ commands: [command] })
-                command.autonomy!.runId,
-                command.autonomy!.rootDir,
-              )
               return
             }
             await markAutonomyRunFailed(
@@ -2899,10 +2893,7 @@ function runHeadlessStreaming(
           })
           if (!command) return
           if (inputClosed) {
-            await markAutonomyRunCancelled(
+            await cancelQueuedAutonomyCommands({ commands: [command] })
-              command.autonomy!.runId,
-              command.autonomy!.rootDir,
-            )
             return
           }
           enqueue({

src/screens/REPL.tsx

@@ -3474,7 +3474,7 @@ export function REPL({
       onBeforeQueryCallback?: (input: string, newMessages: MessageType[]) => Promise<boolean>,
       input?: string,
       effort?: EffortValue,
-    ): Promise<void> => {
+    ): Promise<boolean> => {
       // If this is a teammate, mark them as active when starting a turn
       if (isAgentSwarmsEnabled()) {
         const teamName = getTeamName();
@@ -3505,7 +3505,7 @@ export function REPL({
               logEvent('tengu_concurrent_onquery_enqueued', {});
             }
           });
-        return;
+        return false;
       }
 
       try {
@@ -3538,7 +3538,7 @@ export function REPL({
         if (onBeforeQueryCallback && input) {
           const shouldProceed = await onBeforeQueryCallback(input, latestMessages);
           if (!shouldProceed) {
-            return;
+            return true;
           }
         }
 
@@ -3687,6 +3687,7 @@ export function REPL({
           }
         }
       }
+      return true;
     },
     [onQueryImpl, setAppState, resetLoadingState, queryGuard, mrOnBeforeQuery, mrOnTurnComplete],
   );
@@ -4851,13 +4852,37 @@ export function REPL({
 
         // Create a user message with the formatted content (includes XML wrapper)
         const userMessage = createUserMessage({
-          content: command.value as string,
+          content: command.value,
           isMeta: command.isMeta ? true : undefined,
           origin: command.origin,
         });
 
+        let executed = false;
+        try {
+          executed = (await onQuery([userMessage], newAbortController, true, [], mainLoopModel)) !== false;
+        } catch (error: unknown) {
+          try {
+            await finalizeAutonomyCommandsForTurn({
+              commands: claim.claimedCommands,
+              outcome: { type: 'failed', error },
+              currentDir: getCwd(),
+              priority: 'later',
+            });
+          } catch (finalizeError: unknown) {
+            logError(toError(finalizeError));
+          }
+          logError(toError(error));
+          return;
+        }
+
+        // Only finalize as completed when onQuery actually executed the turn
+        // (it returns false from the concurrent-guard path without running).
+        // Keep this finalize in its own try/catch so a failure here does not
+        // trigger a second finalize as `failed` for the same commands.
+        if (!executed) {
+          return;
+        }
         try {
-          await onQuery([userMessage], newAbortController, true, [], mainLoopModel);
           const nextCommands = await finalizeAutonomyCommandsForTurn({
             commands: claim.claimedCommands,
             outcome: { type: 'completed' },
@@ -4867,14 +4892,8 @@ export function REPL({
           for (const nextCommand of nextCommands) {
             enqueue(nextCommand);
           }
-        } catch (error: unknown) {
+        } catch (finalizeError: unknown) {
-          await finalizeAutonomyCommandsForTurn({
+          logError(toError(finalizeError));
-            commands: claim.claimedCommands,
-            outcome: { type: 'failed', error },
-            currentDir: getCwd(),
-            priority: 'later',
-          });
-          logError(toError(error));
         }
       })().catch((error: unknown) => {
         logError(toError(error));

src/utils/autonomyAuthority.ts

@@ -143,15 +143,24 @@ function mergeAgentsAuthority(files: AutonomyAuthorityFile[]): string | null {
 function maskCodeFencedLines(lines: string[]): string[] {
   const masked = lines.slice()
   let activeFenceChar: '`' | '~' | null = null
+  let activeFenceLen = 0
   for (let i = 0; i < masked.length; i++) {
     const trimmed = masked[i]!.trim()
-    const fenceMatch = trimmed.match(/^(```+|~~~+)/)
+    const fenceMatch = trimmed.match(/^([`~])\1{2,}/)
     if (fenceMatch) {
-      const fenceChar = fenceMatch[1]![0] as '`' | '~'
+      const fenceChar = fenceMatch[1]! as '`' | '~'
+      const fenceLen = fenceMatch[0]!.length
+      const trailing = trimmed.slice(fenceLen)
       if (activeFenceChar === null) {
         activeFenceChar = fenceChar
-      } else if (activeFenceChar === fenceChar) {
+        activeFenceLen = fenceLen
+      } else if (
+        activeFenceChar === fenceChar &&
+        fenceLen >= activeFenceLen &&
+        trailing.trim() === ''
+      ) {
         activeFenceChar = null
+        activeFenceLen = 0
       }
       masked[i] = ''
       continue

src/utils/autonomyRuns.ts

@@ -130,20 +130,18 @@ function isAutonomyRunActive(run: AutonomyRunRecord): boolean {
 function selectPersistedAutonomyRuns(
   runs: AutonomyRunRecord[],
 ): AutonomyRunRecord[] {
-  const retained = runs
+  const cloned = runs.slice().map(cloneRunRecord)
-    .slice()
+  const active = cloned
-    .map(cloneRunRecord)
+    .filter(isAutonomyRunActive)
-    .sort((left, right) => {
+    .sort((left, right) => right.createdAt - left.createdAt)
-      const leftActive = isAutonomyRunActive(left)
+  const history = cloned
-      const rightActive = isAutonomyRunActive(right)
+    .filter(run => !isAutonomyRunActive(run))
-      if (leftActive !== rightActive) {
+    .sort((left, right) => right.createdAt - left.createdAt)
-        return leftActive ? -1 : 1
+    .slice(0, Math.max(0, AUTONOMY_RUNS_MAX - active.length))
-      }
-      return right.createdAt - left.createdAt
-    })
-    .slice(0, AUTONOMY_RUNS_MAX)
 
-  return retained.sort((left, right) => right.createdAt - left.createdAt)
+  return [...active, ...history].sort(
+    (left, right) => right.createdAt - left.createdAt,
+  )
 }
 
 function normalizePersistedRunRecord(
@@ -260,7 +258,7 @@ function isValidOwnerProcessId(pid: number | undefined): pid is number {
     typeof pid === 'number' &&
     Number.isInteger(pid) &&
     pid > 0 &&
-    pid < 4_194_304
+    pid <= 4_194_304
   )
 }
 
@@ -407,10 +405,7 @@ async function persistAutonomyRunRecord(
           continue
         }
         if (isStaleActiveAutonomyRun(run)) {
-          const recovered = recoverStaleActiveAutonomyRun(
+          const recovered = recoverStaleActiveAutonomyRun(run, record.createdAt)
-            run,
-            record.createdAt,
-          )
           runs[i] = recovered
           recoveredStaleRuns.push(recovered)
           staleRecoveriesApplied = true

src/utils/handlePromptSubmit.ts

@@ -74,7 +74,7 @@ type BaseExecutionParams = {
     onBeforeQuery?: (input: string, newMessages: Message[]) => Promise<boolean>,
     input?: string,
     effort?: EffortValue,
-  ) => Promise<void>
+  ) => Promise<boolean>
   setAppState: (updater: (prev: AppState) => AppState) => void
   onBeforeQuery?: (input: string, newMessages: Message[]) => Promise<boolean>
   canUseTool?: CanUseToolFn

src/utils/processUserInput/processSlashCommand.tsx

@@ -260,8 +260,13 @@ async function executeForkedSlashCommand(
       }
       const resultText = extractResultText(agentMessages, 'Command completed');
       logForDebugging(`Background forked command /${commandName} completed (agent ${agentId})`);
-      await finalizeDeferredAutonomyRunCompleted();
+      // Enqueue the worker's result before finalizing the autonomy run so the
+      // <scheduled-task-result> notification is observed before any follow-up
+      // autonomy commands the finalizer enqueues at the same priority. Without
+      // this ordering, both land at `priority: 'later'` and the next autonomy
+      // step can run before the main thread sees this worker's output.
       enqueueResult(`<scheduled-task-result command="/${commandName}">\n${resultText}\n</scheduled-task-result>`);
+      await finalizeDeferredAutonomyRunCompleted();
     })().catch(async err => {
       logError(err);
       enqueueResult(

tests/integration/dependency-overrides.test.ts

@@ -1,14 +1,43 @@
 import { describe, expect, test } from 'bun:test'
-import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { createRequire } from 'node:module'
 import { tmpdir } from 'node:os'
-import { join, resolve } from 'node:path'
+import { dirname, join, resolve } from 'node:path'
 import { pathToFileURL } from 'node:url'
 
 const repoRoot = resolve(import.meta.dir, '..', '..')
 const uuidV4Pattern =
   /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/
 
+async function findPackageJson(
+  startPath: string,
+  expectedName: string,
+): Promise<string> {
+  let current = dirname(startPath)
+  for (let depth = 0; depth < 10; depth++) {
+    const candidate = join(current, 'package.json')
+    const file = Bun.file(candidate)
+    if (await file.exists()) {
+      try {
+        const parsed = JSON.parse(await file.text()) as { name?: unknown }
+        if (parsed.name === expectedName) {
+          return candidate
+        }
+      } catch {
+        // ignore parse errors and keep walking up
+      }
+    }
+    const parent = dirname(current)
+    if (parent === current) {
+      break
+    }
+    current = parent
+  }
+  throw new Error(
+    `package.json with name "${expectedName}" not found above ${startPath}`,
+  )
+}
+
 describe('dependency security overrides', () => {
   test('mcpb can load patched inquirer prompts from its package context', async () => {
     const mcpbRequire = createRequire(import.meta.resolve('@anthropic-ai/mcpb'))
@@ -28,10 +57,7 @@ describe('dependency security overrides', () => {
     )
     const gaxios = vertexRequire('gaxios') as {
       request(options: {
-        adapter(options: {
+        adapter(options: { headers: Headers; url: string }): Promise<{
-          headers: Headers
-          url: string
-        }): Promise<{
           config: unknown
           data: string
           headers: Record<string, string>
@@ -47,8 +73,10 @@ describe('dependency security overrides', () => {
 
     const response = await gaxios.request({
       url: 'https://example.com/upload',
-      multipart: [{ body: 'payload', headers: { 'Content-Type': 'text/plain' } }],
+      multipart: [
-      adapter: async (options) => {
+        { body: 'payload', headers: { 'Content-Type': 'text/plain' } },
+      ],
+      adapter: async options => {
         contentType = options.headers.get('content-type') ?? undefined
         return {
           config: options,
@@ -62,14 +90,14 @@ describe('dependency security overrides', () => {
     })
 
     expect(response.status).toBe(200)
-    expect(contentType).toMatch(
+    expect(contentType).toMatch(/^multipart\/related; boundary=[0-9a-f-]{36}$/)
-      /^multipart\/related; boundary=[0-9a-f-]{36}$/,
-    )
     expect(contentType?.split('boundary=')[1]).toMatch(uuidV4Pattern)
   })
 
   test('azure identity msal guid generation works through its package context', () => {
-    const identityRequire = createRequire(import.meta.resolve('@azure/identity'))
+    const identityRequire = createRequire(
+      import.meta.resolve('@azure/identity'),
+    )
     const msal = identityRequire('@azure/msal-node') as {
       CryptoProvider: new () => { createNewGuid(): string }
     }
@@ -91,11 +119,13 @@ describe('dependency security overrides', () => {
       pathToFileURL(streamdownRequire.resolve('uuid')).href
     )) as { v4(): string }
     const mermaidPath = streamdownRequire.resolve('mermaid')
-    const mermaidPackagePath = streamdownRequire.resolve(
+    // mermaid does not export ./package.json in its exports map, so resolving
-      'mermaid/package.json',
+    // 'mermaid/package.json' throws ERR_PACKAGE_PATH_NOT_EXPORTED in runtimes
-    )
+    // that honor exports semantics. Walk up from the resolved entry until a
+    // package.json with name === 'mermaid' is found.
+    const mermaidPackagePath = await findPackageJson(mermaidPath, 'mermaid')
     const mermaidPackage = JSON.parse(
-      readFileSync(mermaidPackagePath, 'utf8'),
+      await Bun.file(mermaidPackagePath).text(),
     ) as {
       name?: unknown
       exports?: { '.'?: { import?: unknown } }

tests/mocks/auth.ts

@@ -0,0 +1,19 @@
+/**
+ * Shared mock for `src/utils/auth.js`. Use it via:
+ *
+ *   import { authMock } from '../../tests/mocks/auth'
+ *   mock.module('src/utils/auth.js', authMock)
+ *
+ * Tests that need different return values can override the helper used by
+ * the suite (e.g. by extending this object and re-registering with mock.module).
+ * Always extend here rather than inlining a different shape per test, so the
+ * surface stays consistent when `auth.ts` exports change.
+ */
+export const authMock = () => ({
+  checkAndRefreshOAuthTokenIfNeeded: async () => {},
+  getClaudeAIOAuthTokens: () => ({ accessToken: 'token' }),
+  isClaudeAISubscriber: () => true,
+  isProSubscriber: () => false,
+  isMaxSubscriber: () => false,
+  isTeamSubscriber: () => false,
+})