Merge pull request #3841 from Momentum96/fix/background-manager-tmux-ordering
This commit is contained in:
@@ -322,6 +322,82 @@ function createToastRemoveTaskTracker(): { removeTaskCalls: string[]; resetToast
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
describe("BackgroundManager tmux callback ordering", () => {
|
||||||
|
test("starts promptAsync before a blocking tmux callback resolves", async () => {
|
||||||
|
//#given
|
||||||
|
const events: string[] = []
|
||||||
|
let resolveTmuxCallback: () => void = () => {}
|
||||||
|
const tmuxCallbackPromise = new Promise<void>((resolve) => {
|
||||||
|
resolveTmuxCallback = resolve
|
||||||
|
})
|
||||||
|
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
get: async () => {
|
||||||
|
events.push("session.get")
|
||||||
|
return { data: { directory: "/tmp/test" } }
|
||||||
|
},
|
||||||
|
create: async () => {
|
||||||
|
events.push("session.create")
|
||||||
|
return { data: { id: "ses_manager_blocking_tmux" } }
|
||||||
|
},
|
||||||
|
promptAsync: async () => {
|
||||||
|
events.push("promptAsync")
|
||||||
|
return { data: {} }
|
||||||
|
},
|
||||||
|
abort: async () => ({ data: {} }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const onSubagentSessionCreated = mock(async () => {
|
||||||
|
events.push("tmux.callback.start")
|
||||||
|
await tmuxCallbackPromise
|
||||||
|
events.push("tmux.callback.end")
|
||||||
|
})
|
||||||
|
const manager = new BackgroundManager({
|
||||||
|
pluginContext: createPluginInput(client, "/tmp/test"),
|
||||||
|
tmuxConfig: {
|
||||||
|
enabled: true,
|
||||||
|
layout: "main-vertical",
|
||||||
|
main_pane_size: 60,
|
||||||
|
main_pane_min_width: 120,
|
||||||
|
agent_pane_min_width: 40,
|
||||||
|
isolation: "inline",
|
||||||
|
},
|
||||||
|
onSubagentSessionCreated,
|
||||||
|
enableParentSessionNotifications: false,
|
||||||
|
})
|
||||||
|
const originalTmux = process.env.TMUX
|
||||||
|
process.env.TMUX = "/tmp/fake-tmux-socket"
|
||||||
|
|
||||||
|
try {
|
||||||
|
//#when
|
||||||
|
await manager.launch({
|
||||||
|
description: "Blocking tmux test",
|
||||||
|
prompt: "Do work",
|
||||||
|
agent: "general",
|
||||||
|
parentSessionId: "ses_parent",
|
||||||
|
parentMessageId: "msg_parent",
|
||||||
|
})
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 20))
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(events).toContain("session.create")
|
||||||
|
expect(events).toContain("promptAsync")
|
||||||
|
expect(events).toContain("tmux.callback.start")
|
||||||
|
const promptIdx = events.indexOf("promptAsync")
|
||||||
|
const tmuxStartIdx = events.indexOf("tmux.callback.start")
|
||||||
|
expect(promptIdx < tmuxStartIdx).toBe(true)
|
||||||
|
expect(events).not.toContain("tmux.callback.end")
|
||||||
|
} finally {
|
||||||
|
resolveTmuxCallback()
|
||||||
|
if (originalTmux === undefined) delete process.env.TMUX
|
||||||
|
else process.env.TMUX = originalTmux
|
||||||
|
manager.shutdown()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("BackgroundManager session.error fallback hydration", () => {
|
describe("BackgroundManager session.error fallback hydration", () => {
|
||||||
test("hydrates fallbackChain from session fallback state before retrying sync child-session errors", async () => {
|
test("hydrates fallbackChain from session fallback state before retrying sync child-session errors", async () => {
|
||||||
//#given
|
//#given
|
||||||
@@ -3463,7 +3539,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
expect(getConcurrencyManager(manager).getCount("test-agent")).toBe(0)
|
expect(getConcurrencyManager(manager).getCount("test-agent")).toBe(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
test("should keep task cancelled when cancelled during tmux callback before running state is assigned", async () => {
|
test("should start prompt before tmux callback cancellation", async () => {
|
||||||
// given
|
// given
|
||||||
resetClaudeCodeSessionState()
|
resetClaudeCodeSessionState()
|
||||||
const originalTmuxEnvironment = process.env.TMUX
|
const originalTmuxEnvironment = process.env.TMUX
|
||||||
@@ -3474,9 +3550,9 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
const abortCalls: string[] = []
|
const abortCalls: string[] = []
|
||||||
const promptAsyncSessionIDs: string[] = []
|
const promptAsyncSessionIDs: string[] = []
|
||||||
let taskID: string | undefined
|
let taskID: string | undefined
|
||||||
let resolveAbortCalled: (() => void) | undefined
|
let resolveCancelCalled: (() => void) | undefined
|
||||||
const abortCalled = new Promise<void>((resolve) => {
|
const cancelCalled = new Promise<void>((resolve) => {
|
||||||
resolveAbortCalled = resolve
|
resolveCancelCalled = resolve
|
||||||
})
|
})
|
||||||
|
|
||||||
manager.shutdown()
|
manager.shutdown()
|
||||||
@@ -3496,7 +3572,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
status: async () => ({ data: {} }),
|
status: async () => ({ data: {} }),
|
||||||
abort: async ({ path }: { path: { id: string } }) => {
|
abort: async ({ path }: { path: { id: string } }) => {
|
||||||
abortCalls.push(path.id)
|
abortCalls.push(path.id)
|
||||||
resolveAbortCalled?.()
|
|
||||||
return {}
|
return {}
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -3523,6 +3598,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
source: "test",
|
source: "test",
|
||||||
abortSession: false,
|
abortSession: false,
|
||||||
})
|
})
|
||||||
|
resolveCancelCalled?.()
|
||||||
}, }
|
}, }
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -3539,7 +3615,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
|
|
||||||
// when
|
// when
|
||||||
await Promise.race([
|
await Promise.race([
|
||||||
abortCalled,
|
cancelCalled,
|
||||||
new Promise<never>((_, reject) => setTimeout(() => reject(new Error("timeout")), 500)),
|
new Promise<never>((_, reject) => setTimeout(() => reject(new Error("timeout")), 500)),
|
||||||
])
|
])
|
||||||
await flushBackgroundNotifications()
|
await flushBackgroundNotifications()
|
||||||
@@ -3547,12 +3623,12 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
|
|||||||
// then
|
// then
|
||||||
const updatedTask = manager.getTask(task.id)
|
const updatedTask = manager.getTask(task.id)
|
||||||
expect(updatedTask?.status).toBe("cancelled")
|
expect(updatedTask?.status).toBe("cancelled")
|
||||||
expect(updatedTask?.sessionId).toBeUndefined()
|
expect(updatedTask?.sessionId).toBe(createdSessionID)
|
||||||
expect(promptAsyncSessionIDs).not.toContain(createdSessionID)
|
expect(promptAsyncSessionIDs).toContain(createdSessionID)
|
||||||
expect(abortCalls).toEqual([createdSessionID])
|
expect(abortCalls).toEqual([])
|
||||||
expect(getConcurrencyManager(manager).getCount("test-agent")).toBe(0)
|
expect(getConcurrencyManager(manager).getCount("test-agent")).toBe(0)
|
||||||
expect(getRootDescendantCounts(manager).has("parent-session")).toBe(false)
|
expect(getRootDescendantCounts(manager).has("parent-session")).toBe(false)
|
||||||
expect(subagentSessions.has(createdSessionID)).toBe(false)
|
expect(subagentSessions.has(createdSessionID)).toBe(true)
|
||||||
} finally {
|
} finally {
|
||||||
resetClaudeCodeSessionState()
|
resetClaudeCodeSessionState()
|
||||||
if (originalTmuxEnvironment === undefined) {
|
if (originalTmuxEnvironment === undefined) {
|
||||||
|
|||||||
@@ -761,33 +761,8 @@ export class BackgroundManager {
|
|||||||
this.settlePreStartDescendantReservation(task)
|
this.settlePreStartDescendantReservation(task)
|
||||||
subagentSessions.add(sessionID)
|
subagentSessions.add(sessionID)
|
||||||
|
|
||||||
log("[background-agent] tmux callback check", {
|
|
||||||
hasCallback: !!this.onSubagentSessionCreated,
|
|
||||||
tmuxEnabled: this.tmuxEnabled,
|
|
||||||
isInsideTmux: isInsideTmux(),
|
|
||||||
sessionID,
|
|
||||||
parentID: input.parentSessionId,
|
|
||||||
})
|
|
||||||
|
|
||||||
if (!input.suppressTmuxSpawn && this.onSubagentSessionCreated && this.tmuxEnabled && isInsideTmux()) {
|
|
||||||
log("[background-agent] Invoking tmux callback NOW", { sessionID })
|
|
||||||
await this.onSubagentSessionCreated({
|
|
||||||
sessionID,
|
|
||||||
parentID: input.parentSessionId,
|
|
||||||
title: input.description,
|
|
||||||
}).catch((err) => {
|
|
||||||
log("[background-agent] Failed to spawn tmux pane:", err)
|
|
||||||
})
|
|
||||||
log("[background-agent] tmux callback completed, waiting 200ms")
|
|
||||||
await new Promise(r => setTimeout(r, 200))
|
|
||||||
} else {
|
|
||||||
log("[background-agent] SKIP tmux callback - conditions not met", {
|
|
||||||
suppressTmuxSpawn: !!input.suppressTmuxSpawn,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
if (this.tasks.get(task.id)?.status === "cancelled") {
|
if (this.tasks.get(task.id)?.status === "cancelled") {
|
||||||
await this.abortSessionWithLogging(sessionID, "cancelled during tmux setup")
|
await this.abortSessionWithLogging(sessionID, "cancelled during launch setup")
|
||||||
subagentSessions.delete(sessionID)
|
subagentSessions.delete(sessionID)
|
||||||
if (task.rootSessionId) {
|
if (task.rootSessionId) {
|
||||||
this.unregisterRootDescendant(task.rootSessionId)
|
this.unregisterRootDescendant(task.rootSessionId)
|
||||||
@@ -982,6 +957,29 @@ The fallback retry session is now created and can be inspected directly.
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
log("[background-agent] tmux callback check", {
|
||||||
|
hasCallback: !!this.onSubagentSessionCreated,
|
||||||
|
tmuxEnabled: this.tmuxEnabled,
|
||||||
|
isInsideTmux: isInsideTmux(),
|
||||||
|
sessionID,
|
||||||
|
parentID: input.parentSessionId,
|
||||||
|
})
|
||||||
|
|
||||||
|
if (!input.suppressTmuxSpawn && this.onSubagentSessionCreated && this.tmuxEnabled && isInsideTmux()) {
|
||||||
|
log("[background-agent] Invoking tmux callback (fire-and-forget)", { sessionID })
|
||||||
|
void this.onSubagentSessionCreated({
|
||||||
|
sessionID,
|
||||||
|
parentID: input.parentSessionId,
|
||||||
|
title: input.description,
|
||||||
|
}).catch((err) => {
|
||||||
|
log("[background-agent] Failed to spawn tmux pane:", err)
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
log("[background-agent] SKIP tmux callback - conditions not met", {
|
||||||
|
suppressTmuxSpawn: !!input.suppressTmuxSpawn,
|
||||||
|
})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
getTask(id: string): BackgroundTask | undefined {
|
getTask(id: string): BackgroundTask | undefined {
|
||||||
|
|||||||
@@ -0,0 +1,18 @@
|
|||||||
|
/// <reference types="bun-types" />
|
||||||
|
|
||||||
|
import { describe, expect, test } from "bun:test"
|
||||||
|
|
||||||
|
import { isAttachableSessionStatus } from "./attachable-session-status"
|
||||||
|
|
||||||
|
describe("isAttachableSessionStatus", () => {
|
||||||
|
test("#given a busy session #when checking attachability #then it is attachable", () => {
|
||||||
|
//#given
|
||||||
|
const status = "busy"
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const attachable = isAttachableSessionStatus(status)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(attachable).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
const ATTACHABLE_SESSION_STATUSES = ["idle", "running"] as const
|
const ATTACHABLE_SESSION_STATUSES = ["idle", "running", "busy"] as const
|
||||||
|
|
||||||
export type AttachableSessionStatus = (typeof ATTACHABLE_SESSION_STATUSES)[number]
|
export type AttachableSessionStatus = (typeof ATTACHABLE_SESSION_STATUSES)[number]
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,7 @@
|
|||||||
* failing because there is no real tmux server running.
|
* failing because there is no real tmux server running.
|
||||||
*/
|
*/
|
||||||
export function isCmuxCompatEnvironment(): boolean {
|
export function isCmuxCompatEnvironment(): boolean {
|
||||||
return Boolean(process.env.CMUX_SOCKET_PATH) ||
|
const tmuxEnvironment = process.env.TMUX
|
||||||
process.env.TMUX?.includes("cmuxterm") === true
|
return tmuxEnvironment?.includes("cmuxterm") === true ||
|
||||||
|
(Boolean(process.env.CMUX_SOCKET_PATH) && !tmuxEnvironment)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -63,6 +63,33 @@ afterAll(async () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("runTmuxCommand", () => {
|
describe("runTmuxCommand", () => {
|
||||||
|
test("#given cmux socket and real tmux session #when run #then uses requested executable instead of cmux compat", async () => {
|
||||||
|
// given
|
||||||
|
const originalCmuxSocketPath = process.env.CMUX_SOCKET_PATH
|
||||||
|
const originalTmux = process.env.TMUX
|
||||||
|
process.env.CMUX_SOCKET_PATH = "/tmp/cmux.sock"
|
||||||
|
process.env.TMUX = "/private/tmp/tmux-501/default,123,0"
|
||||||
|
|
||||||
|
try {
|
||||||
|
// when
|
||||||
|
const result = await runTmuxCommand("sh", ["-c", "printf '%s\\n' real-tmux"])
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result).toEqual({
|
||||||
|
success: true,
|
||||||
|
output: "real-tmux",
|
||||||
|
stdout: "real-tmux",
|
||||||
|
stderr: "",
|
||||||
|
exitCode: 0,
|
||||||
|
})
|
||||||
|
} finally {
|
||||||
|
if (originalCmuxSocketPath === undefined) delete process.env.CMUX_SOCKET_PATH
|
||||||
|
else process.env.CMUX_SOCKET_PATH = originalCmuxSocketPath
|
||||||
|
if (originalTmux === undefined) delete process.env.TMUX
|
||||||
|
else process.env.TMUX = originalTmux
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
test("#given command exits 0 with stdout #when run #then success true, output and stdout equal trimmed value, stderr empty", async () => {
|
test("#given command exits 0 with stdout #when run #then success true, output and stdout equal trimmed value, stderr empty", async () => {
|
||||||
// given
|
// given
|
||||||
const commandArguments = ["-c", "printf '%s\\n' '%42'"]
|
const commandArguments = ["-c", "printf '%s\\n' '%42'"]
|
||||||
|
|||||||
Reference in New Issue
Block a user