[WIP] Node Manager: desired-state, reconciliation & multi-step tasks - #3948
Draft
Apollon77 wants to merge 128 commits into
Draft
[WIP] Node Manager: desired-state, reconciliation & multi-step tasks#3948Apollon77 wants to merge 128 commits into
Apollon77 wants to merge 128 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…edStateBehavior Drop the second `, unknown` type param from both Observable declarations in DesiredStateBehavior.Events to match the codebase convention. Convert the two test handlers that return number (Array.push) to block bodies so they type-check under the stricter void return. Remove the inline WHAT comment above static schema. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add export * from "./desired-state/index.js" to system behavior barrel - Add DesiredStateBehavior to ClientNode.RootEndpoint.with(...) - New DesiredStatePersistenceTest: verifies registration on ClientNode and intent persistence across a node restart via shared Environment storage - Update PEER1_STATE in ClientNodeTest to include desiredState initial state Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add test coverage for GroupCapacityExceededError mapping and remove the unused unknownKindMapped fixture entry from the cache. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-emit a peer's BasicInformation softwareVersion change as a node-level lifecycle signal, wired into Peers BasicInformation instrumentation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure status x mode branch-table function, internal to the package (consumed by the reconciler via #-import, not part of the public API). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Opt-in ServerNode-root behavior driving reachable peers toward intended state: pure planActions decision + executeActions executor, six triggers (settle, sweep, peers add/del, subscription-active, intent-change, software-version-change), per-peer in-flight guard, capacity refresh, and asyncDispose cleanup. Reachability gated on an active sustained subscription. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion - Wire per-peer trigger handlers directly on the peer ObserverGroup instead of via this.callback, so they are torn down on peer removal (no reactor leak on peer churn). - #reachable mirrors NetworkClient.subscriptionActive: a sustained subscription counts as reachable only once active, not merely created. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reconciler engine: @matter/node-manager package, ReconcilerBehavior (opt-in ServerNode-root, planActions decision + executeActions executor, six triggers, per-peer in-flight guard, capacity refresh, asyncDispose), plus @matter/node ephemeral capacity cache and softwareVersionChanged signal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rity bands Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng, dispose race Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ting Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ard test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… tradeoff Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… unread Pre-flight admission stays meaningful instead of failing open; the device write remains the authoritative gate for over-capacity (RESOURCE_EXHAUSTED). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two tests covering a rollback the manager cannot prepare were removed with the previous change, on the grounds that no route to that state remained. That was wrong: shutdown still refuses the spawn, and so does an exhausted identity reservation. Both are restored and reach the refusal through exhaustion, which is deterministic and needs no unhealthy node. Their previous setup could only reach it by starting a rollback through the public path, which no longer exists. Each fails against the code it guards: removing the failure path's catch crashes the suite with an unhandled rejection, and dropping the driver a declined cancel must give back leaves the task unable to converge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Undoing a run restores the values it found. Once a later run of the same slot has committed its own, those values are historical, and restoring them overwrites an outcome nobody asked to undo. Only the most recently finished run of a slot may now be rolled back. A persisted run also stays pending until its task has actually been rebuilt and admitted. It was dropped from the pending tier first, so a task that refuses to be rebuilt from its record — a custom type validating its own parameters — left the run holding no slot and answering no lookup, free for new work to overlap the unfinished intents it had already written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A task that named an item kind no registered kind matched — a typo in the kind name, or a kind whose registration had not happened — reported success having written nothing to the device. The item was skipped when actions were applied and then marked committed anyway, so the gate a task waits on resolved and the task completed green. Nothing was written, nothing failed, and nothing was logged. An unregistered kind now fails the item, which drops it and fails the task through the path that already exists for work that cannot converge. The reason is logged where the error is otherwise reduced to a status code, so the failure names the kind that could not be resolved rather than only reporting that an item disappeared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A run's terminal state and its place in the retirement order were written separately, so a process that stopped between the two left a record that was finished but had no position. Loading it gave it order zero, which sorts ahead of every sequenced run of the same target, so the check that stops an older run's undo from overwriting a newer one could not see it. The order is now stamped as the outcome is assigned, so the write that records what happened carries where it belongs. Retirement writes nothing and only hands back the target, which also removes the case where a refused retirement write left a finished run holding a target forever. A record written before this, finished with no position, is discarded at load with a warning, as records predating per-run identity already are. The identity reservation was advanced before the write that makes it durable, so a refused write left a boundary storage does not back and the next start could hand those identities to different work. It now follows the write, alongside the retired-record refresh that already did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/node/test/behavior/system/icd/IcdClientTest.ts # packages/node/test/behavior/system/network/ConnectionStateTest.ts # packages/node/test/behaviors/icd-management/IcdManagementServerTest.ts
A task type was a class that a run was an instance of, so what the type knows and what one attempt has done lived in one object. Asking whether a finished run could still be undone meant rebuilding it as an object, which needed its type registered and risked writing a rebuilt copy back over the stored record. A task type is now a TaskDefinition: its name, the target it claims, its phases, and the questions it answers about a run. A run is a plain object holding identity, parameters and progress, and is never subclassed. Definitions answer questions through a view that a live run and a stored record both satisfy, so asking a live run never builds its storage record and asking a finished one never rebuilds it. Revertibility takes the type's own parameters alongside that view, because whether work can be undone depends on what it was asked to do and not only on how far it got. Registration now takes the definition, which carries its own name. A definition may refuse parameters it cannot interpret. Subclassing allowed that from a constructor and the split would otherwise have dropped it, letting a record written by an older version of a task type be driven on parameters that type no longer honours. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cancel of a finished run rebuilt it as an object before deciding whether it could be rolled back at all, so a run that declines cancel — and one whose answer is a rollback it already recorded — still paid for a writable copy and for the definition hooks that copy runs. Every decision now reads the run through the view a live run and a stored record both satisfy. The copy is created only once the cancel is going ahead, to carry the mutation that follows. Registration of a finished run's type is required at the one point that needs it, which is deciding on a new rollback, and is checked before the copy rather than by making it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nder Whether a run may be rolled back was answered by looking its type up in the registry, which resolves whatever is registered at the time of asking. Registering a type again — the same name, a different definition — could therefore decide the question for work already in flight, and roll back a run its own definition declared forward-only. Auto-rollback suppression resolved the same way. A live run now answers from the definition it was built with, the same one that supplies its phases. Only a stored record resolves a definition by type, because that is all a record has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parameters reach this layer untyped — from a caller, and from storage where an older version of a task type may have written them — while a definition's members are declared for its own parameter type. Nothing established that the one was the other. The type system did not object, because definitions are stored type-erased and their members are methods, whose parameters TypeScript checks bivariantly. So every question a definition answered about a run took parameters nobody had checked, and whether a check had run at all depended on which path had reached it first. A definition and parameters it has accepted are now one thing, and constructing it is the only place parameters cross from untyped to typed and the only place that may refuse. Everything a definition is asked is asked of that, so there is no signature left that takes raw parameters and no order left to get wrong. Starting work takes the definition rather than its name, so a caller's parameters are checked where they are written and never enter untyped at all. A name still identifies a type in storage and on resume, which is all a stored record has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ject Starting work checked that a definition with the given name was registered and then executed the object the caller passed. A lookalike carrying a registered name could therefore bypass that name's rules — including the one that reserves rollbacks for the manager — and would drive phases that a restart, which has only the name, could not resume. The caller's definition names the work and types its parameters; the registered definition of that name is what runs. The two group tasks a caller may start are exported, so requiring the definition does not put them out of reach. Comments that narrated the change rather than stating what holds are replaced, and helper failures throw typed errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Starting work took a definition to type its parameters and ran the definition registered under that name. Those need not be the same object, so a definition declaring different parameters could borrow a registered name, satisfy the compiler against its own declaration, and hand the registered definition parameters it never accepted. The definition given must now be the one registered under its name. With one object there is one parameter type, so what the compiler checks is what runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A run had two shapes and lived in one of three tables. Unfinished work this process was driving was one kind of object; work awaiting resume and work that had finished were another; and which one a run was depended on where it sat. Every verb had to remember all three, and two of them did not — cancel and retrying a rollback both reported a run awaiting resume as absent. A run is now one shape in one table, whatever phase it is in. Which phase it is in is answered by asking, rather than by which table it was put in, so there is no table left to forget. A separate index records which runs this process has instantiated, because that is a different question from which runs exist. A handle now reads the run itself rather than a copy taken when the handle was made, so it keeps answering as the run changes — including after the run has finished, which is the case an operator watches and the one a copy froze. Two other things follow. A run holds its own target from the moment it is loaded, so resuming it must ignore itself when asking who holds that target. And the rule deciding which run a caller's own name refers to is now written down, rather than implied by the order the three tables were searched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lifetime A run's gate, its driver and the note that a cancel was accepted lived in three tables on the manager, keyed by run id and kept in agreement by hand. The object a driver worked with held them at arm's length and carried a copy of the run's progress besides. They are now one object, which exists from the moment a run is admitted until this process stops being responsible for it — when its outcome is recorded, or when a shutdown leaves it for the next start. It is deliberately not the driver: a driver stops before a run's outcome is durable, and through that window the run still holds its target and must still refuse a re-run. Naming that window makes it answerable. A run whose driver stopped without its outcome reaching storage kept its target and could still be joined by a caller re-issuing the same request, who was handed a run nothing was advancing. It is now refused, and says why. The object a driver worked with is gone. A run is its record; what this process is doing about it is the execution. Asking for a gate no longer creates one. It could only ever have created one for a run this process was not responsible for, where an abort against it would have done nothing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A run was changed in memory and then written, so every path that could be refused had to put the run back by hand. Each such path kept its own list of what to undo, and each round of review found the entry one of them had forgotten. A change is now described, written, and adopted only once the write has landed. A refused write leaves the run untouched because it was never touched, so there is nothing to put back and no list to forget an entry from. Storage already restores itself on a failed write; with no second copy of a run in memory, that is the whole of it. The unit is the write, not the field. A run's outcome and its place in the retirement order go together, because a crash between them leaves a record that cannot be ordered and is discarded when it is read back. A run and the rollback that undoes it go together, because a run that named a rollback nothing created could never be rolled back. Whether a run is being rolled back right now is read from the rollback itself rather than from a link written later, so a second cancel arriving while the first is still being recorded is given the rollback that exists instead of trying to create another. Parking is exempt and says why: it reports something already true, and waiting for a write would let a driver read a stale note and stop with nothing left to advance the run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rded A run whose very first write was refused kept the target it had claimed, with no driver to advance it and no record any restart could find. The target stayed blocked for the life of the process. Such a run now leaves the table. Its record carries its outcome out with it, so the handle its caller already holds says what happened instead of answering "running" forever — the one place memory may differ from storage, because there is no longer a record to differ from. Whether a run has ever been recorded belongs to the run and not to an attempt at driving it, so giving a run its driver back does not forget that it is durable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…llback Two faults in how a run's place was read back. History excluded every finished run whose target a run currently holds, rather than the one run still holding it. A second run of the same target therefore hid every earlier run of it for as long as it was live. And a rollback written in the same transaction as the run it undoes was not treated as durable until its own driver wrote something. If that later write was refused, the rollback was discarded as though it had never been recorded, leaving the run naming a rollback nothing held. A record is durable from the moment a transaction carrying it lands, whichever run of that transaction it belongs to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/node/test/behavior/state/managed/values/ListManagerTest.ts
… why a request was refused When a task is cancelled or fails, the manager creates a rollback that puts the device back the way it found it. If that device is physically gone, the rollback waits for it forever — and while it waits, nothing new may be done to that device, because new work would fight the rollback for the same settings. There was no way out of that. `abandon(runId)` is the way out. It names the rollback, stops it, records that the undo was given up on, and releases the device so work can proceed against it again. The device is knowingly left part-changed, so the rollback ends in a new state of its own, `abandoned`: not "cancelled", which means an undo nobody needed, and not "failed", which invites a retry. Retrying an abandoned rollback is refused, and so is abandoning one that a retry has already replaced. The ordering is the whole difficulty. Recording an outcome does not stop the code driving the run — it is waiting on the device and will wake up and keep writing — so a verb that ends a run has to stop the driver, wait for it, decide from whatever state it left behind, and only then release the device. Cancel already did all of that. Rather than write it twice, a verb now takes an exclusive claim on a run's outcome for the length of its transition; a second request for the same run waits for that claim and answers from what it recorded, so a duplicate cancel is still told about the rollback the first one created. The claim lives with the run rather than with the code driving it, because a transition hands that code back part-way through and anything watching only the driver would see the run as free too early. A claim is per run, so it cannot say anything about the relation between two runs. Retrying a rollback rewrites the link on the run being undone and admits the replacement before the write that records it, so an abandon asks which rollbacks are live rather than which one the record names — otherwise it would record an abandonment while a fresh rollback was driving the device. Cancelling a rollback is now refused outright and points at `abandon`: it used to be accepted and left the rollback looking like one nothing needed, with no record that a device had been left inconsistent. "Is a rollback" means one thing everywhere now — the run declares what it undoes — so a task type that declares it is treated the same way whatever it is called. A refusal now says which of its causes applied. One error class covered ten distinct reasons a request could be turned down, distinguishable only by reading its message, so a caller could not act on the difference between "someone else is using this device, decide whether to stop them", "this will clear on its own, retry" and "register the task type first". Each cause has its own error class carrying a code from one enumeration, so a user interface can translate it and a caller can branch on it. The general case can no longer be thrown. Finally, a stop the manager never asked for no longer ends a run quietly. The signal classes are public, and honouring one thrown from inside a task's own phase left the run unfinished with nothing driving it and its device held for the life of the process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five tests main added or changed import the commissioning harness from `packages/node/test/node/`, where it lived before this branch moved it to `src/testing` and exported it as `@matter/node/testing`. The merge is otherwise clean; those imports are repointed at the export. This is the third main merge in a row to produce test-import breakage of this shape, and the incremental build does not see it — only `npm run build-clean` does, because the stale compiled output satisfies the resolver until it is cleared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ly cancel work still in flight A task's parameters are what the manager needs to re-drive its phases after a restart. Once the task has finished there is nothing left to re-drive, but the parameters stayed in storage for the life of the record — and for the group tasks those parameters are raw encryption keys (`AddNodeToGroup`'s `epochKey0`, `RotateGroupKey`'s `newEpochKey`). They are now dropped when a task reaches its final state, whatever that state is. Undoing a task replays the prior values it captured before writing, never its parameters, so nothing that could still happen needs them. Saying "remove this field" was not previously expressible. A write carries the state a task is about to adopt, and a field left undefined there means "unchanged" — deliberately, because an earlier attempt to express "unchanged" as undefined erased a task's place in the retirement order. Writes therefore gain a separate list of fields to remove, with three rules: a write may set a field or remove it but never both, since otherwise the order the two lists are applied in silently decides which wins; a stored record carries every field the task holds, so a write that does not mention a field preserves it; and it carries no field holding undefined, or a removed field reappears empty on the very next write. Retrying an undo had to stop asking the task definition anything. It used to rebuild the definition from the stored parameters purely to ask whether the task may be undone at all — a question already answered when the first undo was created. Rebuilding refuses parameters it cannot interpret, so with them gone it would have thrown an uninterpretable error at the caller. A retry now builds the replacement from the captured values alone. Cancelling a task that already finished no longer undoes it. Restoring the values a finished task found would overwrite whatever has legitimately happened since, so reversing a finished change is a new task the caller starts — `RemoveNodeFromGroup` reads what the device holds now and reference-counts entries other groups still share, which a replay of recorded values cannot. A finished task with no undo of its own is refused, one that already has an undo is still answered with it, and one that was cancelled with nothing to undo still answers "nothing to roll back". That refusal is asked twice, and the second time is the one that matters. A task can reach its own outcome after the cancel has been accepted: the phase loop consults the cancel only between phases, and the check that ends the loop consults nothing. A task held on the write that advances its phase index passes its last check and then records success anyway, so the decision is re-taken once its driver has stopped. Without that, a task that succeeded end to end was recorded as cancelled and its captured values were replayed onto the device. The stored task table gains a schema version, so a later build can refuse a table it would misread rather than half-reading it. It is stamped with every write rather than defaulted to the current version: stored state records what a write changed, so a version that always equals the reading build's own is never written and tells the next build nothing. A table from the future is not loaded at all and every write is refused, because presenting an empty table would let new work start against devices those unread records own. Also removes `TaskStatus.detail`, which nothing produced and nothing read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ade, and hand back the target of a run that finished mid-cancel Three defects in the change this fixes, all found in review. An upgrade did not actually remove the parameters it exists to remove. A write replaces only the records its own transaction names — deliberately, so a run this process never loaded is not erased by one that did — so a task that was already finished when this build first ran is never rewritten and keeps the parameters the previous build stored, including the raw epoch keys the group tasks take. The upgrade now walks the stored table once, gated on the schema version, and drops those. It reads the stored table rather than the loaded one because a record discarded at load still occupies storage and its parameters are just as durable, and it leaves unfinished runs alone because their parameters are what re-drives their phases after the restart. Refusing a cancel whose task finished while the cancel was being accepted left that task holding its target. Releasing a target normally happens once the driver stops, and it is skipped while a cancel owns the task's outcome — so nothing released it, and every later task for that target was refused for the life of the process. That is worse than the harm the refusal exists to prevent. The refusal now releases it first, as the equivalent window in `abandon` does. A task discarded before any of its state reached storage kept its parameters in memory. Its caller still holds a handle that reads from that record, so the removal a normal retirement would have carried has to happen there too. Two tests held a task in flight on a promise that never settles. Cancelling waits for the running phase to stop, and a phase parked that way cannot observe that it was asked to, so the wait could never end — the tests passed only when the cancel happened to be accepted before the phase started. Both now wait through the task context, which does observe it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… record The upgrade that removes a finished task's stored parameters took them out of storage but left them on the record already loaded into memory. A stored snapshot carries every field the record holds, so the next write of a migrated task — `retryRollback` recording the link to a fresh undo, for instance — put the parameters straight back, and the schema stamp the upgrade leaves behind meant it would never run again to remove them a second time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ot just its value The upgrade that strips a finished task's stored parameters tested whether the parameters held a value. A record written by the previous build materialises every field it knows, including the ones holding nothing, so a finished task that ran without parameters kept an empty `params` key — and since the upgrade stamps the schema version on its way out, it never ran again to take it off. Each record is now rebuilt through the same drop-and-snapshot path every ordinary write uses, rather than by editing the stored object. That is what keeps the two senses of "gone" from diverging: the record loaded into memory loses the field as well as the stored one, and the field's key goes rather than its value. It also normalises the other keys the previous build wrote empty, which is the same invariant one step further. The change that decides whether a record needs rewriting compares key sets and never serialises. Serialising drops keys that hold nothing — precisely the shape this pass exists to remove — so a serialised comparison reports that there is nothing to do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…te per-run identity The upgrade that removes a finished task's stored parameters skipped the records it was most important for. A record written before tasks had per-run identities is filed under its target rather than a run id, and the loader deliberately leaves it in storage rather than resuming it — so nothing will ever rewrite it. It was skipped because the upgrade rebuilt each record through the type that represents a run, which such a record cannot satisfy. For a group task its parameters are raw encryption keys, and stamping the schema version on the way out meant they would have stayed there for good. The upgrade now edits the stored record directly and removes the field's key. A record this process did load also has a copy in memory, and that copy loses the field too — otherwise the next write of it puts the field straight back. A record the loader discarded has no such copy, which is why editing storage is the operation that covers both. Rebuilding through the run type also normalised the other fields the previous build stored empty. That was never this pass's promise: the next ordinary write of a record does it, and reaching for it here is what hid the records above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rom what it would restore A run kept one field for two facts: the values it replaced, which an undo replays, and whether it reached the device at all, which every later run of the same target needs to know. Deciding supersession from that field meant the two could not both be right. They are now separate. A run records that it wrote when it writes, and keeps that for good. What it would restore is dropped as soon as nothing can replay it — when the run completed, because reversing a success is a new action the caller starts, and when it retired past its point of no return, because no undo of it can ever exist. For the group tasks those values are the epoch keys of the key set that was rotated away, so they no longer outlive the work. Two ways a later run wrongly blocked an earlier undo are gone with the same change. A run rejected before it reached a device, and a run that completed without changing anything — a removal whose node is already decommissioned, or one whose entries are all still referenced — no longer make an earlier run's recorded values look out of date, so the operator keeps the only way back from changes that are still on the device. Giving up on a failed undo no longer depends on unrelated work. Abandoning it was refused whenever an undo of some later run of the same target was running, even though that undo restores different values. Abandoning records what happens to an undo that already exists and never creates one, so it now asks only whether this undo has been replaced. This also removes the startup pass that dropped parameters from records an earlier build finished. Nothing outside development has ever written this table, so there are no such records, and the pass was machinery for a compatibility problem that does not exist. Its absence is what let the two facts above be stored separately rather than inferred from one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…play it A task run records the values it overwrote on a device so a rollback can put them back. Those values include key material. Until now they were kept on several paths where no rollback existed or could ever exist, so they stayed in storage for the life of the node with nothing able to read them again. One rule now decides this at every retirement: the recorded values survive only while a rollback that can replay them exists. That covers a rollback's own recorded values without naming them, because nothing undoes an undo. The two writes that end a rollback for good — it restored the device, or an operator gave up on it — additionally clear the values of the run it was undoing, in the same transaction that ends the rollback. Along the way this removes the ability to ask whether a run type can be reverted at a moment when the answer no longer matters: a run whose type cannot answer, or answers no, simply gets no rollback and therefore keeps nothing. Also deletes machinery that only ever served stores written by earlier builds. The package is unreleased and no such store exists: two load-time discards and their warning, an unused table snapshot and detach helper, and a rollback state this build cannot produce. A corrupt run identity in storage is now refused with a typed error instead of silently disabling identity allocation for the process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ice changed Cancelling a task answered with the undo it started, or with nothing. "Nothing" carried two opposite meanings: the run had not touched the device, or it had changed the device in a way that cannot be taken back. A caller could not tell those apart, and the documentation only described the first, so an operator who cancelled a half-finished key rotation was told nothing had happened. The second case is reachable. Whether a run can be reverted depends on how far it has progressed, and a cancel asks that question twice: once when it is accepted, and again after the run's driver has stopped. A driver already committing its next step passes the check that would have held it, so between those two questions a run can cross the point beyond which its type declines to be reverted. Cancelling now answers with what happened to the device: an undo is running, the device is untouched, or the device is changed and stays that way. Also validates a stored run identity against the same rule the rest of the code uses, so zero and negative values are refused rather than accepted by a weaker check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…top overstating a cancel's undo Two problems in the previous commit, both found in review. A refusal for a corrupt stored record put the whole record in its message. Task records carry raw group keys, so the message would have leaked key material into logs — the thing this branch exists to stop storing. It would also not have been a typed refusal at all: group task parameters hold a bigint, and serialising one throws before the intended error can be built. The message now names only the storage key. The cancel outcome said an undo was running, but the undo it reports may have finished, failed, or been abandoned long before: a run keeps a durable link to the rollback it once had. The value now says an undo exists, and the caller reads how far it got from the handle it is given. Covers a corrupt table by loading records whose identity is zero, negative, fractional, a string and absent, and asserts the refusal names the record without serialising it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Umbrella / integration branch for the Node Manager feature. Long-lived WIP PR — each phase merges into
node-managervia its own sub-PR; this PR is the CI backstop for the integrated branch againstmain. Do not merge until all phases land and it's de-WIP'd.Design doc:
docs/superpowers/specs/2026-06-14-node-manager-desired-state-design.mdWhat this builds
A controller-side layer holding the intended state of fabric nodes (certs/keys/IPK, group keys, bindings, ACLs, group membership), with offline-tolerant reconciliation and multi-step orchestration (e.g. group-key rotation). Modeled on the JointFabric Datastore cluster (0x0752) and generalized.
Phase checklist
@matter/node): ManagedItem/StatusEntry, ItemKind registry, capacity admission + typed errors, persistentDesiredStateBehavioronClientNode(Node Manager Phase 1: Tier-1 desired-state model #3946)@matter/node-managerpkg): triggers, settle delay, verify-barrier, priority ordering, capacity reads, concrete ItemKinds;RotateGroupKey/MoveNodeToGroup; changeset rollback; 2-node rotation harnessCarry-forwards tracked for Phase 2
limitonly, deriveusedfresh at admission.itemMapKeyseparator unescaped — escape/document before non-identifier keys.🤖 Generated with Claude Code