fix(node-manager): task cancel liveness and lifecycle hardening

[Apollon77]

Hardening pass over the Task layer's cancel, shutdown and task-lifecycle paths. All defects here are pre-existing (they predate Increment 3) and none were covered by a test. Base is node-manager, so CI runs via umbrella #3948 — the full local gate below is the only gate this PR itself gets.

Defects fixed

Abort intent had nowhere to live but the gate, and the gate was created late. #abortGate silently no-ops when no gate entry exists, and the driver only created one inside #drive, after #admit and the first #persist. A cancel arriving in that window was discarded: the task then wrote its intents to the peer after the cancel was accepted, parked on a peer that would never commit, and cancel() never settled — holding the caller's enclosing act transaction open with it. The gate is now created in #track before driving starts, and the driver re-checks for a recorded abort after admission and again after the phase context is built. That second checkpoint matters: a check at the loop top still let a cancel arriving during endpoint.act through to a device write.

A cancel overlapping shutdown was lost. cancel() had no shutdown guard, and everything after its await ran outside what asyncDispose waits for: dispose overwrote the gate's abort reason, the drive unwound without persisting, and cancel()'s continuation then marked the task cancelled in memory and spawned a rollback whose driving dispose had already passed over — persisting into a mutex about to close (Mutex.run throws synchronously once closed). Storage kept the forward task non-terminal with no rollback, so the next start re-applied exactly what was cancelled, with no record the cancel happened. cancel() now refuses with TaskManagerClosingError once the endpoint leaves Lifecycle.Status.Active, and the cancelled record and rollback record are written in one transaction. A task that fails during teardown is left resumable instead of rolled back outside the drain, and a task type registered during teardown no longer resumes into it.

internal.live had no terminal lifecycle. Ids are deterministic and nothing ever removed entries, so re-running a task after a cancel returned the cancelled handle: no phase ran, nothing persisted, no error was raised, and the requested change was never applied for the rest of the process lifetime — a restart "fixed" it, which is the worst possible signature. The map was also unbounded. Terminal ids are now reusable; dedup applies only to a live non-terminal task representing the same request; a differing request is rejected rather than silently ignored; and terminal tasks are kept as bounded observable history (terminalRetention, default 50) in both memory and storage, with inherited records trimmed at startup.

A commit gate only ever observed success. Reconciliation turns an apply error into commitFailed, and an unrecoverable follow-up drops the item — but the wait predicate looked only for committed, so a real device rejection parked its task forever and blocked the automatic rollback. Such a gate now fails with TaskFailedError, which the driver routes into the normal failure path. Underneath that: dropItem announces on itemRemoved while the gate watched only itemChanged, so a parked gate had no wake source at all.

A gate could park on an event already gone. The predicate was evaluated once before the parking observers were registered, so a change, removal or reachability flip arriving in that window was announced before anything listened. Registration now precedes the first evaluation, which goes through the same coalescing path as any later wakeup — closing the window without adding a verify-reconcile to every park.

A caller's externalId was discarded on dedup. The second caller received a handle carrying the first caller's id, and get()/cancel() on its own id both returned undefined — indistinguishable from "no such task" and from "nothing to revert". Tasks now carry the set of external ids that claim them, a conflicting claim is refused, and cancel() raises TaskNotFoundError for an unknown id so undefined means only "nothing to revert".

Contracts chosen

• Dedup identity is (id, parameters) against live non-terminal work only. Parameters compare as storage returns them, because the persistence codec drops undefined-valued properties at every depth and leaves an undefined array element as a hole — comparing raw values rejects an idempotent re-issue after a restart. A Map compares by entries, since a structural comparison sees no properties on one and would accept any map for any other.
• Cancel vs shutdown: refuse. Once teardown starts no state write can land at all, so completing the cancel from inside the behavior is not achievable; the alternative is an in-memory-only cancel that storage contradicts on the next start. The guard reads the endpoint's construction status rather than a behavior-local flag, because the behavior's own dispose runs after the drive resumes (measured). This depends on Construction.close applying Destroying before the destructor reaches behaviors.close().
• Terminal tasks are observable after a restart — loaded into live as history, visible to get()/tasks, still cancellable through their recorded changeSet, never driven again.

Refuted while investigating

