From d1e5bd63c14a2abc8df8107e4a119caa41dae331 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 22 Feb 2026 12:14:26 +0900 Subject: [PATCH] fix: address Oracle + Cubic review feedback for background-agent refactoring - Revert getMessageDir to original join(MESSAGE_STORAGE, sessionID) behavior - Fix dead subagentSessions.delete by capturing previousSessionID before tryFallbackRetry - Add .unref() to process cleanup setTimeout to prevent 6s hang on Ctrl-C - Add missing isUnstableAgent to fallback retry input mapping - Fix process-cleanup tests to use exit listener instead of SIGINT at index 0 - Swap test filenames in compaction-aware-message-resolver to exercise skip logic correctly --- .../compaction-aware-message-resolver.test.ts | 8 ++++---- .../background-agent/fallback-retry-handler.ts | 1 + src/features/background-agent/manager.ts | 10 ++++++---- .../background-agent/process-cleanup.test.ts | 12 +++++++++--- src/features/background-agent/process-cleanup.ts | 2 +- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/features/background-agent/compaction-aware-message-resolver.test.ts b/src/features/background-agent/compaction-aware-message-resolver.test.ts index b8a659b03..17cc07024 100644 --- a/src/features/background-agent/compaction-aware-message-resolver.test.ts +++ b/src/features/background-agent/compaction-aware-message-resolver.test.ts @@ -96,8 +96,8 @@ describe("findNearestMessageExcludingCompaction", () => { agent: "sisyphus", model: { providerID: "anthropic", modelID: "claude-opus-4-6" }, } - writeFileSync(join(tempDir, "001.json"), JSON.stringify(compactionMessage)) - writeFileSync(join(tempDir, "002.json"), JSON.stringify(validMessage)) + writeFileSync(join(tempDir, "002.json"), JSON.stringify(compactionMessage)) + writeFileSync(join(tempDir, "001.json"), JSON.stringify(validMessage)) // when const result = findNearestMessageExcludingCompaction(tempDir) @@ -155,8 +155,8 @@ describe("findNearestMessageExcludingCompaction", () => { agent: "oracle", model: { providerID: "google", modelID: "gemini-2-flash" }, } - writeFileSync(join(tempDir, "001.json"), invalidJson) - writeFileSync(join(tempDir, "002.json"), JSON.stringify(validMessage)) + writeFileSync(join(tempDir, "002.json"), invalidJson) + writeFileSync(join(tempDir, "001.json"), JSON.stringify(validMessage)) // when const result = findNearestMessageExcludingCompaction(tempDir) diff --git a/src/features/background-agent/fallback-retry-handler.ts b/src/features/background-agent/fallback-retry-handler.ts index f21b92288..94184fdbd 100644 --- a/src/features/background-agent/fallback-retry-handler.ts +++ b/src/features/background-agent/fallback-retry-handler.ts @@ -117,6 +117,7 @@ export function tryFallbackRetry(args: { model: task.model, fallbackChain: task.fallbackChain, category: task.category, + isUnstableAgent: task.isUnstableAgent, } queue.push({ task, input: retryInput }) queuesByKey.set(key, queue) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index a77e063af..c807277d2 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -9,7 +9,6 @@ import { TaskHistory } from "./task-history" import { log, getAgentToolRestrictions, - getMessageDir, normalizePromptTools, normalizeSDKResponse, promptWithModelSuggestionRetry, @@ -44,6 +43,8 @@ import { import { tryFallbackRetry } from "./fallback-retry-handler" import { registerManagerForCleanup, unregisterManagerForCleanup } from "./process-cleanup" import { isCompactionAgent, findNearestMessageExcludingCompaction } from "./compaction-aware-message-resolver" +import { MESSAGE_STORAGE } from "../hook-message-injector" +import { join } from "node:path" import { pruneStaleTasksAndNotifications } from "./task-poller" import { checkAndInterruptStaleTasks } from "./task-poller" @@ -931,6 +932,7 @@ export class BackgroundManager { errorInfo: { name?: string; message?: string }, source: string, ): boolean { + const previousSessionID = task.sessionID const result = tryFallbackRetry({ task, errorInfo, @@ -941,8 +943,8 @@ export class BackgroundManager { queuesByKey: this.queuesByKey, processKey: (key: string) => this.processKey(key), }) - if (result && task.sessionID) { - subagentSessions.delete(task.sessionID) + if (result && previousSessionID) { + subagentSessions.delete(previousSessionID) } return result } @@ -1345,7 +1347,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea parentSessionID: task.parentSessionID, }) } - const messageDir = getMessageDir(task.parentSessionID) + const messageDir = join(MESSAGE_STORAGE, task.parentSessionID) const currentMessage = messageDir ? findNearestMessageExcludingCompaction(messageDir) : null agent = currentMessage?.agent ?? task.parentAgent model = currentMessage?.model?.providerID && currentMessage?.model?.modelID diff --git a/src/features/background-agent/process-cleanup.test.ts b/src/features/background-agent/process-cleanup.test.ts index 9560852db..621a6cb1a 100644 --- a/src/features/background-agent/process-cleanup.test.ts +++ b/src/features/background-agent/process-cleanup.test.ts @@ -64,7 +64,9 @@ describe("process-cleanup", () => { registerManagerForCleanup(manager) - const [, listener] = processOnCalls[0] + const exitEntry = processOnCalls.find(([signal]) => signal === "exit") + expect(exitEntry).toBeDefined() + const [, listener] = exitEntry! listener() expect(mockShutdown).toHaveBeenCalled() @@ -83,7 +85,9 @@ describe("process-cleanup", () => { registerManagerForCleanup(manager2) registerManagerForCleanup(manager3) - const [, listener] = processOnCalls[0] + const exitEntry = processOnCalls.find(([signal]) => signal === "exit") + expect(exitEntry).toBeDefined() + const [, listener] = exitEntry! listener() expect(shutdown1).toHaveBeenCalledTimes(1) @@ -144,7 +148,9 @@ describe("process-cleanup", () => { registerManagerForCleanup(manager1) registerManagerForCleanup(manager2) - const [, listener] = processOnCalls[0] + const exitEntry = processOnCalls.find(([signal]) => signal === "exit") + expect(exitEntry).toBeDefined() + const [, listener] = exitEntry! unregisterManagerForCleanup(manager2) listener() diff --git a/src/features/background-agent/process-cleanup.ts b/src/features/background-agent/process-cleanup.ts index e5d432142..2b29fc9bf 100644 --- a/src/features/background-agent/process-cleanup.ts +++ b/src/features/background-agent/process-cleanup.ts @@ -11,7 +11,7 @@ function registerProcessSignal( handler() if (exitAfter) { process.exitCode = 0 - setTimeout(() => process.exit(), 6000) + setTimeout(() => process.exit(), 6000).unref() } } process.on(signal, listener)