Forráskód Böngészése

fix(background-job-board): restore log field semantics after review

Fix logging regressions caught in PR 651 review:

- task-session-manager: 'previous' fields now read from `before` snapshot,
  not from the board after markRunningFromLiveSession mutation
- task-session-manager: 'updated' fields now read from `updated` snapshot
- task-session-manager: separate parsedState from boardState in late
  cancelled logs (was overloading the `state` key)
- cancel-task: revert speculative `|| isRunning(taskID)` in stillRunning
  that changed returned state from 'error' to 'running'
- cancel-task: restore boardState as state string (was boolean)

Add regression test for cancelSessionByID state contract: must return
'error' when abort throws non-SessionStillRunningError, regardless of
board state.

Refs #648
Michael Henke 1 hónapja
szülő
commit
9a8cd303ef

+ 23 - 41
src/hooks/task-session-manager/index.ts

@@ -235,9 +235,10 @@ export function createTaskSessionManagerHook(
     if (isLateCancelledTaskError(existing, status.state)) {
       log('[task-session-manager] suppressed late cancelled task error', {
         taskID: status.taskID,
-        alias: backgroundJobBoard.getAlias(status.taskID),
-        state: status.state,
-        terminalState: backgroundJobBoard.getTerminalState(status.taskID),
+        alias: existing?.alias,
+        parsedState: status.state,
+        boardState: existing?.state,
+        terminalState: existing?.terminalState,
         result: status.result,
       });
       return existing;
@@ -317,9 +318,10 @@ export function createTaskSessionManagerHook(
       );
       log('[task-session-manager] normalized late cancelled injected failure', {
         taskID: status.taskID,
-        alias: backgroundJobBoard.getAlias(status.taskID),
-        state: status.state,
-        terminalState: backgroundJobBoard.getTerminalState(status.taskID),
+        alias: existing?.alias,
+        parsedState: status.state,
+        boardState: existing?.state,
+        terminalState: existing?.terminalState,
         result: status.result,
       });
       rememberProcessedInjectedCompletion(occurrenceId);
@@ -763,22 +765,16 @@ export function createTaskSessionManagerHook(
         const before = sessionId
           ? backgroundJobBoard.get(sessionId)
           : undefined;
-        if (sessionId) {
-          backgroundJobBoard.markRunningFromLiveSession(sessionId);
-        }
-        if (
-          before &&
-          sessionId &&
-          backgroundJobBoard.wasCancellationRequested(sessionId)
-        ) {
+        const updated = sessionId
+          ? backgroundJobBoard.markRunningFromLiveSession(sessionId)
+          : undefined;
+        if (before?.cancellationRequested) {
           log('[task-session-manager] busy observed after cancel request', {
             sessionID: sessionId,
-            previousState: backgroundJobBoard.getState(sessionId),
-            previousTerminalState:
-              backgroundJobBoard.getTerminalState(sessionId),
-            terminalUnreconciled:
-              backgroundJobBoard.isTerminalUnreconciled(sessionId),
-            resultSummary: backgroundJobBoard.getResultSummary(sessionId),
+            previousState: before.state,
+            previousTerminalState: before.terminalState,
+            terminalUnreconciled: before.terminalUnreconciled,
+            resultSummary: before.resultSummary,
           });
         }
         log('[task-session-manager] busy/status busy observed', {
@@ -786,27 +782,13 @@ export function createTaskSessionManagerHook(
           managesSession: sessionId
             ? options.shouldManageSession(sessionId)
             : false,
-          previousState: sessionId
-            ? backgroundJobBoard.getState(sessionId)
-            : undefined,
-          previousTerminalState: sessionId
-            ? backgroundJobBoard.getTerminalState(sessionId)
-            : undefined,
-          previousCancellationRequested: sessionId
-            ? backgroundJobBoard.wasCancellationRequested(sessionId)
-            : false,
-          previousLastLiveBusyAt: sessionId
-            ? backgroundJobBoard.getLastLiveBusyAt(sessionId)
-            : undefined,
-          updatedState: sessionId
-            ? backgroundJobBoard.getState(sessionId)
-            : undefined,
-          updatedCancellationRequested: sessionId
-            ? backgroundJobBoard.wasCancellationRequested(sessionId)
-            : false,
-          updatedLastLiveBusyAt: sessionId
-            ? backgroundJobBoard.getLastLiveBusyAt(sessionId)
-            : undefined,
+          previousState: before?.state,
+          previousTerminalState: before?.terminalState,
+          previousCancellationRequested: before?.cancellationRequested ?? false,
+          previousLastLiveBusyAt: before?.lastLiveBusyAt,
+          updatedState: updated?.state,
+          updatedCancellationRequested: updated?.cancellationRequested ?? false,
+          updatedLastLiveBusyAt: updated?.lastLiveBusyAt,
         });
         return;
       }

+ 30 - 0
src/tools/cancel-task.test.ts

@@ -426,6 +426,36 @@ describe('cancel_task tool', () => {
     });
   });
 
+  test('cancelSessionByID returns state: error when abort throws non-SessionStillRunningError, even if board shows running', async () => {
+    const { board, abort, cancelTask } = createTool({
+      includeDelete: false,
+      abort: async () => {
+        throw new Error('network timeout');
+      },
+    });
+    // Register a running job so that isRunning(taskID) would be true
+    // if the function incorrectly checks it.
+    board.registerLaunch({
+      taskID: 'ses_running',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+    });
+    // Override resolve to return undefined, forcing the cancelSessionByID
+    // raw session path instead of the tracked task path.
+    board.resolve = mock(() => undefined);
+
+    const output = await cancelTask.execute(
+      { task_id: 'ses_running', reason: 'regression guard' },
+      context,
+    );
+
+    expect(abort).toHaveBeenCalledWith({ path: { id: 'ses_running' } });
+    // cancelSessionByID must return state: error for non-SessionStillRunningError,
+    // NOT state: running (which would happen if || isRunning() were present).
+    expect(String(output)).toContain('state: error');
+    expect(String(output)).not.toContain('state: running');
+  });
+
   test('denies non-orchestrator agents', async () => {
     const { cancelTask } = createTool();
 

+ 2 - 4
src/tools/cancel-task.ts

@@ -193,9 +193,7 @@ async function cancelSessionByID(
   try {
     await abortAndVerifySession(options, taskID);
   } catch (error) {
-    const stillRunning =
-      error instanceof SessionStillRunningError ||
-      options.backgroundJobBoard.isRunning(taskID); // ponytail: intent-revealing query
+    const stillRunning = error instanceof SessionStillRunningError;
     log('[cancel-task] raw session abort failed', {
       taskID,
       stillRunning,
@@ -275,7 +273,7 @@ async function abortAndVerifySession(
       stableStoppedForMs: stableStoppedSince
         ? Date.now() - stableStoppedSince
         : 0,
-      boardState: options.backgroundJobBoard.isRunning(taskID), // ponytail: intent-revealing query
+      boardState: options.backgroundJobBoard.getState(taskID),
       boardLastLiveBusyAt: options.backgroundJobBoard.getLastLiveBusyAt(taskID),
     });
     const boardLastLiveBusyAt =