Fix the defects found reviewing PRs #3–#17

README.md

@@ -175,7 +175,7 @@ Two prompts appear as slash commands in clients that show MCP prompts: `record-d
 
 ### Confirming live actions
 
-With `OBS_MCP_CONFIRM_LIVE=true`, `obs-stop-stream`, `obs-toggle-stream`, `obs-stop-record`, `obs-toggle-record`, and `obs-take-stop` ask before they run, as do `obs-call-request` and `obs-batch` when they carry one of those requests. Clients that support elicitation (Claude Code, VS Code, Cursor) show the user a confirmation. Other clients get an error asking the model to get the user's agreement and call again with `confirm: true`. These tools are also marked destructive, so clients that ask before destructive tools already prompt without this setting.
+With `OBS_MCP_CONFIRM_LIVE=true`, `obs-stop-stream`, `obs-toggle-stream`, `obs-stop-record`, `obs-toggle-record`, `obs-take-stop`, `obs-stop-output`, and `obs-toggle-output` ask before they run, as do `obs-call-request` and `obs-batch` when they carry one of those requests. The generic output tools are included because the stream and the recording are outputs too. Clients that support elicitation (Claude Code, VS Code, Cursor) show the user a confirmation, and `confirm: true` from the model does not skip it. Other clients get an error asking the model to get the user's agreement and call again with `confirm: true`. These tools are also marked destructive, so clients that ask before destructive tools already prompt without this setting.
 
 ## Where it fits
 
@@ -250,20 +250,21 @@ Without `apply: true` it returns the plan. When applying, it creates what is mis
 
 `obs-snapshot` saves the settings, mute, volume, and audio tracks of inputs, and the transform, visibility, lock, and order of scene items: by default the program scene and the inputs in it. `obs-restore` compares OBS with a snapshot and sends only what differs, in one request batch; `dryRun: true` lists the changes first. It cannot recreate a removed input or item, and leaves items added since in place; it reports both. `obs-capture-window` takes a snapshot automatically before it changes an existing capture.
 
-Snapshots are saved in the server's state directory (the last 20, readable only by you), so `obs-restore` works after a restart and from another agent's session. Each server lists only the snapshots of the OBS instance it connects to. They contain input settings, such as file paths and browser source URLs. OBS's own scene collection file is not a substitute: OBS rewrites it while running.
+Snapshots are saved in the server's state directory (the last 20 you take, plus the last 10 taken automatically before a change, readable only by you), so `obs-restore` works after a restart and from another agent's session. Each server lists only the snapshots of the OBS instance it connects to. They contain input settings, such as file paths and browser source URLs. OBS's own scene collection file is not a substitute: OBS rewrites it while running.
 
 ### Sharing one server
 
 By default each agent starts its own server over stdio, with its own OBS connection, and nothing stops two agents from changing OBS at once. With `OBS_MCP_HTTP_PORT`, one server listens on `http://127.0.0.1:<port>/mcp` and every agent connects to it:
 
 ```bash
 OBS_MCP_HTTP_PORT=8765 OBS_MCP_READ_OBS_CONFIG=true node /path/to/OBS-MCP/build/index.js
-claude mcp add --transport http obs http://127.0.0.1:8765/mcp
+claude mcp add --transport http obs 'http://127.0.0.1:8765/mcp?client=claude'
+# In Codex's config, point it at http://127.0.0.1:8765/mcp?client=codex
 ```
 
-The server listens only on 127.0.0.1 and rejects requests whose Host or Origin header is not localhost, which blocks DNS rebinding from web pages. Set `OBS_MCP_HTTP_TOKEN` to also require a bearer token. Each HTTP request is served statelessly, so `OBS_MCP_DYNAMIC_TOOLSETS` is ignored over HTTP.
+The server listens only on 127.0.0.1 and rejects requests whose Host or Origin header is not localhost, which blocks DNS rebinding from web pages. Without `OBS_MCP_HTTP_TOKEN`, any program running on the machine can control OBS through the server; set it to require a bearer token. Each HTTP request is served statelessly, so `OBS_MCP_DYNAMIC_TOOLSETS` is ignored over HTTP.
 
-While agents share a server, `obs-claim-control` gives one of them control of OBS for a set time (30 minutes by default). Until it calls `obs-release-control` or the time runs out, tools that change OBS refuse calls from other clients; reading OBS and stopping outputs always work. `obs-control-status` shows who holds control. `obs-take-start` claims control for the length of the take. Clients are told apart by the name they report (Claude Code and Codex report different names), so two sessions of the same client share control.
+While agents share a server, `obs-claim-control` gives one of them control of OBS for a set time (30 minutes by default). Until it calls `obs-release-control` or the time runs out, tools that change OBS refuse calls from other clients; reading OBS and stopping outputs always work. `obs-control-status` shows who holds control. `obs-take-start` claims control for the length of the take. Clients are told apart by name: the `client` parameter of the URL they connect with, or else the name a 2026-07-28 client sends with each request. Clients using the 2025 protocol over HTTP send each request without a name, so give each agent its own `?client=` name. An unnamed client cannot claim control and is refused while another client holds it. Two agents given the same name share control. The names are not credentials; control is for agents cooperating on one machine, not for keeping out someone who can reach the server.
 
 ## Troubleshooting
 

