tasks: address review feedback on onBeforeMarkLost hook

- Re-read task after async hook returns false before calling markTaskLost,
  guarding against concurrent completion during the hook (Codex P1)
- Validate hook return value: normalize undefined/null to { recovered: false }
  instead of crashing the sweep (Codex P2)
- Add log.warn spy assertion to the "hook throws" test to verify the warning
  is actually emitted, not just that the return value is correct (Greptile P2)
- Add comment on previewTaskRegistryMaintenance noting it cannot call the
  async hook, so recovered tasks appear under reconciled in preview (Greptile P2)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/tasks/detached-task-runtime.test.ts

@@ -17,6 +17,24 @@ import {
 } from "./detached-task-runtime.js";
 import type { TaskRecord } from "./task-registry.types.js";
 
+const { mockLogWarn } = vi.hoisted(() => ({
+  mockLogWarn: vi.fn(),
+}));
+vi.mock("../logging/subsystem.js", () => ({
+  createSubsystemLogger: () => ({
+    subsystem: "tasks/detached-runtime",
+    isEnabled: () => true,
+    trace: vi.fn(),
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: mockLogWarn,
+    error: vi.fn(),
+    fatal: vi.fn(),
+    raw: vi.fn(),
+    child: vi.fn(),
+  }),
+}));
+
 function createFakeTaskRecord(overrides?: Partial<TaskRecord>): TaskRecord {
   return {
     taskId: "task-fake",
@@ -37,6 +55,7 @@ function createFakeTaskRecord(overrides?: Partial<TaskRecord>): TaskRecord {
 describe("detached-task-runtime", () => {
   afterEach(() => {
     resetDetachedTaskLifecycleRuntimeForTests();
+    mockLogWarn.mockClear();
   });
 
   it("dispatches lifecycle operations through the installed runtime", async () => {
@@ -200,6 +219,10 @@ describe("detached-task-runtime", () => {
         task,
       });
       expect(result).toEqual({ recovered: false });
+      expect(mockLogWarn).toHaveBeenCalledWith(
+        "onBeforeMarkLost hook threw, proceeding with markTaskLost",
+        expect.objectContaining({ taskId: "task-throw", runtime: "acp" }),
+      );
     });
   });
 });

src/tasks/detached-task-runtime.ts

@@ -119,7 +119,11 @@ export async function onBeforeMarkLost(params: {
     return { recovered: false };
   }
   try {
-    return await hook(params);
+    const result = await hook(params);
+    if (result && typeof result.recovered === "boolean") {
+      return result;
+    }
+    return { recovered: false };
   } catch (err) {
     log.warn("onBeforeMarkLost hook threw, proceeding with markTaskLost", {
       taskId: params.taskId,

src/tasks/task-registry.maintenance.ts

@@ -252,6 +252,9 @@ export function reconcileTaskLookupToken(token: string): TaskRecord | undefined
   return task ? reconcileTaskRecordForOperatorInspection(task) : undefined;
 }
 
+// Preview is synchronous and cannot call the async onBeforeMarkLost hook,
+// so recovered tasks are counted under reconciled here. The real sweep
+// in runTaskRegistryMaintenance splits them into reconciled vs recovered.
 export function previewTaskRegistryMaintenance(): TaskRegistryMaintenanceSummary {
   taskRegistryMaintenanceRuntime.ensureTaskRegistryReady();
   const now = Date.now();
@@ -314,18 +317,23 @@ export async function runTaskRegistryMaintenance(): Promise<TaskRegistryMaintena
         runtime: current.runtime,
         task: current,
       });
-      if (recovery.recovered) {
-        const fresh = taskRegistryMaintenanceRuntime.getTaskById(current.taskId);
-        if (fresh && isActiveTask(fresh)) {
-          recovered += 1;
-        }
+      const freshAfterHook = taskRegistryMaintenanceRuntime.getTaskById(current.taskId);
+      if (!freshAfterHook || !shouldMarkLost(freshAfterHook, now)) {
         processed += 1;
         if (processed % SWEEP_YIELD_BATCH_SIZE === 0) {
           await yieldToEventLoop();
         }
         continue;
       }
-      const next = markTaskLost(current, now);
+      if (recovery.recovered) {
+        recovered += 1;
+        processed += 1;
+        if (processed % SWEEP_YIELD_BATCH_SIZE === 0) {
+          await yieldToEventLoop();
+        }
+        continue;
+      }
+      const next = markTaskLost(freshAfterHook, now);
       if (next.status === "lost") {
         reconciled += 1;
       }