• The shutdown variant of the pre-gate defect does not hang. Once close() begins every endpoint.act throws DestroyedDependencyError, so such a task fails on its next act rather than reaching a phase and parking. The real (smaller) defect was shutdown turning a resumable task into an in-memory failed; that is fixed.
• Two proposed retention self-eviction paths were traced and shown unreachable (Set.add of a present member leaves size unchanged; #find reads only live and #resumeType skips terminal records). Two other reachable ones were found and fixed: terminalRetention of 0 made every write evict the id it was storing, and a terminal write could queue an id a live re-run already owned.

Verification

Every fix was written test-first and mutation-proofed per hunk (revert the hunk, confirm red, restore). Notable reds: Test timeout: Promise did not resolve within one (virtual) hour at the cancel() await; a device write present after an accepted cancel; and a test-suite crash from an unhandled rejection when #spawnRevert threw out of #drive's catch.

Three review rounds: the implementer's own, an independent adversarial pass over the cumulative diff with an assumption-free prompt, and a further adversarial pass. The independent pass caught that three tests claiming to cover the new itemRemoved subscription did not — the fake peer dropped the item during the initial evaluation, so the gate returned before observers registered and deleting the production line left all three green. Those now reach a parked gate. FakePeer also now models the reconciler's real two-pass recoverable/unrecoverable behaviour.

Gates (repository root):

npm run build-clean   exit 0
npm run format-verify  All matched files use Prettier code style!
npm run lint           exit 0
npm test -p packages/node-manager   188/188 ESM + CJS + Web  (was 165)
npm test -p packages/node           1723 ESM / 1723 CJS / 1714 Web

No CHANGELOG entry: the package is unreleased on this branch and the entry lands at umbrella merge. No type casts added in production code.

Known-weak, stated plainly

• The pre-gate shutdown test asserts an in-memory progress.state, because nothing observable is persisted in that scenario. It also no longer uniquely pins the post-admission abort check — the closing guard produces the same outcome, and the only remaining discriminator is a log message.
• The startup-prune durability check is a witness rather than a discriminating assertion.

Carried forward, not in this PR

#resumeType bypasses the parameter, cancelling, pending-revert and resourceKey guards that #spawn enforces (only the closing guard was added here). Reachable when the paired persist in #drive's catch fails while the rollback's own persist succeeds: a forward task and its own rollback then both resume non-terminal and write opposing intents. Tracked for the next hardening PR together with the abandoned in-flight evaluation on abort.

Reconcile-layer defects found in passing, untouched here: planActions maps a failed remove to retry, which calls apply and re-adds the item a rollback was deleting; ReconcilerBehavior.#wirePeer ignores itemRemoved, so a drop triggers no reconcile pass; defaultRecoverable(0) treats any non-Matter-status apply exception as unrecoverable.

🤖 Generated with Claude Code

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[Copilot]

Pull request overview

Hardens Task Manager cancellation, lifecycle, reconciliation gates, deduplication, and retention.

Changes:

• Makes cancellation and rollback persistence lifecycle-aware.
• Adds terminal retention and external-ID deduplication.
• Fixes gate wakeups and rejected-intent handling with expanded tests.

Reported build, formatting, lint, and relevant package tests pass.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/node-manager/src/task/TaskManagerBehavior.ts	Implements lifecycle, deduplication, rollback, and retention changes.
packages/node-manager/src/task/RunningTaskContext.ts	Hardens gate evaluation and item-removal handling.
packages/node-manager/src/task/Task.ts	Persists multiple external IDs.
packages/node-manager/src/task/types.ts	Updates task status typing.
packages/node-manager/src/task/errors.ts	Adds closing and not-found errors.
packages/node-manager/test/task/CancelRobustnessTest.ts	Tests cancellation and shutdown races.
packages/node-manager/test/task/TaskContextGateTest.ts	Tests gate wakeups, aborts, and failures.
packages/node-manager/test/task/TaskLifecycleTest.ts	Tests restart, deduplication, and rollback lifecycle.
packages/node-manager/test/task/TaskManagerBehaviorTest.ts	Tests retention and external-ID behavior.
packages/node-manager/test/task/helpers.ts	Expands fake reconciliation behavior.
packages/node-manager/test/task/groups/RotateGroupKeyIntegrationTest.ts	Makes phase-specific parking assertions robust.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.