src/client.test.ts

@@ -333,6 +333,19 @@ describe("OBSWebSocketClient high-volume events", () => {
     await vi.waitFor(() => expect(server.eventSubscriptions(2)).toBe(EventSubscription.All));
   });
 
+  it("sends a subscription added while identifying once OBS has answered", async () => {
+    const server = await createServer();
+    const client = createClient(server);
+
+    const connecting = client.connect();
+    await server.waitForFrame(({ frame }) => frame.op === OBS_OP.Identify);
+    const release = client.subscribeHighVolume(EventSubscription.InputVolumeMeters);
+    await connecting;
+
+    await vi.waitFor(() => expect(server.eventSubscriptions()).toBe(EventSubscription.All | EventSubscription.InputVolumeMeters));
+    release();
+  });
+
   it("keeps unapplied output settings flagged when OBS acknowledges a Reidentify", async () => {
     const server = await createServer();
     const client = createClient(server);

src/client.ts

@@ -251,6 +251,8 @@ export class OBSWebSocketClient extends EventEmitter {
   private readonly pendingRequests = new Map<string, PendingRequest>();
   // How many callers want each high-volume subscription bit.
   private readonly highVolumeCounts = new Map<number, number>();
+  // The subscriptions OBS was last told about.
+  private sentSubscriptions: number = EventSubscription.All;
 
   constructor(url = "ws://localhost:4455", password: string | null = null) {
     super();
@@ -323,6 +325,7 @@ export class OBSWebSocketClient extends EventEmitter {
     const socket = this.ws;
     if (eventSubscriptions === before || !this.isConnected() || !socket || socket.readyState !== WebSocket.OPEN) return;
     try {
+      this.sentSubscriptions = eventSubscriptions;
       socket.send(JSON.stringify({ op: OpCode.Reidentify, d: { eventSubscriptions } }));
     } catch (error) {
       logger.error(`Unable to change OBS event subscriptions: ${asError(error).message}`);
@@ -595,8 +598,12 @@ export class OBSWebSocketClient extends EventEmitter {
         }
         // OBS answers Reidentify with Identified too; only a new session means
         // OBS may have restarted and rebuilt its outputs.
-        if (!this.identified) this.outputSettingsPending = false;
-        this.identified = true;
+        if (!this.identified) {
+          this.outputSettingsPending = false;
+          this.identified = true;
+          // A subscription added while Identify was in flight is not in what OBS was sent.
+          this.reidentifyIfChanged(this.sentSubscriptions);
+        }
         this.emit("identified", socket);
         break;
       case OpCode.RequestResponse: {
@@ -679,7 +686,7 @@ export class OBSWebSocketClient extends EventEmitter {
       op: OpCode.Identify,
       d: {
         rpcVersion: hello.rpcVersion,
-        eventSubscriptions: this.eventSubscriptions(),
+        eventSubscriptions: this.sentSubscriptions = this.eventSubscriptions(),
         ...(authentication ? { authentication } : {}),
       },
     };

src/http.ts

@@ -7,6 +7,7 @@ import crypto from "node:crypto";
 import http from "node:http";
 import type { AddressInfo } from "node:net";
 import { Readable } from "node:stream";
+import { pipeline } from "node:stream/promises";
 import {
   createMcpHandler,
   hostHeaderValidationResponse,
@@ -64,9 +65,20 @@ async function send(res: http.ServerResponse, response: Response): Promise<void>
     res.end();
     return;
   }
-  const body = Readable.fromWeb(response.body as import("node:stream/web").ReadableStream);
-  res.on("close", () => body.destroy());
-  body.pipe(res);
+  // pipeline destroys both sides on error or client disconnect instead of throwing an unhandled 'error'.
+  await pipeline(Readable.fromWeb(response.body as import("node:stream/web").ReadableStream), res).catch((error: unknown) => {
+    logger.debug(`MCP HTTP response stream ended early: ${error instanceof Error ? error.message : String(error)}`);
+  });
+}
+
+/**
+ * The name a client declares with `?client=<name>` in the URL it connects to.
+ * 2025-era clients cannot otherwise be told apart over stateless HTTP, and
+ * the control lease needs a name.
+ */
+function declaredClient(url: URL): string | undefined {
+  const name = url.searchParams.get("client")?.trim();
+  return name && /^[\w.@:-]{1,64}$/.test(name) ? name : undefined;
 }
 
 /**
@@ -84,8 +96,8 @@ export async function serveHttp(factory: () => McpServer, options: HttpOptions):
   const server = http.createServer((req, res) => {
     void (async () => {
       try {
-        const path = new URL(req.url ?? "/", `http://${LOOPBACK}`).pathname;
-        if (path !== MCP_PATH) {
+        const url = new URL(req.url ?? "/", `http://${LOOPBACK}`);
+        if (url.pathname !== MCP_PATH) {
           res.writeHead(404, { "content-type": "text/plain" }).end(`Not found; the MCP endpoint is ${MCP_PATH}`);
           return;
         }
@@ -99,7 +111,8 @@ export async function serveHttp(factory: () => McpServer, options: HttpOptions):
           res.writeHead(401, { "content-type": "text/plain", "www-authenticate": "Bearer" }).end("Missing or wrong bearer token");
           return;
         }
-        await send(res, await handler.fetch(request));
+        const clientId = declaredClient(url);
+        await send(res, await handler.fetch(request, clientId ? { authInfo: { token: options.token ?? "", clientId, scopes: [] } } : undefined));
       } catch (error) {
         logger.error(`MCP HTTP request failed: ${error instanceof Error ? error.message : String(error)}`);
         if (!res.headersSent) res.writeHead(500, { "content-type": "text/plain" });

src/server.ts

@@ -91,7 +91,9 @@ async function attemptOBSConnection(): Promise<void> {
 
 obsClient.on("disconnected", () => {
   obsConnected = false;
-  if (!shuttingDown) {
+  // A failed attempt schedules its own retry once it has counted the failure;
+  // scheduling here too would use the count from before it.
+  if (!shuttingDown && !connectionAttempt) {
     logger.log("OBS WebSocket disconnected; scheduling a reconnect.");
     scheduleReconnect();
   }

src/tools/apply-scene.test.ts

@@ -85,8 +85,8 @@ describe("obs-apply-scene", () => {
     const batches = harness.fakeObs.history().filter(({ frame }) => frame.op === OBS_OP.RequestBatch).length;
     expect(batches).toBeGreaterThanOrEqual(2);
     expect(result.structuredContent).toMatchObject({ applied: true, converged: true, failures: [] });
-    // A new scene has nothing to undo.
-    expect(result.structuredContent).not.toHaveProperty("snapshotId");
+    // The scene is new, but it reuses existing inputs, whose state is saved.
+    expect(result.structuredContent).toHaveProperty("snapshotId");
   });
 
   it("changes only what differs in an existing scene, and saves a snapshot first", async () => {
@@ -135,6 +135,54 @@ describe("obs-apply-scene", () => {
     expect(sent()).toEqual([]);
   });
 
+  it("stacks the listed sources in front, in order, from any starting order, in one call", async () => {
+    state.inputs.Webcam = { kind: "image_source", settings: {} };
+    state.inputs.Slides = { kind: "image_source", settings: {} };
+    const all = ["Background", "Chrome", "Webcam", "Slides"];
+    const permutations = (list: string[]): string[][] => list.length <= 1 ? [list]
+      : list.flatMap((head, index) => permutations([...list.slice(0, index), ...list.slice(index + 1)]).map((rest) => [head, ...rest]));
+    for (const order of permutations(all)) {
+      state.scenes.Demo = order.map((sourceName, index) => ({ sceneItemId: index + 1, sourceName, enabled: true, locked: false, transform: {} }));
+
+      const result = await harness.call("obs-apply-scene", {
+        sceneName: "Demo",
+        sources: [{ name: "Slides" }, { name: "Chrome" }],
+        order: true,
+        apply: true,
+      });
+
+      const names = state.scenes.Demo.map(({ sourceName }) => sourceName);
+      expect(names.slice(-2), `from ${order.join(",")}`).toEqual(["Slides", "Chrome"]);
+      expect(result.isError, `from ${order.join(",")}: ${resultText(result)}`).toBeFalsy();
+    }
+  });
+
+  it("refuses a source listed twice or named like a scene, before changing anything", async () => {
+    state.scenes["Be Right Back"] = [];
+
+    const twice = await harness.call("obs-apply-scene", { sceneName: "Demo", sources: [{ name: "Chrome" }, { name: "Chrome" }], apply: true });
+    const scene = await harness.call("obs-apply-scene", { sceneName: "Demo", sources: [{ name: "Be Right Back", kind: "image_source" }], apply: true });
+
+    expect(resultText(twice)).toContain("Chrome is listed more than once");
+    expect(resultText(scene)).toContain("Be Right Back is a scene, not an input");
+    expect(sent()).toEqual([]);
+  });
+
+  it("converges on decimal transforms, which OBS keeps as 32-bit floats", async () => {
+    const result = await harness.call("obs-apply-scene", {
+      sceneName: "Demo",
+      sources: [{ name: "Chrome", transform: { positionX: 100.1, scaleX: 0.3333, rotation: 12.34 } }],
+      apply: true,
+    });
+
+    expect(result.isError).toBeFalsy();
+    expect(resultText(result)).toContain("OBS now matches the spec");
+    expect(resultText(await harness.call("obs-apply-scene", {
+      sceneName: "Demo",
+      sources: [{ name: "Chrome", transform: { positionX: 100.1, scaleX: 0.3333, rotation: 12.34 } }],
+    }))).toBe("Scene Demo already matches");
+  });
+
   it("does not warn about an audio file, which has no picture", async () => {
     const result = await harness.call("obs-apply-scene", {
       sceneName: "Demo",