fix: address Copilot review on the _meta widening

- Widen the three `_meta` annotations the bulk pass missed, all of them
  return types rather than identifier declarations: `mergeMeta`,
  `aggregateListParams`, and the `tools/call` params object. The second was
  unsound in a way callers could see — it advertised
  `{ _meta: Record<string, string> }` while the runtime carried nested JSON.

- `normalizeStoredMetadata`: keep an explicit `null` from a legacy pair
  instead of folding it into `""` (only a missing `value` defaults now), and
  store through `Object.defineProperty` so a legacy `__proto__` key becomes an
  own property rather than hitting the prototype setter.

- Drop the unjustified `as unknown as` from the migration test; the legacy
  fixture is untyped and cast once where it crosses into the typed parser.

- The TUI settings test asserted only that the app mounted, so it could not
  have caught a pair array being forwarded as numeric `_meta` keys. The fake
  client now captures its constructor options and the test asserts the nested
  object arrives intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>

clients/tui/__tests__/App.test.tsx

@@ -102,7 +102,13 @@ const h = vi.hoisted(() => {
   // handler for the event (the common single-server case).
   type EventEntry = { client: unknown; fn: (event: unknown) => void };
   const clientEvents = new Map<string, Set<EventEntry>>();
-  const clientInstances: Array<{ cfg?: { type?: string; url?: string } }> = [];
+  const clientInstances: Array<{
+    cfg?: { type?: string; url?: string };
+    // The options the mount effect built for this server. Captured so a test
+    // can assert what was actually handed to the client — `defaultMetadata`
+    // in particular, which is otherwise invisible to a render-only assertion.
+    opts?: { defaultMetadata?: unknown };
+  }> = [];
   const fireClientEvent = (event: string, detail?: unknown) => {
     clientEvents.get(event)?.forEach((e) => e.fn({ detail }));
   };
@@ -127,8 +133,13 @@ const h = vi.hoisted(() => {
   }
   class FakeClient {
     cfg: { type?: string; url?: string } | undefined;
-    constructor(config?: { type?: string; url?: string }) {
+    opts: { defaultMetadata?: unknown } | undefined;
+    constructor(
+      config?: { type?: string; url?: string },
+      opts?: { defaultMetadata?: unknown },
+    ) {
       this.cfg = config;
+      this.opts = opts;
       clientInstances.push(this);
     }
     // Derive the transport type from the server config the client was built
@@ -393,10 +404,10 @@ function httpWithSettings(): Record<string, TuiServer> {
       config: { type: "streamable-http", url: "http://x" },
       settings: {
         requestTimeout: 5000,
-        metadata: [
+        // Object-shaped with a nested value (#1910) — the shape the TUI now
-          { key: "team", value: "alpha" },
+        // forwards verbatim. As a pair array this would have reached the wire
-          { key: "  ", value: "ignored" },
+        // as numeric `_meta` keys.
-        ],
+        metadata: { team: "alpha", trace: { id: "abc", hops: [1, 2] } },
         oauthClientId: "cid",
         oauthClientSecret: "secret",
         oauthScopes: "read write",
@@ -1114,6 +1125,13 @@ describe("App (input handling, focus, effects)", () => {
   it("builds a client with saved settings (metadata, oauth, timeout)", async () => {
     const r = await mount(httpWithSettings());
     await expectFrame(r, "MCP Servers");
+    // Not just "it mounted": the saved metadata must reach the client as the
+    // same object, nesting intact (#1910).
+    const web = h.clientInstances.find((c) => c.cfg?.url === "http://x");
+    expect(web?.opts?.defaultMetadata).toEqual({
+      team: "alpha",
+      trace: { id: "abc", hops: [1, 2] },
+    });
   });
 
   it("passes top-level oauth client credentials into an http client", async () => {

clients/web/src/test/core/mcp/serverList.test.ts

@@ -1424,6 +1424,26 @@ describe("normalizeStoredMetadata (#1910)", () => {
     ).toEqual({ kept: "yes" });
   });
 
+  it("preserves an explicit null in a legacy pair rather than blanking it", () => {
+    // `null` is a legal `_meta` value, so the migration must not fold it into
+    // `""` — only a genuinely absent `value` defaults.
+    expect(normalizeStoredMetadata([{ key: "z", value: null }])).toEqual({
+      z: null,
+    });
+  });
+
+  it("stores a legacy `__proto__` key as an own property", () => {
+    // Plain assignment would hit the prototype setter: the entry would vanish
+    // (and an object value would mutate the prototype) instead of being stored.
+    const out = normalizeStoredMetadata([
+      { key: "__proto__", value: { polluted: true } },
+    ]);
+    expect(Object.hasOwn(out, "__proto__")).toBe(true);
+    expect(out["__proto__"]).toEqual({ polluted: true });
+    expect(Object.getPrototypeOf(out)).toBe(Object.prototype);
+    expect(({} as Record<string, unknown>).polluted).toBeUndefined();
+  });
+
   it("reads a legacy pair with no value as an empty string", () => {
     expect(normalizeStoredMetadata([{ key: "k" }])).toEqual({ k: "" });
   });
@@ -1460,18 +1480,19 @@ describe("normalizeStoredMetadata (#1910)", () => {
   it("migrates a legacy file to the object shape on the next write", () => {
     // Read side accepts the pair array; write side emits only the object, so
     // one round-trip is the migration.
-    const entries = mcpConfigToServerEntries({
+    // The legacy on-disk shape no longer matches `MCPConfig` — that is what
+    // the test is about — so the fixture is untyped and cast once where it
+    // crosses into the typed parser, the same boundary a real file crosses.
+    const legacyFile: unknown = {
       mcpServers: {
         legacy: {
           type: "streamable-http",
           url: "https://x.test/mcp",
-          metadata: [{ key: "tenant", value: "acme" }] as unknown as Record<
+          metadata: [{ key: "tenant", value: "acme" }],
-            string,
-            never
-          >,
         },
       },
-    });
+    };
+    const entries = mcpConfigToServerEntries(legacyFile as MCPConfig);
     expect(entries[0]?.settings?.metadata).toEqual({ tenant: "acme" });
     const round = serverEntriesToMcpConfig(entries);
     expect(round.mcpServers.legacy?.metadata).toEqual({ tenant: "acme" });

core/mcp/inspectorClient.ts

@@ -1037,7 +1037,7 @@ export class InspectorClient extends InspectorClientEventTarget {
    */
   private mergeMeta(
     callMetadata?: RequestMetadata,
-  ): Record<string, string> | undefined {
+  ): RequestMetadata | undefined {
     const defaults = this.defaultMetadata;
     // Modern-era per-request log level (#1629): stamp the opt-in `_meta` key on
     // every request so the server emits `notifications/message` on this
@@ -1132,7 +1132,7 @@ export class InspectorClient extends InspectorClientEventTarget {
    */
   private aggregateListParams(
     metadata?: RequestMetadata,
-  ): { _meta: Record<string, string> } | undefined {
+  ): { _meta: RequestMetadata } | undefined {
     const effectiveMeta = this.mergeMeta(metadata);
     return effectiveMeta ? { _meta: effectiveMeta } : undefined;
   }
@@ -4279,7 +4279,7 @@ export class InspectorClient extends InspectorClientEventTarget {
     const callParams: {
       name: string;
       arguments: Record<string, JsonValue>;
-      _meta?: Record<string, string>;
+      _meta?: RequestMetadata;
       task?: { ttl: number };
     } = {
       name: tool.name,

core/mcp/serverList.ts

@@ -234,7 +234,19 @@ export function normalizeStoredMetadata(
       }
       const { key, value } = entry as { key: string; value?: JsonValue };
       if (key.trim() === "") continue;
-      out[key] = value ?? "";
+      // `??`, not `||`: `null` is a legal `_meta` value and must survive the
+      // migration as `null`. Only a genuinely absent `value` becomes `""`.
+      //
+      // `defineProperty` rather than `out[key] = …` because a legacy key of
+      // `__proto__` would otherwise hit the prototype setter — silently
+      // dropping the entry (or, for an object value, mutating the prototype)
+      // instead of storing an own property under that name.
+      Object.defineProperty(out, key, {
+        value: value === undefined ? "" : value,
+        writable: true,
+        enumerable: true,
+        configurable: true,
+      });
     }
     return out;
   }