fix(framework): stop() waits for an operator's operation already running, and refuses new ones

.github/workflows/ci.yml

@@ -56,4 +56,4 @@ jobs:
       # closed"). These files run unforced so that class fails here, not in a
       # later PR's qualification.
       - name: Test unforced (work in flight at stop)
-        run: node --test dist/test/stop-with-work-in-flight.test.js dist/test/conversation-routing.test.js
+        run: node --test dist/test/stop-with-work-in-flight.test.js dist/test/stop-with-host-operations.test.js dist/test/conversation-routing.test.js

changelog.d/stop-waits-for-operator-operations.fixed.md

@@ -0,0 +1,81 @@
+- **`stop()` waits for an operator's operation that is already running, and
+  refuses new ones.** A quiesce, a resume, a
+  `host/command`, a store surgery, a branch switch, an undo or redo's config
+  materialization, a puppet tool call, an awareness sync, or an MCPL
+  connect, disconnect or restart admitted before `stop()` began now
+  finishes before the event gate, the modules, the MCPL connections and the
+  store go, unless a budget runs out first (see the operators' note below).
+  Before, such an operation kept running through the teardown. A resume
+  waiting on its maintenance pass or previews could go on to leave quiesce
+  while the host stopped, so the host booted unquiesced; a puppet call
+  could find the store closed partway; a surgery could still be running at
+  the final sync; a `host/command` could answer after its connection
+  closed; an API server branch switch requested during `stop()` moved the
+  branch, mounted its config after the store had closed and answered
+  success; and a connect could leave a connection that nothing closed.
+  - Once `stop()` has begun, `quiesce()`, `resume()`, `rollbackToMessage()`,
+    `suppressMessages()`, `switchBranch()`, `undoLastTurn()`, `redo()`,
+    `maintenanceTick()`, `puppetToolCall()`, `syncDiscordAwarenessMarkers()`,
+    `connectMcplServer()`, `disconnectMcplServer()` and `restartMcplServer()`
+    refuse with "<verb> refused: the framework is stopping" before they
+    read or change
+    anything, as a `host/command` already did. That includes a call made
+    from a trace emitted inside `stop()`'s own start, before its queue has
+    closed, and the MCPL listener now refuses a `host/command` from that
+    moment too.
+  - The awareness delivery that a surgery, a branch switch, a `host/command`
+    undo, or the API server's undo or redo starts
+    (`startDiscordAwarenessDelivery()`) runs like a server's drain: `stop()`
+    doesn't wait for it, and once `stop()` has begun none is started, since
+    the teardown would cut it off. The durable outbox keeps the marks, and
+    the next start delivers them.
+  - `stop()` also waits for what those operations start while it waits: the
+    config materialization that undo, redo and the two surgeries start
+    without waiting for it themselves. The framework now logs a failed
+    materialization.
+  - A quiesce stops waiting for its drain, or for an abandon's grace, once
+    `stop()` begins. `stop()` cancels the streams and waits for the event
+    loop and the operations above, puppet calls included. It doesn't wait
+    for any other turn that holds its token with no stream to cancel, so a
+    quiesce cut short can report `drained: false`. The host stays quiesced,
+    so it boots quiesced. A drain cut short logs that it stopped waiting,
+    emits no `host:quiesce` trace, and leaves an abandon it hadn't begun to
+    `stop()`, which cancels the streams. An abandon whose grace is cut short
+    reports its abandon as usual.
+  - A resume refuses at its start once `stop()` has begun, whether or not
+    the host is quiesced, and checks again after its maintenance and preview
+    waits, before it leaves quiesce, so a quiesced host stays quiesced. A
+    resume past that point finishes, its durable clear included, and its log
+    says that the parked wakes it released won't run.
+  - A host's trace listener can call `stop()` from inside an operation, such
+    as on its completion trace, and `stop()` still waits for that operation.
+    A multi-turn `host/command` undo stopped that way still undoes every
+    turn it was asked for.
+  - `undoLastTurn()` and `redo()` refuse with `OperatorActionError`
+    (`agent-busy`) while a surgery or a branch switch holds the store,
+    instead of moving the branch beneath it.
+  - A `stop()` called from a trace emitted inside `stop()`'s own start, as
+    the push coalescer's suspension emits for a render in flight, gets the
+    attempt already running. Before, it started a second teardown beside
+    the first, which then failed at its final sync, and every later
+    `stop()` rejected with "Store has been closed" for a framework that had
+    stopped.
+  - While the host stops, an MCPL awareness barrier that fails no longer
+    recycles the connections, which `stop()` is closing. The framework
+    releases it as it releases a completed one, so nothing waits on a
+    rejected barrier, and unless the host is quiesced, what it held back
+    reaches the listeners' stopping refusal.
+  - **For operators:** `stop()` waits as long as an operation's own work
+    runs. An awareness drain finishes its deliveries, each within its own
+    deadline, and a puppet call waits for its tool, so a tool on an MCPL
+    server that never answers holds `stop()` until the request times out
+    (60 s by default, never with `requestTimeoutMs: 0`).
+    `stop(teardownTimeoutMs)` gives this wait its own budget, shared by all
+    its rounds, and then gives the modules' and MCPL's teardown the budget
+    again, so a budgeted `stop()` can take up to twice the budget, beside
+    the stream and event-loop waits that no budget bounds. An operation
+    that outlasts the budget fails `stop()` with a `ShutdownTimeoutError`
+    that names it, on its own or inside an `AggregateError` when other
+    cleanup failed too. `stop()` still tears down the rest, while that
+    operation may still run, and keeps the store open, so `stop()` can be
+    called again.

changelog.d/switch-branch.added.md

@@ -0,0 +1,16 @@
+- `AgentFramework.switchBranch(name)` switches the store to an existing
+  branch, starts the delivery of the Discord awareness marks the move
+  changes, re-materializes the config mount, and then resolves. It doesn't
+  wait for Discord, as the surgeries don't. If the mount fails, it rejects
+  with the branch already moved. The branch moves under every agent
+  sharing the store, so, as for `rollbackToMessage()`, every agent must be
+  idle: during a live turn it refuses with `OperatorActionError`
+  (`agent-busy`), and no turn starts until it's done. Once `stop()` has
+  begun it refuses, and `stop()` waits for one already running.
+  - The API server's branch switch used to run these steps itself, on the
+    store, and now calls `switchBranch()`. So it refuses during a live turn
+    instead of moving the branch beneath it. Its current branch follows the
+    store from the moment it moves, and clients hear of a move even when a
+    later step fails, but not of a refused switch. A branch create with
+    `switchTo` creates the branch first, so during a live turn it creates
+    it and answers the switch's refusal.

src/api/server.ts

@@ -537,34 +537,42 @@ export class ApiServer {
     }
 
     const store = this.framework.getStore();
-    const previousBranch = this.currentBranch;
-
-    store.switchBranch(params.name);
-    this.currentBranch = params.name;
-
-    // Reversible Discord awareness markers are part of the branch projection.
-    // The switch is acknowledged once it has landed: delivery of the marks it
-    // adds and removes starts behind the MCPL data-plane gate, which holds
-    // channel traffic that arrives after it until they are attempted, or
-    // deferred while their server is disconnected, and a failure is an ops
-    // alert rather than a failed switch. (It used to await the awareness
-    // sync, which after a large rollback took the drain's whole length.)
-    this.framework.startDiscordAwarenessDelivery('branch switch');
-
-    // Materialize config files from the new branch
-    const ws = this.framework.getModule('workspace');
-    if (ws && 'materializeMount' in ws) {
-      await (ws as any).materializeMount('_config');
+    const from = store.isClosed() ? this.currentBranch : store.currentBranch().name;
+
+    // The framework switches the store, starts the delivery of its Discord
+    // awareness marks behind the data-plane gate, and re-materializes the
+    // config mount. It refuses during a live turn or once stop() has begun,
+    // and stop() waits for one already running.
+    // The store has moved when the call returns, before its promise settles
+    // (a refusal moves nothing). The branch read then is this switch's
+    // destination: another switch can move the store before this one's
+    // awaited steps end, so the event names what was read here.
+    const switching = this.framework.switchBranch(params.name);
+    void switching.catch(() => {}); // awaited below; handled even if the next line throws
+    this.followStoreBranch();
+    const to = this.currentBranch;
+    const moved = to !== from;
+    let switched = false;
+    try {
+      await switching;
+      switched = true;
+    } finally {
+      // A later step can fail after the store has moved: clients still hear
+      // where it went. A refusal moved nothing, so there's nothing to tell.
+      if (switched || moved) {
+        this.broadcast('branch:switched', { from, to });
+      }
     }
 
-    this.broadcast('branch:switched', {
-      from: previousBranch,
-      to: params.name,
-    });
-
     return { switched: true };
   }
 
+  /** Take the current branch from the store, while it's open. */
+  private followStoreBranch(): void {
+    const store = this.framework.getStore();
+    if (!store.isClosed()) this.currentBranch = store.currentBranch().name;
+  }
+
   private async cmdBranchCurrent(): Promise<{ name: string }> {
     return { name: this.currentBranch };
   }