fix: address review-work round 3 findings (async shutdown, signal generation, stale test name)

This commit is contained in:
YeonGyu-Kim
2026-03-11 21:30:04 +09:00
parent ff536e992a
commit 2c3c447dc4
6 changed files with 14 additions and 13 deletions
+2 -2
View File
@@ -53,8 +53,8 @@ export function createManagers(args: {
log("[index] onSubagentSessionCreated callback completed") log("[index] onSubagentSessionCreated callback completed")
}, },
onShutdown: () => { onShutdown: async () => {
tmuxSessionManager.cleanup().catch((error) => { await tmuxSessionManager.cleanup().catch((error) => {
log("[index] tmux cleanup error during shutdown:", error) log("[index] tmux cleanup error during shutdown:", error)
}) })
}, },
+4 -4
View File
@@ -116,7 +116,7 @@ export class BackgroundManager {
private config?: BackgroundTaskConfig private config?: BackgroundTaskConfig
private tmuxEnabled: boolean private tmuxEnabled: boolean
private onSubagentSessionCreated?: OnSubagentSessionCreated private onSubagentSessionCreated?: OnSubagentSessionCreated
private onShutdown?: () => void private onShutdown?: () => void | Promise<void>
private queuesByKey: Map<string, QueueItem[]> = new Map() private queuesByKey: Map<string, QueueItem[]> = new Map()
private processingKeys: Set<string> = new Set() private processingKeys: Set<string> = new Set()
@@ -134,7 +134,7 @@ export class BackgroundManager {
options?: { options?: {
tmuxConfig?: TmuxConfig tmuxConfig?: TmuxConfig
onSubagentSessionCreated?: OnSubagentSessionCreated onSubagentSessionCreated?: OnSubagentSessionCreated
onShutdown?: () => void onShutdown?: () => void | Promise<void>
enableParentSessionNotifications?: boolean enableParentSessionNotifications?: boolean
} }
) { ) {
@@ -1702,7 +1702,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
* Cancels all pending concurrency waiters and clears timers. * Cancels all pending concurrency waiters and clears timers.
* Should be called when the plugin is unloaded. * Should be called when the plugin is unloaded.
*/ */
shutdown(): void { async shutdown(): Promise<void> {
if (this.shutdownTriggered) return if (this.shutdownTriggered) return
this.shutdownTriggered = true this.shutdownTriggered = true
log("[background-agent] Shutting down BackgroundManager") log("[background-agent] Shutting down BackgroundManager")
@@ -1720,7 +1720,7 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
// Notify shutdown listeners (e.g., tmux cleanup) // Notify shutdown listeners (e.g., tmux cleanup)
if (this.onShutdown) { if (this.onShutdown) {
try { try {
this.onShutdown() await this.onShutdown()
} catch (error) { } catch (error) {
log("[background-agent] Error in onShutdown callback:", error) log("[background-agent] Error in onShutdown callback:", error)
} }
@@ -19,6 +19,7 @@ export function registerProcessCleanup(state: SkillMcpManagerState): void {
state.cleanupRegistered = true state.cleanupRegistered = true
const cleanup = async (): Promise<void> => { const cleanup = async (): Promise<void> => {
state.shutdownGeneration++
for (const managed of state.clients.values()) { for (const managed of state.clients.values()) {
await closeManagedClient(managed) await closeManagedClient(managed)
} }
@@ -142,7 +142,7 @@ describe("getOrCreateClient disconnect race", () => {
expect(createdTransports[0]?.close).toHaveBeenCalledTimes(1) expect(createdTransports[0]?.close).toHaveBeenCalledTimes(1)
}) })
it("#given session A in disconnectedSessions #when new connection is requested for session A #then session A is removed from disconnectedSessions and connection proceeds normally", async () => { it("#given session A in disconnectedSessions #when new connection is requested for session A #then connection proceeds normally and disconnectedSessions entry is retained for pending race protection", async () => {
const state = createState() const state = createState()
const info = createClientInfo("session-a") const info = createClientInfo("session-a")
const clientKey = createClientKey(info) const clientKey = createClientKey(info)
+4 -4
View File
@@ -7,7 +7,7 @@ describe("createPluginDispose", () => {
test("#given plugin with active managers and hooks #when dispose() is called #then backgroundManager.shutdown() is called", async () => { test("#given plugin with active managers and hooks #when dispose() is called #then backgroundManager.shutdown() is called", async () => {
// given // given
const backgroundManager = { const backgroundManager = {
shutdown: (): void => {}, shutdown: async (): Promise<void> => {},
} }
const skillMcpManager = { const skillMcpManager = {
disconnectAll: async (): Promise<void> => {}, disconnectAll: async (): Promise<void> => {},
@@ -29,7 +29,7 @@ describe("createPluginDispose", () => {
test("#given plugin with active MCP connections #when dispose() is called #then skillMcpManager.disconnectAll() is called", async () => { test("#given plugin with active MCP connections #when dispose() is called #then skillMcpManager.disconnectAll() is called", async () => {
// given // given
const backgroundManager = { const backgroundManager = {
shutdown: (): void => {}, shutdown: async (): Promise<void> => {},
} }
const skillMcpManager = { const skillMcpManager = {
disconnectAll: async (): Promise<void> => {}, disconnectAll: async (): Promise<void> => {},
@@ -64,7 +64,7 @@ describe("createPluginDispose", () => {
const autoSlashCommandDisposeSpy = spyOn(autoSlashCommand, "dispose") const autoSlashCommandDisposeSpy = spyOn(autoSlashCommand, "dispose")
const dispose = createPluginDispose({ const dispose = createPluginDispose({
backgroundManager: { backgroundManager: {
shutdown: (): void => {}, shutdown: async (): Promise<void> => {},
}, },
skillMcpManager: { skillMcpManager: {
disconnectAll: async (): Promise<void> => {}, disconnectAll: async (): Promise<void> => {},
@@ -90,7 +90,7 @@ describe("createPluginDispose", () => {
test("#given dispose already called #when dispose() called again #then no errors", async () => { test("#given dispose already called #when dispose() called again #then no errors", async () => {
// given // given
const backgroundManager = { const backgroundManager = {
shutdown: (): void => {}, shutdown: async (): Promise<void> => {},
} }
const skillMcpManager = { const skillMcpManager = {
disconnectAll: async (): Promise<void> => {}, disconnectAll: async (): Promise<void> => {},
+2 -2
View File
@@ -2,7 +2,7 @@ export type PluginDispose = () => Promise<void>
export function createPluginDispose(args: { export function createPluginDispose(args: {
backgroundManager: { backgroundManager: {
shutdown: () => void shutdown: () => void | Promise<void>
} }
skillMcpManager: { skillMcpManager: {
disconnectAll: () => Promise<void> disconnectAll: () => Promise<void>
@@ -19,7 +19,7 @@ export function createPluginDispose(args: {
} }
disposePromise = (async (): Promise<void> => { disposePromise = (async (): Promise<void> => {
backgroundManager.shutdown() await backgroundManager.shutdown()
await skillMcpManager.disconnectAll() await skillMcpManager.disconnectAll()
disposeHooks() disposeHooks()
})() })()