Merge pull request #2035 from code-yeongyu/fix/background-agent-review-feedback
fix: address Oracle + Cubic review feedback for background-agent refactoring
This commit is contained in:
commit
a8f0300ba6
@ -96,8 +96,8 @@ describe("findNearestMessageExcludingCompaction", () => {
|
|||||||
agent: "sisyphus",
|
agent: "sisyphus",
|
||||||
model: { providerID: "anthropic", modelID: "claude-opus-4-6" },
|
model: { providerID: "anthropic", modelID: "claude-opus-4-6" },
|
||||||
}
|
}
|
||||||
writeFileSync(join(tempDir, "001.json"), JSON.stringify(compactionMessage))
|
writeFileSync(join(tempDir, "002.json"), JSON.stringify(compactionMessage))
|
||||||
writeFileSync(join(tempDir, "002.json"), JSON.stringify(validMessage))
|
writeFileSync(join(tempDir, "001.json"), JSON.stringify(validMessage))
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const result = findNearestMessageExcludingCompaction(tempDir)
|
const result = findNearestMessageExcludingCompaction(tempDir)
|
||||||
@ -155,8 +155,8 @@ describe("findNearestMessageExcludingCompaction", () => {
|
|||||||
agent: "oracle",
|
agent: "oracle",
|
||||||
model: { providerID: "google", modelID: "gemini-2-flash" },
|
model: { providerID: "google", modelID: "gemini-2-flash" },
|
||||||
}
|
}
|
||||||
writeFileSync(join(tempDir, "001.json"), invalidJson)
|
writeFileSync(join(tempDir, "002.json"), invalidJson)
|
||||||
writeFileSync(join(tempDir, "002.json"), JSON.stringify(validMessage))
|
writeFileSync(join(tempDir, "001.json"), JSON.stringify(validMessage))
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const result = findNearestMessageExcludingCompaction(tempDir)
|
const result = findNearestMessageExcludingCompaction(tempDir)
|
||||||
|
|||||||
@ -117,6 +117,7 @@ export function tryFallbackRetry(args: {
|
|||||||
model: task.model,
|
model: task.model,
|
||||||
fallbackChain: task.fallbackChain,
|
fallbackChain: task.fallbackChain,
|
||||||
category: task.category,
|
category: task.category,
|
||||||
|
isUnstableAgent: task.isUnstableAgent,
|
||||||
}
|
}
|
||||||
queue.push({ task, input: retryInput })
|
queue.push({ task, input: retryInput })
|
||||||
queuesByKey.set(key, queue)
|
queuesByKey.set(key, queue)
|
||||||
|
|||||||
@ -9,7 +9,6 @@ import { TaskHistory } from "./task-history"
|
|||||||
import {
|
import {
|
||||||
log,
|
log,
|
||||||
getAgentToolRestrictions,
|
getAgentToolRestrictions,
|
||||||
getMessageDir,
|
|
||||||
normalizePromptTools,
|
normalizePromptTools,
|
||||||
normalizeSDKResponse,
|
normalizeSDKResponse,
|
||||||
promptWithModelSuggestionRetry,
|
promptWithModelSuggestionRetry,
|
||||||
@ -44,6 +43,8 @@ import {
|
|||||||
import { tryFallbackRetry } from "./fallback-retry-handler"
|
import { tryFallbackRetry } from "./fallback-retry-handler"
|
||||||
import { registerManagerForCleanup, unregisterManagerForCleanup } from "./process-cleanup"
|
import { registerManagerForCleanup, unregisterManagerForCleanup } from "./process-cleanup"
|
||||||
import { isCompactionAgent, findNearestMessageExcludingCompaction } from "./compaction-aware-message-resolver"
|
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 { pruneStaleTasksAndNotifications } from "./task-poller"
|
||||||
import { checkAndInterruptStaleTasks } from "./task-poller"
|
import { checkAndInterruptStaleTasks } from "./task-poller"
|
||||||
|
|
||||||
@ -931,6 +932,7 @@ export class BackgroundManager {
|
|||||||
errorInfo: { name?: string; message?: string },
|
errorInfo: { name?: string; message?: string },
|
||||||
source: string,
|
source: string,
|
||||||
): boolean {
|
): boolean {
|
||||||
|
const previousSessionID = task.sessionID
|
||||||
const result = tryFallbackRetry({
|
const result = tryFallbackRetry({
|
||||||
task,
|
task,
|
||||||
errorInfo,
|
errorInfo,
|
||||||
@ -941,8 +943,8 @@ export class BackgroundManager {
|
|||||||
queuesByKey: this.queuesByKey,
|
queuesByKey: this.queuesByKey,
|
||||||
processKey: (key: string) => this.processKey(key),
|
processKey: (key: string) => this.processKey(key),
|
||||||
})
|
})
|
||||||
if (result && task.sessionID) {
|
if (result && previousSessionID) {
|
||||||
subagentSessions.delete(task.sessionID)
|
subagentSessions.delete(previousSessionID)
|
||||||
}
|
}
|
||||||
return result
|
return result
|
||||||
}
|
}
|
||||||
@ -1345,7 +1347,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
|
|||||||
parentSessionID: task.parentSessionID,
|
parentSessionID: task.parentSessionID,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
const messageDir = getMessageDir(task.parentSessionID)
|
const messageDir = join(MESSAGE_STORAGE, task.parentSessionID)
|
||||||
const currentMessage = messageDir ? findNearestMessageExcludingCompaction(messageDir) : null
|
const currentMessage = messageDir ? findNearestMessageExcludingCompaction(messageDir) : null
|
||||||
agent = currentMessage?.agent ?? task.parentAgent
|
agent = currentMessage?.agent ?? task.parentAgent
|
||||||
model = currentMessage?.model?.providerID && currentMessage?.model?.modelID
|
model = currentMessage?.model?.providerID && currentMessage?.model?.modelID
|
||||||
|
|||||||
@ -64,7 +64,9 @@ describe("process-cleanup", () => {
|
|||||||
|
|
||||||
registerManagerForCleanup(manager)
|
registerManagerForCleanup(manager)
|
||||||
|
|
||||||
const [, listener] = processOnCalls[0]
|
const exitEntry = processOnCalls.find(([signal]) => signal === "exit")
|
||||||
|
expect(exitEntry).toBeDefined()
|
||||||
|
const [, listener] = exitEntry!
|
||||||
listener()
|
listener()
|
||||||
|
|
||||||
expect(mockShutdown).toHaveBeenCalled()
|
expect(mockShutdown).toHaveBeenCalled()
|
||||||
@ -83,7 +85,9 @@ describe("process-cleanup", () => {
|
|||||||
registerManagerForCleanup(manager2)
|
registerManagerForCleanup(manager2)
|
||||||
registerManagerForCleanup(manager3)
|
registerManagerForCleanup(manager3)
|
||||||
|
|
||||||
const [, listener] = processOnCalls[0]
|
const exitEntry = processOnCalls.find(([signal]) => signal === "exit")
|
||||||
|
expect(exitEntry).toBeDefined()
|
||||||
|
const [, listener] = exitEntry!
|
||||||
listener()
|
listener()
|
||||||
|
|
||||||
expect(shutdown1).toHaveBeenCalledTimes(1)
|
expect(shutdown1).toHaveBeenCalledTimes(1)
|
||||||
@ -144,7 +148,9 @@ describe("process-cleanup", () => {
|
|||||||
registerManagerForCleanup(manager1)
|
registerManagerForCleanup(manager1)
|
||||||
registerManagerForCleanup(manager2)
|
registerManagerForCleanup(manager2)
|
||||||
|
|
||||||
const [, listener] = processOnCalls[0]
|
const exitEntry = processOnCalls.find(([signal]) => signal === "exit")
|
||||||
|
expect(exitEntry).toBeDefined()
|
||||||
|
const [, listener] = exitEntry!
|
||||||
unregisterManagerForCleanup(manager2)
|
unregisterManagerForCleanup(manager2)
|
||||||
|
|
||||||
listener()
|
listener()
|
||||||
|
|||||||
@ -11,7 +11,7 @@ function registerProcessSignal(
|
|||||||
handler()
|
handler()
|
||||||
if (exitAfter) {
|
if (exitAfter) {
|
||||||
process.exitCode = 0
|
process.exitCode = 0
|
||||||
setTimeout(() => process.exit(), 6000)
|
setTimeout(() => process.exit(), 6000).unref()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
process.on(signal, listener)
|
process.on(signal, listener)
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user