Merge pull request #3078 from code-yeongyu/refactor/deslop-features-managers

refactor(features): fix empty catches in manager modules

src/features/background-agent/manager.ts

@@ -191,6 +191,19 @@ export class BackgroundManager {
     this.registerProcessCleanup()
   }
 
+  private async abortSessionWithLogging(sessionID: string, reason: string): Promise<void> {
+    try {
+      await this.client.session.abort({
+        path: { id: sessionID },
+      })
+    } catch (error) {
+      log(`[background-agent] Failed to abort session during ${reason}:`, {
+        sessionID,
+        error,
+      })
+    }
+  }
+
   async assertCanSpawn(parentSessionID: string): Promise<SubagentSpawnContext> {
     const spawnContext = await resolveSubagentSpawnContext(this.client, parentSessionID)
     const maxDepth = getMaxSubagentDepth(this.config)
@@ -448,11 +461,7 @@ export class BackgroundManager {
     const sessionID = createResult.data.id
 
     if (task.status === "cancelled") {
-      await this.client.session.abort({
+      await this.abortSessionWithLogging(sessionID, "cancelled pre-start cleanup")
-        path: { id: sessionID },
-      }).catch((error) => {
-        log("[background-agent] Failed to abort cancelled pre-start session:", error)
-      })
       this.concurrencyManager.release(concurrencyKey)
       return
     }
@@ -570,9 +579,7 @@ export class BackgroundManager {
 
         // Abort the session to prevent infinite polling hang
         // Awaited to prevent dangling promise during subagent teardown (Bun/WebKit SIGABRT)
-        await this.client.session.abort({
+        await this.abortSessionWithLogging(sessionID, "launch error cleanup")
-          path: { id: sessionID },
-        }).catch(() => {})
 
         this.markForNotification(existingTask)
         this.enqueueNotificationForParent(existingTask.parentSessionID, () => this.notifyParentSession(existingTask)).catch(err => {
@@ -853,9 +860,7 @@ export class BackgroundManager {
       // Abort the session to prevent infinite polling hang
       // Awaited to prevent dangling promise during subagent teardown (Bun/WebKit SIGABRT)
       if (existingTask.sessionID) {
-        await this.client.session.abort({
+        await this.abortSessionWithLogging(existingTask.sessionID, "resume error cleanup")
-          path: { id: existingTask.sessionID },
-        }).catch(() => {})
       }
 
       this.markForNotification(existingTask)
@@ -879,7 +884,11 @@ export class BackgroundManager {
         (t) => t.status !== "completed" && t.status !== "cancelled"
       )
       return incomplete.length > 0
-    } catch {
+    } catch (error) {
+      log("[background-agent] Failed to check session todos:", {
+        sessionID,
+        error,
+      })
       return false
     }
   }
@@ -1269,7 +1278,6 @@ export class BackgroundManager {
         return false
       }
 
-      // Additionally check that at least one message has content (not just empty)
       // OpenCode API uses different part types than Anthropic's API:
       // - "reasoning" with .text property (thinking/reasoning content)
       // - "tool" with .state.output property (tool call results)
@@ -1438,9 +1446,7 @@ export class BackgroundManager {
 
     if (abortSession && task.sessionID) {
       // Awaited to prevent dangling promise during subagent teardown (Bun/WebKit SIGABRT)
-      await this.client.session.abort({
+      await this.abortSessionWithLogging(task.sessionID, `task cancellation (${source})`)
-        path: { id: task.sessionID },
-      }).catch(() => {})
 
       SessionCategoryRegistry.remove(task.sessionID)
     }
@@ -1557,9 +1563,7 @@ export class BackgroundManager {
 
     if (task.sessionID) {
       // Awaited to prevent dangling promise during subagent teardown (Bun/WebKit SIGABRT)
-      await this.client.session.abort({
+      await this.abortSessionWithLogging(task.sessionID, `task completion (${source})`)
-        path: { id: task.sessionID },
-      }).catch(() => {})
 
       SessionCategoryRegistry.remove(task.sessionID)
     }
@@ -1576,9 +1580,6 @@ export class BackgroundManager {
   }
 
   private async notifyParentSession(task: BackgroundTask): Promise<void> {
-    // Note: Callers must release concurrency before calling this method
-    // to ensure slots are freed even if notification fails
-
     const duration = formatDuration(task.startedAt ?? new Date(), task.completedAt)
 
     log("[background-agent] notifyParentSession called for task:", task.id)
@@ -1903,16 +1904,11 @@ export class BackgroundManager {
           continue
         }
 
-        // Explicit terminal non-idle status (e.g., "interrupted") — complete immediately,
-        // skipping output validation (session will never produce more output).
-        // Unknown statuses fall through to the idle/gone path with output validation.
         if (sessionStatus && isTerminalSessionStatus(sessionStatus.type)) {
           await this.tryCompleteTask(task, `polling (terminal session status: ${sessionStatus.type})`)
           continue
         }
 
-        // Unknown non-idle status — not active, not terminal, not idle.
-        // Fall through to idle/gone completion path with output validation.
         if (sessionStatus && sessionStatus.type !== "idle") {
           log("[background-agent] Unknown session status, treating as potentially idle:", {
             taskId: task.id,
@@ -2065,17 +2061,24 @@ export class BackgroundManager {
     }
 
     const previous = this.notificationQueueByParent.get(parentSessionID) ?? Promise.resolve()
+    const cleanupQueueEntry = (): void => {
+      if (this.notificationQueueByParent.get(parentSessionID) === current) {
+        this.notificationQueueByParent.delete(parentSessionID)
+      }
+    }
+
     const current = previous
-      .catch(() => {})
+      .catch((error) => {
+        log("[background-agent] Continuing notification queue after previous failure:", {
+          parentSessionID,
+          error,
+        })
+      })
       .then(operation)
 
     this.notificationQueueByParent.set(parentSessionID, current)
 
-    void current.finally(() => {
+    void current.then(cleanupQueueEntry, cleanupQueueEntry)
-      if (this.notificationQueueByParent.get(parentSessionID) === current) {
-        this.notificationQueueByParent.delete(parentSessionID)
-      }
-    }).catch(() => {})
 
     return current
   }

src/features/tmux-subagent/manager.ts

@@ -43,18 +43,6 @@ const DEFERRED_SESSION_TTL_MS = 5 * 60 * 1000
 const MAX_DEFERRED_QUEUE_SIZE = 20
 const MAX_CLOSE_RETRY_COUNT = 3
 
-/**
- * State-first Tmux Session Manager
- * 
- * Architecture:
- * 1. QUERY: Get actual tmux pane state (source of truth)
- * 2. DECIDE: Pure function determines actions based on state
- * 3. EXECUTE: Execute actions with verification
- * 4. UPDATE: Update internal cache only after tmux confirms success
- * 
- * The internal `sessions` Map is just a cache for sessionId<->paneId mapping.
- * The REAL source of truth is always queried from tmux.
- */
 export class TmuxSessionManager {
   private client: OpencodeClient
   private tmuxConfig: TmuxConfig
@@ -78,16 +66,20 @@ export class TmuxSessionManager {
     this.deps = deps
     const defaultPort = process.env.OPENCODE_PORT ?? "4096"
     const fallbackUrl = `http://localhost:${defaultPort}`
+    const rawServerUrl = ctx.serverUrl?.toString()
     try {
-      const raw = ctx.serverUrl?.toString()
+      if (rawServerUrl) {
-      if (raw) {
+        const parsed = new URL(rawServerUrl)
-        const parsed = new URL(raw)
         const port = parsed.port || (parsed.protocol === 'https:' ? '443' : '80')
-        this.serverUrl = port === '0' ? fallbackUrl : raw
+        this.serverUrl = port === '0' ? fallbackUrl : rawServerUrl
       } else {
         this.serverUrl = fallbackUrl
       }
-    } catch {
+    } catch (error) {
+      log("[tmux-session-manager] failed to parse server URL, using fallback", {
+        serverUrl: rawServerUrl,
+        error: String(error),
+      })
       this.serverUrl = fallbackUrl
     }
     this.sourcePaneId = deps.getCurrentPaneId()
@@ -124,7 +116,13 @@ export class TmuxSessionManager {
   ): Promise<string | null> {
     if (!this.isIsolated()) return null
     if (this.isolatedWindowPaneId) {
-      const state = await queryWindowState(this.isolatedWindowPaneId).catch(() => null)
+      const state = await queryWindowState(this.isolatedWindowPaneId).catch((error) => {
+        log("[tmux-session-manager] failed to query isolated window state", {
+          paneId: this.isolatedWindowPaneId,
+          error: String(error),
+        })
+        return null
+      })
       if (state) return null
       this.isolatedContainerPaneId = undefined
       this.isolatedWindowPaneId = undefined
@@ -736,7 +734,11 @@ export class TmuxSessionManager {
 
   private async enqueueSpawn(run: () => Promise<void>): Promise<void> {
     this.spawnQueue = this.spawnQueue
-      .catch(() => undefined)
+      .catch((error) => {
+        log("[tmux-session-manager] recovering spawn queue after previous failure", {
+          error: String(error),
+        })
+      })
       .then(run)
       .catch((err) => {
         log("[tmux-session-manager] spawn queue task failed", {