fix(mcpl): a server closed for good leaves the registry, and a failed runtime connect settles its gate

changelog.d/mcpl-close-down-server.fixed.md

@@ -0,0 +1,36 @@
+- A `reconnect: true` MCPL server that the framework closes for good now
+  leaves the registry even when it's down at the time, so a later connect of
+  its id isn't refused as already registered. The framework closes a server
+  for good when a runtime `connectMcplServer` or `restartMcplServer` fails
+  its Discord awareness accounting or its policy registration, and when the
+  server refuses the host's policy with the `"close"` fallback. The registry
+  dropped a closed server only when its connection reported the close, and a
+  connection that's down between retries reports none. So the stub a failed
+  first connect leaves stayed registered after such a close, and a later
+  `connectMcplServer` of its id failed as already registered until
+  `disconnectMcplServer` or `restartMcplServer` dropped it. A close that
+  fails still keeps the server registered, so a later disconnect, restart or
+  shutdown can close it again, and the failed connect now logs it.
+- A runtime `connectMcplServer` or `restartMcplServer` that fails after
+  pausing the other MCPL servers' data planes no longer leaves their data
+  held. The connect paused them behind a gate that it settled only when it
+  succeeded, so their pushes, incoming messages and inference requests
+  waited until some other gate completed.
+  - When its Discord awareness accounting fails, it now fails that gate, as
+    a failed reconnect, tools-list reconciliation,
+    `syncDiscordAwarenessMarkers()` or `resume()` already does: each running
+    server is recycled, and its reconnect's own gate holds its data until
+    that reconnect's accounting succeeds. A server without `reconnect: true`
+    is closed for good there, as on those paths. While the host is stopping,
+    the gate is released instead.
+  - When its policy registration throws after accounting succeeded, it now
+    closes the new server for good, where it stayed connected and half
+    registered, and completes the gate, with no recycle.
+- `McplServerConnection.ready()` and `readyControlPlane()` now do nothing once
+  the connection is closed for good: closed, with reconnect off, as after
+  `close()` or a drop with reconnect off. What it held stays held. Any gate's
+  completion used to pass the part of it that needs no grant to listeners,
+  as for a connection whose close failed and so stayed registered. While the
+  host stops, a connection closed for good, such as one being closed, no
+  longer passes what it held to the stopping refusal, whose answers can't be
+  sent on a closed connection.

src/framework.ts

@@ -17520,7 +17520,8 @@ export class AgentFramework {
     // would start reconnects only for stop() to close them again. Release the
     // barrier instead, as a completed one is released: nothing waits on a
     // rejected barrier, and unless the host is quiesced, what the pause held
-    // back reaches the listeners' stopping refusal.
+    // back reaches the listeners' stopping refusal, except what a connection
+    // closed for good held, which stays held.
     if (this.queue?.isClosed) {
       this.completeMcplDataPlaneGate(barrier);
       return;
@@ -17535,6 +17536,25 @@ export class AgentFramework {
     }));
   }
 
