Ver código fonte

feat(background-job-board): complete strangulation with remaining query methods

Add 5 more query methods to fully eliminate direct field reads:
- getParentSessionID(taskID)
- getTerminalState(taskID)
- isTimedOut(taskID)
- isStatusUncertain(taskID)
- getState(taskID)

Migrate all remaining direct field reads in 3 production consumers,
including logging and output formatting. No direct field reads remain
in production code outside of the board itself.

Refs #648
Michael Henke 1 mês atrás
pai
commit
feb30fba62

+ 38 - 33
src/hooks/task-session-manager/index.ts

@@ -235,9 +235,9 @@ export function createTaskSessionManagerHook(
     if (isLateCancelledTaskError(existing, status.state)) {
       log('[task-session-manager] suppressed late cancelled task error', {
         taskID: status.taskID,
-        alias: existing?.alias,
+        alias: backgroundJobBoard.getAlias(status.taskID),
         state: status.state,
-        terminalState: existing?.terminalState,
+        terminalState: backgroundJobBoard.getTerminalState(status.taskID),
         result: status.result,
       });
       return existing;
@@ -259,14 +259,16 @@ export function createTaskSessionManagerHook(
 
     log('[task-session-manager] background job status updated', {
       taskID: updated.taskID,
-      alias: updated.alias,
-      parentSessionID: updated.parentSessionID,
+      alias: backgroundJobBoard.getAlias(updated.taskID),
+      parentSessionID: backgroundJobBoard.getParentSessionID(updated.taskID),
       state: updated.state,
-      terminalUnreconciled: updated.terminalUnreconciled,
-      timedOut: updated.timedOut,
+      terminalUnreconciled: backgroundJobBoard.isTerminalUnreconciled(
+        updated.taskID,
+      ),
+      timedOut: backgroundJobBoard.isTimedOut(updated.taskID),
     });
 
-    if (updated.terminalUnreconciled) {
+    if (backgroundJobBoard.isTerminalUnreconciled(updated.taskID)) {
       pendingManagedTaskIds.delete(updated.taskID);
       backgroundJobBoard.addContext(
         updated.taskID,
@@ -315,9 +317,9 @@ export function createTaskSessionManagerHook(
       );
       log('[task-session-manager] normalized late cancelled injected failure', {
         taskID: status.taskID,
-        alias: existing?.alias,
+        alias: backgroundJobBoard.getAlias(status.taskID),
         state: status.state,
-        terminalState: existing?.terminalState,
+        terminalState: backgroundJobBoard.getTerminalState(status.taskID),
         result: status.result,
       });
       rememberProcessedInjectedCompletion(occurrenceId);
@@ -339,8 +341,8 @@ export function createTaskSessionManagerHook(
 
     log('[task-session-manager] processed injected background completion', {
       taskID: updated.taskID,
-      alias: updated.alias,
-      parentSessionID: updated.parentSessionID,
+      alias: backgroundJobBoard.getAlias(updated.taskID),
+      parentSessionID: backgroundJobBoard.getParentSessionID(updated.taskID),
       state: updated.state,
       occurrenceId,
     });
@@ -761,9 +763,9 @@ export function createTaskSessionManagerHook(
         const before = sessionId
           ? backgroundJobBoard.get(sessionId)
           : undefined;
-        const updated = sessionId
-          ? backgroundJobBoard.markRunningFromLiveSession(sessionId)
-          : undefined;
+        if (sessionId) {
+          backgroundJobBoard.markRunningFromLiveSession(sessionId);
+        }
         if (
           before &&
           sessionId &&
@@ -771,13 +773,12 @@ export function createTaskSessionManagerHook(
         ) {
           log('[task-session-manager] busy observed after cancel request', {
             sessionID: sessionId,
-            previousState: before.state,
-            previousTerminalState: before.terminalState,
+            previousState: backgroundJobBoard.getState(sessionId),
+            previousTerminalState:
+              backgroundJobBoard.getTerminalState(sessionId),
             terminalUnreconciled:
               backgroundJobBoard.isTerminalUnreconciled(sessionId),
-            resultSummary:
-              backgroundJobBoard.getResultSummary(sessionId) ??
-              before.resultSummary,
+            resultSummary: backgroundJobBoard.getResultSummary(sessionId),
           });
         }
         log('[task-session-manager] busy/status busy observed', {
@@ -785,22 +786,26 @@ export function createTaskSessionManagerHook(
           managesSession: sessionId
             ? options.shouldManageSession(sessionId)
             : false,
-          previousState: before?.state,
-          previousTerminalState: before?.terminalState,
+          previousState: sessionId
+            ? backgroundJobBoard.getState(sessionId)
+            : undefined,
+          previousTerminalState: sessionId
+            ? backgroundJobBoard.getTerminalState(sessionId)
+            : undefined,
           previousCancellationRequested: sessionId
             ? backgroundJobBoard.wasCancellationRequested(sessionId)
             : false,
           previousLastLiveBusyAt: sessionId
-            ? (backgroundJobBoard.getLastLiveBusyAt(sessionId) ??
-              before?.lastLiveBusyAt)
+            ? backgroundJobBoard.getLastLiveBusyAt(sessionId)
+            : undefined,
+          updatedState: sessionId
+            ? backgroundJobBoard.getState(sessionId)
             : undefined,
-          updatedState: updated?.state,
           updatedCancellationRequested: sessionId
             ? backgroundJobBoard.wasCancellationRequested(sessionId)
             : false,
           updatedLastLiveBusyAt: sessionId
-            ? (backgroundJobBoard.getLastLiveBusyAt(sessionId) ??
-              updated?.lastLiveBusyAt)
+            ? backgroundJobBoard.getLastLiveBusyAt(sessionId)
             : undefined,
         });
         return;
@@ -817,10 +822,10 @@ export function createTaskSessionManagerHook(
           sessionID: sessionId,
           deletedJob: backgroundJobBoard.get(sessionId)
             ? {
-                state: backgroundJobBoard.get(sessionId)?.state,
+                state: backgroundJobBoard.getState(sessionId),
                 parentSessionID:
-                  backgroundJobBoard.get(sessionId)?.parentSessionID,
-                alias: backgroundJobBoard.get(sessionId)?.alias,
+                  backgroundJobBoard.getParentSessionID(sessionId),
+                alias: backgroundJobBoard.getAlias(sessionId),
               }
             : undefined,
           childJobCount: backgroundJobBoard.list(sessionId).length,
@@ -855,14 +860,14 @@ export function createTaskSessionManagerHook(
     if (!isLateCancelledTaskError(existing, status.state)) return;
     log('[task-session-manager] normalized late cancelled task output', {
       taskID: status.taskID,
-      alias: existing?.alias,
-      state: existing?.state,
-      terminalState: existing?.terminalState,
+      alias: backgroundJobBoard.getAlias(status.taskID),
+      state: backgroundJobBoard.getState(status.taskID),
+      terminalState: backgroundJobBoard.getTerminalState(status.taskID),
       result: status.result,
     });
     output.output = formatCancelledTaskStatusOutput(
       status.taskID,
-      existing?.resultSummary,
+      backgroundJobBoard.getResultSummary(status.taskID),
     );
     if (isObjectRecord(output) && isObjectRecord(output.metadata)) {
       output.metadata.state = 'cancelled';

+ 6 - 6
src/multiplexer/session-manager.ts

@@ -252,7 +252,7 @@ export class MultiplexerSessionManager {
         tracked: this.sessions.has(sessionId),
         known: this.knownSessions.has(sessionId),
         ownerInstanceId: this.sessions.get(sessionId)?.ownerInstanceId,
-        backgroundJobState: this.backgroundJobBoard?.get(sessionId)?.state,
+        backgroundJobState: this.backgroundJobBoard?.getState(sessionId),
       });
 
       await this.closeSession(sessionId, 'idle');
@@ -273,7 +273,7 @@ export class MultiplexerSessionManager {
         tracked: this.sessions.has(sessionId),
         known: this.knownSessions.has(sessionId),
         ownerInstanceId: this.sessions.get(sessionId)?.ownerInstanceId,
-        backgroundJobState: this.backgroundJobBoard?.get(sessionId)?.state,
+        backgroundJobState: this.backgroundJobBoard?.getState(sessionId),
       });
       await this.closeSession(sessionId, 'idle');
       return;
@@ -290,7 +290,7 @@ export class MultiplexerSessionManager {
         tracked: this.sessions.has(sessionId),
         known: this.knownSessions.has(sessionId),
         ownerInstanceId: this.sessions.get(sessionId)?.ownerInstanceId,
-        backgroundJobState: this.backgroundJobBoard?.get(sessionId)?.state,
+        backgroundJobState: this.backgroundJobBoard?.getState(sessionId),
       });
       await this.respawnIfKnown(sessionId);
     }
@@ -309,7 +309,7 @@ export class MultiplexerSessionManager {
       tracked: this.sessions.has(sessionId),
       known: this.knownSessions.has(sessionId),
       ownerInstanceId: this.sessions.get(sessionId)?.ownerInstanceId,
-      backgroundJobState: this.backgroundJobBoard?.get(sessionId)?.state,
+      backgroundJobState: this.backgroundJobBoard?.getState(sessionId),
     });
 
     this.deferredIdleCloses.delete(sessionId);
@@ -456,7 +456,7 @@ export class MultiplexerSessionManager {
           sessionId,
           paneId: tracked.paneId,
           reason,
-          backgroundJobState: this.backgroundJobBoard?.get(sessionId)?.state,
+          backgroundJobState: this.backgroundJobBoard?.getState(sessionId),
         },
       );
       return;
@@ -470,7 +470,7 @@ export class MultiplexerSessionManager {
       sessionId,
       paneId: tracked.paneId,
       reason,
-      backgroundJobState: this.backgroundJobBoard?.get(sessionId)?.state,
+      backgroundJobState: this.backgroundJobBoard?.getState(sessionId),
       parentId: tracked.parentId,
       title: tracked.title,
     });

+ 18 - 9
src/tools/cancel-task.ts

@@ -64,8 +64,12 @@ Use only for obsolete, wrong, conflicting, or user-requested cancellation. Accep
         alias: job?.taskID
           ? options.backgroundJobBoard.getAlias(job.taskID)
           : undefined,
-        state: job?.state,
-        terminalState: job?.terminalState,
+        state: job?.taskID
+          ? options.backgroundJobBoard.getState(job.taskID)
+          : undefined,
+        terminalState: job?.taskID
+          ? options.backgroundJobBoard.getTerminalState(job.taskID)
+          : undefined,
         cancellationRequested: job?.taskID
           ? options.backgroundJobBoard.wasCancellationRequested(job.taskID)
           : undefined,
@@ -81,11 +85,16 @@ Use only for obsolete, wrong, conflicting, or user-requested cancellation. Accep
           }
 
           const knownJob = options.backgroundJobBoard.get(requested);
-          if (knownJob && knownJob.parentSessionID !== parentSessionID) {
+          if (
+            knownJob &&
+            options.backgroundJobBoard.getParentSessionID(requested) !==
+              parentSessionID
+          ) {
             log('[cancel-task] rejected unowned tracked raw session', {
               parentSessionID,
               taskID: requested,
-              ownerParentSessionID: knownJob.parentSessionID,
+              ownerParentSessionID:
+                options.backgroundJobBoard.getParentSessionID(requested),
             });
             return unknownTaskOutput(
               requested,
@@ -147,7 +156,7 @@ Use only for obsolete, wrong, conflicting, or user-requested cancellation. Accep
         ].join('\n');
       }
 
-      const cancelled = options.backgroundJobBoard.markCancelled(
+      options.backgroundJobBoard.markCancelled(
         job.taskID,
         args.reason,
         Date.now(),
@@ -156,18 +165,18 @@ Use only for obsolete, wrong, conflicting, or user-requested cancellation. Accep
       log('[cancel-task] marked job cancelled after verified abort', {
         taskID: job.taskID,
         alias: options.backgroundJobBoard.getAlias(job.taskID),
-        previousState: job.state,
-        state: cancelled?.state,
+        previousState: options.backgroundJobBoard.getState(job.taskID),
+        state: options.backgroundJobBoard.getState(job.taskID),
         cancellationRequested:
           options.backgroundJobBoard.wasCancellationRequested(job.taskID),
       });
 
       return [
         `task_id: ${job.taskID}`,
-        `state: ${cancelled?.state ?? 'cancelled'}`,
+        `state: ${options.backgroundJobBoard.getState(job.taskID) ?? 'cancelled'}`,
         '',
         '<task_error>',
-        cancelled?.resultSummary ?? 'cancelled',
+        options.backgroundJobBoard.getResultSummary(job.taskID) ?? 'cancelled',
         '</task_error>',
       ].join('\n');
     },

+ 68 - 0
src/utils/background-job-board.test.ts

@@ -832,5 +832,73 @@ describe('BackgroundJobBoard', () => {
       expect(board.getLastLiveBusyAt('job-1')).toBe(200);
       expect(board.getLastLiveBusyAt('unknown-1')).toBeUndefined();
     });
+
+    test('getParentSessionID: returns parentSessionID after registerLaunch', () => {
+      const board = new BackgroundJobBoard();
+      board.registerLaunch({
+        taskID: 'job-1',
+        parentSessionID: 'parent-1',
+        agent: 'fixer',
+        now: 100,
+      });
+
+      expect(board.getParentSessionID('job-1')).toBe('parent-1');
+      expect(board.getParentSessionID('unknown-1')).toBeUndefined();
+    });
+
+    test('getTerminalState: returns terminal state after updateStatus to terminal', () => {
+      const board = new BackgroundJobBoard();
+      board.registerLaunch({
+        taskID: 'job-1',
+        parentSessionID: 'parent-1',
+        agent: 'fixer',
+        now: 100,
+      });
+
+      expect(board.getTerminalState('job-1')).toBeUndefined();
+      board.updateStatus({ taskID: 'job-1', state: 'completed', now: 200 });
+      expect(board.getTerminalState('job-1')).toBe('completed');
+      expect(board.getTerminalState('unknown-1')).toBeUndefined();
+    });
+
+    test('isTimedOut: true after updateStatus with timedOut: true', () => {
+      const board = new BackgroundJobBoard();
+      board.registerLaunch({
+        taskID: 'job-1',
+        parentSessionID: 'parent-1',
+        agent: 'fixer',
+        now: 100,
+      });
+
+      expect(board.isTimedOut('job-1')).toBe(false);
+      board.updateStatus({
+        taskID: 'job-1',
+        state: 'running',
+        timedOut: true,
+        now: 200,
+      });
+      expect(board.isTimedOut('job-1')).toBe(true);
+      expect(board.isTimedOut('unknown-1')).toBe(false);
+    });
+
+    test('isStatusUncertain: true after updateStatus with statusUncertain: true', () => {
+      const board = new BackgroundJobBoard();
+      board.registerLaunch({
+        taskID: 'job-1',
+        parentSessionID: 'parent-1',
+        agent: 'fixer',
+        now: 100,
+      });
+
+      expect(board.isStatusUncertain('job-1')).toBe(false);
+      board.updateStatus({
+        taskID: 'job-1',
+        state: 'running',
+        statusUncertain: true,
+        now: 200,
+      });
+      expect(board.isStatusUncertain('job-1')).toBe(true);
+      expect(board.isStatusUncertain('unknown-1')).toBe(false);
+    });
   });
 });

+ 40 - 0
src/utils/background-job-board.ts

@@ -352,6 +352,46 @@ export class BackgroundJobBoard {
     return job?.lastLiveBusyAt;
   }
 
+  /**
+   * Get the parent session ID for a job, or undefined if not found.
+   */
+  getParentSessionID(taskID: string): string | undefined {
+    const job = this.get(taskID);
+    return job?.parentSessionID;
+  }
+
+  /**
+   * Get the terminal state for a job, or undefined if not found or not terminal.
+   */
+  getTerminalState(taskID: string): TaskOutputState | undefined {
+    const job = this.get(taskID);
+    return job?.terminalState;
+  }
+
+  /**
+   * Get the timedOut flag for a job, or false if not found.
+   */
+  isTimedOut(taskID: string): boolean {
+    const job = this.get(taskID);
+    return !!job?.timedOut;
+  }
+
+  /**
+   * Get the statusUncertain flag for a job, or false if not found.
+   */
+  isStatusUncertain(taskID: string): boolean {
+    const job = this.get(taskID);
+    return !!job?.statusUncertain;
+  }
+
+  /**
+   * Get the current state of a job, or undefined if not found.
+   */
+  getState(taskID: string): BackgroundJobState | undefined {
+    const job = this.get(taskID);
+    return job?.state;
+  }
+
   resolve(
     parentSessionID: string,
     taskIDOrAlias: string,