Summary
Follow-up from PR #443 review (AgentWorkforce/relay#1794 fix). The bind path's capacity accounting was hardened there, but the same reserve→act→compensate pattern has pre-existing races in several callers that this PR deliberately did not expand into. Filing so the findings don't die in review comments.
Findings (from fresh-eyes review, verified against source)
- Reservation-before-move race:
reserveNodeAgentSlot runs before the location/provider move commits. Between reservation and commit, a concurrent bind/register on the same node can observe inflated used counts, or a crash window can leave a reserved slot with no committed binding (leak until reconcile).
- Compensating-release failure: when the move fails after reservation, the compensating
releaseNodeAgentSlot can itself fail (D1 transient), leaving a permanently inflated slot count. There is no retry/reconcile path for stranded reservations.
- Bind TOCTOU:
bindAgentToNode reads bindings/capacity, then acts — a concurrent bind of the same agent to a different node between the read and the atomic write can produce two live via_node locations or a stolen-active-binding edge not covered by the conflict guard's snapshot.
- Same shape elsewhere:
registerAgentViaNode, recoverAgentViaNode, and the inventory-reconcile path all use the same reserve/act/compensate structure and share these windows.
Why separate
PR #443 fixed the provider-routing defect that prevented spawned subscription agents from waking (verified live on 8.11.3). These items are structural concerns in the capacity model predating that fix — they warrant a dedicated design pass (e.g. reservation records reconciled by the node-reaper, single-writer serialization per node, or folding reservation into the same atomic unit as the move) rather than incremental patches.
Context
Summary
Follow-up from PR #443 review (AgentWorkforce/relay#1794 fix). The bind path's capacity accounting was hardened there, but the same reserve→act→compensate pattern has pre-existing races in several callers that this PR deliberately did not expand into. Filing so the findings don't die in review comments.
Findings (from fresh-eyes review, verified against source)
reserveNodeAgentSlotruns before the location/provider move commits. Between reservation and commit, a concurrent bind/register on the same node can observe inflatedusedcounts, or a crash window can leave a reserved slot with no committed binding (leak until reconcile).releaseNodeAgentSlotcan itself fail (D1 transient), leaving a permanently inflated slot count. There is no retry/reconcile path for stranded reservations.bindAgentToNodereads bindings/capacity, then acts — a concurrent bind of the same agent to a different node between the read and the atomic write can produce two livevia_nodelocations or a stolen-active-binding edge not covered by the conflict guard's snapshot.registerAgentViaNode,recoverAgentViaNode, and the inventory-reconcile path all use the same reserve/act/compensate structure and share these windows.Why separate
PR #443 fixed the provider-routing defect that prevented spawned subscription agents from waking (verified live on 8.11.3). These items are structural concerns in the capacity model predating that fix — they warrant a dedicated design pass (e.g. reservation records reconciled by the node-reaper, single-writer serialization per node, or folding reservation into the same atomic unit as the move) rather than incremental patches.
Context
packages/engine/src/engine/node.ts(bindAgentToNode,runAtomicWrites,reserveNodeAgentSlot,releaseNodeAgentSlot)