fix(mcp): the modern engine's failed requests, and a connect in flight, in #506's words

changelog.d/mcpl-agent-words.changed.md

@@ -2,7 +2,7 @@
   - **A failed MCPL request:**
     - A connection that closes with a call pending reads `The connection to the "<server>" server closed before it answered, so whether the request took effect is unknown.` It then says whether this host is reconnecting, where it read `MCPL server "<server>" disconnected unexpectedly (code=…, signal=…, reason=…) while awaiting <method> (id=N)`.
     - The other texts change the same way: a close by this host, a request on a closed connection or one the transport couldn't write, a server's error answer (in the server's own words, or, with none, saying the outcome is unknown) and a timeout.
-    - The log gains a line for a close with requests pending, with the close's code, signal and reason, whether this host closed it, and the requests left unanswered; one for a server's error answer, with its method, id and code; one for a timeout, with its method and id; and one for a request the transport couldn't write, with its method, id and the transport's words.
+    - The log gains a line for a close with requests pending, with the close's code, signal and reason, whether this host closed it, and the requests left unanswered; one for a server's error answer, with its method, id and code, and its message, whatever JSON value it is, with any credential the server's configuration holds replaced, where the caller's text keeps the message unredacted; one for a timeout, with its method and id; and one for a request the transport couldn't write, with its method, id and the transport's words.
   - **The framework's MCPL tool calls:**
     - A call to a server missing from the registry, or to a tool its policy denies, says it didn't run and whether anything can change that, in the same words for the model's call and a script's. One made while this host is connecting to the server, in a runtime connect or a restart's, says so and to try again in a moment.
     - An error result with no text, or only whitespace, reads as one whose effect is unknown, where it read `Tool call failed`, or the blank text.

changelog.d/mcpl-connect-in-flight.fixed.md

@@ -1 +1 @@
-- A runtime connect, or a restart of another server into an id whose legacy connect is still in its handshake, is now refused before it changes anything, with `MCPL server "<id>" has a connect in flight; try again once it settles`. While a modern connect is in flight, the id reads as already registered. Before, a second connect went ahead beside a legacy one, and whichever settled last replaced the other in the registry without closing it. A disconnect of the id, in either engine, now ends the connect in flight and removes whatever it registered. Before, the disconnect removed the server's config and tool prefix, and the connect then registered the server without them, where no call reached it and only another disconnect cleared it.
+- A runtime connect, or a restart of another server into an id whose connect is still in flight, in either engine, is now refused before it changes anything, with `MCPL server "<id>" has a connect in flight; try again once it settles`, until that connect lets go of the id, just before it lists its tools. Before, a second connect went ahead beside a legacy one, and whichever settled last replaced the other in the registry without closing it. A disconnect of the id now ends the connect in flight and removes whatever it registered. Before, the disconnect removed the server's config and tool prefix, and the connect then registered the server without them, where no call reached it and only another disconnect cleared it.

changelog.d/modern-mcp-client.added.md

@@ -30,7 +30,12 @@
     `complete` result is a final answer: a call that ends after an
     `input_required` answer, or after a result type this revision doesn't
     define, is `no-response`. A complete result that can't be used is a
-    plain error.
+    plain error. The message an agent reads is the one an MCPL server's
+    failure gives where the two share an outcome, and has its shape where
+    they don't: what happened, whether anything took effect, and what works
+    from there. The method, the tool, the JSON-RPC code and the HTTP status
+    go to the host's log. A tool call made while the server's connect is
+    still under way reads as one made while an MCPL server connects.
   - **Lifetime:** reconnects use the legacy backoff settings. A tool-list
     change subscription is kept open, reopened if refused or lost, and its
     changes go through ordinary admission, so they park under quiesce like

changelog.d/redact-recorded-mcp-credentials.fixed.md

@@ -1 +1 @@
-- Server and transport text the framework records has the server's configured credentials replaced, in both engines: connection traces, console lines, stderr traces, the last of a server's stderr that a failed handshake keeps (in its trace and console line), the ops alert (and so `logs/failures.log` and the webhook), an MCP tool call that fails without an answer, as its caller and the model get it and in the `tool:failed` trace's message and stack. The credentials are those the configuration holds and those its dials sent: from the url as each dial sends it (a WebSocket dial sets the token in the query, which writes the whole query again, form-encoded, and an access provider's token can be in it), credential-like path segments and query names, credential-shaped or credential-named query values (each as written and decoded), every non-empty parameter as its whole `name=value`, and a WebSocket url's username and password, its userinfo as one unit before its `@` (the username alone when there's no password), with no length floor, so a short password goes with its username and a short username alone goes too, and the Basic credential a WebSocket client sends for it; the static `token`, and each token an access provider handed a dial (the newest sixteen), raw, as JSON encoders write it (with `JSON.stringify`'s escapes, and with its slashes escaped too, as some encoders write them), and as a url carries it, percent- or form-encoded with either case of hex; and a stdio server's `env` values whose names have a part ending in a credential word, singular or plural, with any digits after it (`GITHUB_TOKEN`, `API_KEYS`, `PGPASSWORD`, `DB_PASS`, `AUTHORIZATION`, `SESSION_COOKIE`, `TOKEN2`), raw, as JSON encoders write them, and percent-encoded with either case of hex. A stdio server's command-line arguments are not derived. A string replaced on its own has at least eight characters, since a shorter one can't be told from ordinary text, and is replaced wherever it occurs. Every non-empty query parameter is also replaced as its whole `name=value`, only where no letter or digit adjoins the pair (so `v=2` leaves `dev=2` alone); a short value goes only that way. A connection's event records use a redactor built once for it, and built again once a dial has sent something new. An operation's records replace the credentials of the configurations present when it began and of those present when each record is made, so a server removed, or replaced by a restart, while the operation waits is still covered, and so is one connected meanwhile. Every place a credential matches is replaced, places that overlap included, and text that may come from any server is matched against every server's strings at once, so two servers' secrets that overlap are both replaced whole. Over HTTP, which only the modern engine speaks, a 401 or 403's response body is left out of the request error's data; where the HTTP client builds the error's message from the body, as it does for most statuses, the message is redacted like any record. Credentials neither the configuration holds nor a dial sent are outside this, and so are a stdio server's command-line arguments. JSON-RPC answers, and the error a host's own connect raises, pass as they are.
+- Server and transport text the framework records has the server's configured credentials replaced, in both engines: connection traces, console lines, stderr traces, the last of a server's stderr that a failed handshake keeps (in its trace and console line), the ops alert (and so `logs/failures.log` and the webhook), an MCP tool call that fails without an answer, as its caller and the model get it and in the `tool:failed` trace's message and stack. The credentials are those the configuration holds and those its dials sent: from the url as each dial sends it (a WebSocket dial sets the token in the query, which writes the whole query again, form-encoded, and an access provider's token can be in it), credential-like path segments and query names, credential-shaped or credential-named query values (each as written and decoded), every non-empty parameter as its whole `name=value`, and a WebSocket url's username and password, its userinfo as one unit before its `@` (the username alone when there's no password), with no length floor, so a short password goes with its username and a short username alone goes too, and the Basic credential a WebSocket client sends for it; the static `token`, and each token an access provider handed a dial (the newest sixteen), raw, and as a url carries it, percent- or form-encoded with either case of hex; and a stdio server's `env` values whose names have a part ending in a credential word, singular or plural, with any digits after it (`GITHUB_TOKEN`, `API_KEYS`, `PGPASSWORD`, `DB_PASS`, `AUTHORIZATION`, `SESSION_COOKIE`, `TOKEN2`), raw, and percent-encoded with either case of hex. Each of these strings, from every source, the `name=value` pairs and userinfo units too, is also replaced as JSON encoders write it, with `JSON.stringify`'s escapes, and with its slashes escaped too, as some encoders write them. A stdio server's command-line arguments are not derived. A string replaced on its own has at least eight characters, since a shorter one can't be told from ordinary text, and is replaced wherever it occurs. Every non-empty query parameter is also replaced as its whole `name=value`, only where no letter or digit adjoins the pair (so `v=2` leaves `dev=2` alone; a JSON escape before it, such as `\n`, isn't one); a short value goes only that way. A connection's event records use a redactor built once for it, and built again once a dial has sent something new. An operation's records replace the credentials of the configurations present when it began and of those present when each record is made, so a server removed, or replaced by a restart, while the operation waits is still covered, and so is one connected meanwhile. Every place a credential matches is replaced, places that overlap included, and text that may come from any server is matched against every server's strings at once, so two servers' secrets that overlap are both replaced whole. Over HTTP, which only the modern engine speaks, a 401 or 403's response body is left out of the request error's data; where the HTTP client builds the error's message from the body, as it does for most statuses, the message is redacted like any record. Credentials neither the configuration holds nor a dial sent are outside this, and so are a stdio server's command-line arguments. JSON-RPC answers, and the error a host's own connect raises, pass as they are.

src/framework.ts

@@ -147,17 +147,20 @@ import { stampThinkingTokenEstimates } from './thinking-token-stamp.js';
  *  closed for good, one removed, or one whose connect failed without
  *  reconnect, which this host doesn't bring back on its own. A server that's
  *  reconnecting stays registered, as does the stub a failed connect with
- *  reconnect leaves, and its closed connection answers the call instead. One
- *  this host is connecting to isn't registered yet, and
- *  mcplServerConnectingText answers for it. */
+ *  reconnect leaves, and its closed connection answers the call instead.
+ *  mcplServerConnectingText answers for one this host is connecting to: a
+ *  legacy server, which isn't registered yet, or a modern connection, which
+ *  is, until its first session is up. */
 function mcplServerGoneText(serverId: string, toolName: string): string {
   return `The "${serverId}" server that provides ${toolName} isn't connected, and this host won't reconnect to it on its own, ` +
     "so the call didn't run. If you need it, ask whoever runs this host.";
 }
 
 /** A tool call refused while this host connects to its server, in a
  *  runtime connect or a restart's: the server's prefix routes calls to it
- *  before the server is registered, which it is once its connect is done. */
+ *  before a legacy server is registered, which it is once its connect is
+ *  done, and to a modern connection, registered before it starts, until its
+ *  first session is up (modernStillConnecting). */
 function mcplServerConnectingText(serverId: string, toolName: string): string {
   return `This host is connecting to the "${serverId}" server that provides ${toolName}, so the call didn't run. ` +
     'Try again in a moment.';
@@ -171,12 +174,11 @@ function mcplToolDeniedText(serverId: string, toolName: string): string {
     "and calling it again won't change that. If you need it, ask whoever runs this host.";
 }
 
-/** A runtime connect or restart refused because a legacy connect of the
- *  server's id is still in its handshake, before its server is registered
- *  (mcpServerIdTaken). A modern connect registers its connection first, so an
- *  id it holds reads as already registered. A disconnect isn't refused: it
- *  ends the connect in flight, in either engine. Whoever asked reads it: an
- *  operator, or an agent through a host's tool that passes it on. */
+/** A runtime connect or restart refused because a connect of the server's id
+ *  is in flight, in either engine, whether or not it has registered its
+ *  server yet (mcpServerIdTaken). A disconnect isn't refused: it ends that
+ *  connect. Whoever asked reads it: an operator, or an agent through a host's
+ *  tool that passes it on. */
 function mcplConnectInFlightText(serverId: string): string {
   return `MCPL server "${serverId}" has a connect in flight; try again once it settles`;
 }
@@ -18307,7 +18309,7 @@ export class AgentFramework {
     }
     const modern = this.modernServers().get(serverId) ?? null;
     const server = modern ? null : this.mcplServerRegistry.getServer(serverId);
-    if (!modern && !server) {
+    if ((!modern && !server) || this.modernStillConnecting(modern)) {
       return { success: false, error: this.mcplServerAbsentText(serverId, call.name), isError: true };
     }
     const config = this.mcplServerConfigs.get(serverId);
@@ -22948,14 +22950,19 @@ export class AgentFramework {
 
     const collision = this.mcplPrefixCollision(config, undefined, claim);
     if (collision) throw new Error(collision);
+    // The id is refused here too, before the trees are read, so a connect
+    // refused for its id reads nothing and reports only its refusal. The
+    // connect asks again as it takes the id, with nothing awaited between.
+    const taken = this.mcpServerIdTaken(config.id, claim);
+    if (taken) throw new Error(taken);
 
     // The server's restored host-managed trees are read before the server is
     // added, so a read that fails leaves it unconnected; MCPL itself stays
     // up, even when this connect brought it up. A feature set with no
     // restored tree, as after a disconnect removed it, is read when it
-    // registers. A server already connected is refused by addServer below,
-    // so its trees are left as they are.
-    if (!this.mcplServerRegistry?.getServer(config.id)) this.checkpointManager?.loadHostStates(config.id);
+    // registers. A server already registered was refused just above, so the
+    // read never reaches a registered server's trees.
+    this.checkpointManager?.loadHostStates(config.id);
     const oldToolNames = new Set(this.mcplTools.map(t => t.name));
     const connection = await this.connectMcplServerInternal(config, false, claim);
     // A stop() that began during the connect's setup or its tool listing
@@ -23235,11 +23242,14 @@ export class AgentFramework {
     }
     // A disconnect of the id began during the restart, and has finished.
     if (claim?.endedBy) return claim.endedBy;
+    // A connect in flight, in either engine: a legacy one registers its
+    // server once its handshake is done, and a modern one registers its
+    // connection before it starts. Whatever the id has registered meanwhile
+    // is that connect's.
+    if (op) return `${refused}${mcplConnectInFlightText(id)}`;
     const registered = this.modernServers().get(id) ?? this.mcplServerRegistry?.getServer(id);
-    // A connect in flight that hasn't registered its server yet.
-    if (op && !registered) return `${refused}${mcplConnectInFlightText(id)}`;
-    if (op || registered) {
-      const idle = !op && registered && !registered.isConnected && !registered.willReconnect;
+    if (registered) {
+      const idle = !registered.isConnected && !registered.willReconnect;
       return `${refused}MCPL server "${id}" is already registered${idle ? ' (not connected and not reconnecting: disconnect it first)' : ''}`;
     }
     return null;
@@ -24427,18 +24437,27 @@ export class AgentFramework {
     }
   }
 
-  /** The refusal of a tool call whose server isn't registered, for the
-   *  model's call and a script's alike: "connecting" while a connect of its
-   *  id is in flight (mcpServerOps) or a restart holds the id, and "gone"
-   *  otherwise. A legacy connect registers its server once its handshake is
-   *  done; a modern connect registers its connection first, so a call during
-   *  it reads that connection's own refusal. */
+  /** The refusal of a tool call whose server isn't one to call yet, for the
+   *  model's call, a script's and an in-process one alike: none registered,
+   *  or a modern connection whose connect is still under way
+   *  (modernStillConnecting). It says "connecting" while a connect of its id
+   *  is in flight (mcpServerOps) or a restart holds the id, and "gone"
+   *  otherwise. */
   private mcplServerAbsentText(serverId: string, toolName: string): string {
     return this.mcpServerOps?.get(serverId)?.kind === 'connect' || this.restartClaims().ids.has(serverId)
       ? mcplServerConnectingText(serverId, toolName)
       : mcplServerGoneText(serverId, toolName);
   }
 
+  /** A modern connection a connect still under way registered: it registers
+   *  before it starts, so until its first session is up it's no server to
+   *  call, and a call reads the "connecting" text, as one during a legacy
+   *  connect's handshake does. Once that session is up, a call goes to it,
+   *  even while the connect lists its tools. */
+  private modernStillConnecting(modern: ModernMcpConnection | null): boolean {
+    return modern !== null && !modern.isConnected && this.mcpServerOps?.get(modern.id)?.kind === 'connect';
+  }
+
   /**
    * Dispatch a tool call to an MCPL server.
    * Strips the configured toolPrefix and routes to the server.
@@ -24448,7 +24467,7 @@ export class AgentFramework {
     const modern = this.modernServers().get(serverId) ?? null;
     const server = modern ? null : this.mcplServerRegistry!.getServer(serverId);
 
-    if (!modern && !server) {
+    if ((!modern && !server) || this.modernStillConnecting(modern)) {
       // RFC-007: refused before executing — no lifecycle events.
       this.toolLifecycleEmitter?.refuse(agentName, call.id);
       this.pushEvent({

src/mcpl/log-value.ts

@@ -45,6 +45,16 @@ export function logValue(value: unknown): string {
   return printed(textOf(value), FIELD);
 }
 
+/**
+ * `logValue` of a value a server chose whose text may hold a credential:
+ * its text, as logValue reads it (a JSON value as its JSON text), with
+ * `redact` applied, then printed as logValue prints a field. A value whose
+ * text holds nothing `redact` replaces prints exactly as logValue prints it.
+ */
+export function logValueRedacted(value: unknown, redact: (text: string) => string): string {
+  return printed(redact(textOf(value)), FIELD);
+}
+
 /**
  * The host's own message, as the free text that ends a log line after its
  * fields: `reason=coalesce-invalid field=payload.content: "content must be an