Warning log assertion missing from "hook throws" test

The test verifies the return value ({ recovered: false }) when the hook throws, but doesn't assert that a warning was actually logged. Given that the log warning is an explicit behavioural commitment (operators rely on it to diagnose misbehaving plugins), a spy on log.warn would make the contract explicit and guard against accidentally silencing the log in the future.

This is a comment left during a code review.
Path: src/tasks/detached-task-runtime.test.ts
Line: 183-199

Comment:
**Warning log assertion missing from "hook throws" test**

The test verifies the return value (`{ recovered: false }`) when the hook throws, but doesn't assert that a warning was actually logged. Given that the log warning is an explicit behavioural commitment (operators rely on it to diagnose misbehaving plugins), a spy on `log.warn` would make the contract explicit and guard against accidentally silencing the log in the future.

How can I resolve this? If you propose a fix, please make it concise.

Fixed in e276b09. Added vi.hoisted mock for createSubsystemLogger and asserted mockLogWarn is called with the expected taskId, runtime, and message.

Re-read task state before calling markTaskLost

runTaskRegistryMaintenance now awaits onBeforeMarkLost before reconciling stale tasks, but then calls markTaskLost(current, now) using the pre-await snapshot. If a task reaches a terminal state while the hook is running (for example, an async plugin check returns { recovered: false } after the task succeeded), this path overwrites that terminal state to lost. Re-fetching the task and re-running shouldMarkLost after the await is needed to avoid corrupting final outcomes.

Useful? React with 👍 / 👎.

Fixed in e276b09. The sweep now re-reads the task and re-checks shouldMarkLost after the async hook returns, regardless of the hook result. If the task completed or was deleted during the hook call, the sweep skips it.

Guard against invalid onBeforeMarkLost return values

The maintenance loop assumes the hook always returns an object with a recovered boolean and dereferences it directly. A JS plugin or buggy runtime implementation that returns undefined/null causes a TypeError here, aborting the sweep instead of falling back to normal markTaskLost behavior. Since this seam is plugin-provided and intended to be fail-safe, normalize/validate the hook result before reading recovered.

Useful? React with 👍 / 👎.

Fixed in e276b09. The dispatch wrapper now validates the hook result: if (result && typeof result.recovered === "boolean"). Invalid returns (undefined, null, missing field) normalize to { recovered: false }.

Time out detached recovery hooks in maintenance sweep

runTaskRegistryMaintenance now awaits tryRecoverTaskBeforeMarkLost(...) inline for each stale task, but there is no timeout or cancellation path if a plugin hook never resolves. In that case the sweep promise never settles, startScheduledSweep never clears sweepInProgress, and periodic maintenance stops running entirely until process restart. Please race this await with a bounded timeout and fall back to normal markTaskLost behavior on timeout so one bad hook cannot wedge reconciliation.

Useful? React with 👍 / 👎.

Bound recovery hook await with a timeout

Awaiting tryRecoverTaskBeforeMarkLost(...) here has no timeout, so a plugin hook that never resolves will keep runTaskRegistryMaintenance pending indefinitely; in that state startScheduledSweep never clears sweepInProgress, and all subsequent scheduled sweeps are skipped until restart. This turns one bad recovery hook into a permanent reconciliation outage for stale tasks, so the await should be raced with a bounded timeout and fall back to normal markTaskLost behavior on timeout.

Useful? React with 👍 / 👎.

Bound recovery hook await with timeout

Awaiting tryRecoverTaskBeforeMarkLost(...) without any timeout lets a misbehaving plugin hook stall this sweep forever (for example, if it returns a never-resolving Promise). In that state runTaskRegistryMaintenance() never settles, startScheduledSweep() never clears sweepInProgress, and all subsequent scheduled maintenance runs are skipped until restart, so stale tasks stop being reconciled entirely.

Useful? React with 👍 / 👎.