+  /**
+   * Close a runtime connect's new connection for good, through the registry,
+   * after a step of that connect failed. A close that fails keeps the entry,
+   * so a later disconnect, restart or shutdown can close it again, and is
+   * logged here rather than replacing the failure the connect reports.
+   */
+  private async closeFailedRuntimeConnect(
+    connection: McplServerConnection,
+    failedStep: string,
+  ): Promise<void> {
+    await this.mcplServerRegistry!.closeServer(connection.id, connection).catch((closeError) => {
+      console.error(
+        `[mcpl] could not close MCPL server "${connection.id}" after its ${failedStep} failed; ` +
+          'it stays registered until a disconnect, restart or shutdown closes it:',
+        closeError instanceof Error ? closeError.message : closeError,
+      );
+    });
+  }
+
   /**
    * Connect a single MCPL server: register routing entries, open the
    * connection, wire events, and initialize feature sets / scopes /
@@ -17627,21 +17647,50 @@ export class AgentFramework {
       // the runtime connect or restart that asked for it. (A startup connect
       // defers awareness and returns before this gate; a startup that fails
       // ends in create()'s release, which closes its servers.)
+      // That close goes through the registry, which drops the entry itself
+      // rather than waiting for a 'close' event: close() emits none for a
+      // connection that's down between retries, such as a stub waiting for
+      // its next retry, and a stub left registered would refuse a later
+      // connect of its id as already registered.
+      // Then fail the gate, as every other runtime gate does when its
+      // accounting fails. Installing it paused every registered server's
+      // data plane, and left standing it would hold their data until some
+      // other gate completed. While serving, failMcplDataPlaneGate recycles
+      // those servers, so a server without `reconnect: true` stays closed;
+      // while stopping, it releases the gate. It runs after the close, which
+      // has taken this connection out of the registry unless the close
+      // failed, which closeFailedRuntimeConnect logs.
       // Ordering matters: awareness accounting settles BEFORE the §5.3
       // policy round-trip below, so an accounting failure aborts with the
       // grant still empty and every buffered event still behind the gate —
       // a failed connect never reports a released data plane. (The reverse
       // order let a push arrive during the policy await and get flushed by
       // teardown.)
-      await connection.close().catch(() => {});
+      await this.closeFailedRuntimeConnect(connection, 'awareness accounting');
+      await this.failMcplDataPlaneGate(awarenessBarrier, `connect of MCPL server "${config.id}"`, error);
       throw error;
     }
 
     // §5.3 initial policy handshake — after accounting, before the gate
     // completes: the grant is established (or knowingly left empty) before
     // ANY buffered traffic (control or data) flows. complete() then
     // ready()s both planes through the admission gate.
-    await this.registerMcplServerFeatures(config, connection);
+    try {
+      await this.registerMcplServerFeatures(config, connection);
+    } catch (error) {
+      // A policy registration that throws would leave the new connection
+      // live and half registered, and the gate standing over every other
+      // server's data. So close the connection for good through the
+      // registry, as the catch above does, then complete the gate rather
+      // than fail it: accounting succeeded, so nothing calls for a recycle.
+      // A close that fails keeps the connection registered, but a connection
+      // closed for good readies nothing, so this completion, and any later
+      // one, leaves it paused.
+      // Every exit of a runtime connect settles the gate it installed.
+      await this.closeFailedRuntimeConnect(connection, 'policy registration');
+      this.completeMcplDataPlaneGate(awarenessBarrier);
+      throw error;
+    }
     this.completeMcplDataPlaneGate(awarenessBarrier);
 
     this.emitTrace({ type: 'module:added', moduleName: `mcpl:${config.id}` });
@@ -17797,7 +17846,7 @@ export class AgentFramework {
             ...AgentFramework.policyRefusalFields(receipt),
           });
           if (fallback === 'close') {
-            await connection.close();
+            await this.mcplServerRegistry!.closeServer(connection.id, connection);
             return;
           }
           connection.establishGrant(CapabilityGrant.empty());
@@ -17915,7 +17964,7 @@ export class AgentFramework {
           ...AgentFramework.policyRefusalFields(receipt),
         });
         if (fallback === 'close') {
-          await connection.close();
+          await this.mcplServerRegistry!.closeServer(connection.id, connection);
           return;
         }
         // mcp-only (§3.2): grant stays empty, MCPL privileged surface stays

src/mcpl/server-connection.ts

@@ -315,6 +315,13 @@ export class McplServerConnection extends EventEmitter {
     return this.reconnectEnabled;
   }
 
+  /** Closed with no reconnect to come: after close(), whether or not its
+   *  transport close succeeded, or a drop with reconnect off. A connection
+   *  between retries keeps reconnectEnabled, so it isn't closed for good. */
+  private get closedForGood(): boolean {
+    return this.closed && !this.reconnectEnabled;
+  }
+
   /**
    * Private constructor. Use `McplServerConnection.connect()` instead.
    */
@@ -342,8 +349,17 @@ export class McplServerConnection extends EventEmitter {
    * Mark the connection as ready — flushes any events that arrived between
    * construction (when setupMessageRouting starts emitting) and now (when
    * the caller has attached listeners via wireMcplEvents).
+   *
+   * Does nothing once the connection is closed for good. Its policy was reset
+   * when it closed, so what it held that needs a grant would be refused, and a
+   * closed connection sends no response, so nothing it held could be answered.
+   * Only what needs no grant would pass, to listeners of a server that won't
+   * be served again. A connection between retries isn't closed for good: what
+   * it held, such as a reconnect stub's connect-failed, still reaches its
+   * listeners.
    */
   ready(): void {
+    if (this.closedForGood) return;
     this.controlPlaneReady = true;
     this.dataPlaneReady = true;
     const pending = this.bufferedEvents;
@@ -375,9 +391,11 @@ export class McplServerConnection extends EventEmitter {
    * Release only server-establishment and lifecycle/control traffic. Responses
    * to host requests are never event-buffered, so tools/list and tools/call
    * responses remain live as well. Inference-bearing requests stay queued in
-   * their original relative order until {@link ready} is called.
+   * their original relative order until {@link ready} is called. Like
+   * {@link ready}, it does nothing once the connection is closed for good.
    */
   readyControlPlane(): void {
+    if (this.closedForGood) return;
     this.controlPlaneReady = true;
     const pending = this.bufferedEvents;
     this.bufferedEvents = [];
@@ -955,7 +973,8 @@ export class McplServerConnection extends EventEmitter {
     const attempt = Promise.resolve().then(async () => {
       // An already opened handshake owns a transport too. It hands that
       // transport back without admission once reconnect has been disabled.
-      // The registry bounds this wait; retry reuses this real close attempt.
+      // Only the registry's closeAll bounds its caller's wait for this; retry
+      // reuses this real close attempt.
       if (this.reconnectPromise) await this.reconnectPromise;
       if (this.transport) await this.transport.close();
       this.closeIncomplete = false;

src/mcpl/server-registry.ts

@@ -148,7 +148,16 @@ export class McplServerRegistry {
     })), timeoutMs);
   }
 
-  private closeServer(id: string, connection: McplServerConnection): Promise<void> {
+  /**
+   * Close a server for good and drop its entry, if this connection is still
+   * the one registered under `id`. The entry goes when close() resolves,
+   * whatever the connection reports: close() emits no 'close' for a
+   * connection that's down between retries, such as a reconnect stub waiting
+   * for its next retry, so the 'close' listener alone would leave it
+   * registered. A close that fails keeps the entry and rejects; a close
+   * already under way is shared.
+   */
+  closeServer(id: string, connection: McplServerConnection): Promise<void> {
     const existing = this.closing.get(connection);
     if (existing) return existing;
     const attempt = Promise.resolve().then(() => connection.close()).then(() => {