fix: remaining audit findings not yet fixed on main (release 0.8.13)

CHANGELOG.md

@@ -12,6 +12,31 @@ project adheres to [Semantic Versioning](https://semver.org/).
 - ESLint (flat config, typescript-eslint recommended) with a `lint` script, run
   in CI (#23). Tooling only; not part of the published package.
 
+## [0.8.13]
+
+### Fixed
+
+- **A recording whose ffmpeg crashes mid-run is now recorded as `failed`** with
+  the last lines of encoder stderr, instead of an indistinguishable clean
+  `stopped`. A deliberate `stop_recording` still finishes as `stopped`.
+- **`concat` works with relative input paths.** The concat demuxer resolves
+  relative list entries against the list file's directory (the OS temp dir),
+  not the server cwd; inputs are now made absolute before the list is written.
+- **The server advertises its real version.** `src/index.ts` reads the version
+  from `package.json` at runtime instead of a hardcoded string that had fallen
+  behind (it said 0.8.0 while the package was at 0.8.12).
+- **A corrupt `sessions.json` is backed up to `sessions.json.bak`** before the
+  registry starts fresh, instead of being silently discarded along with any
+  live-recording entries it held.
+- **`stop_recording` no longer sleeps a blind 400 ms** before probing the
+  finalized file; it polls for process exit (kill paths) or probes immediately
+  (graceful path).
+
+### Changed
+
+- CLAUDE.md no longer references a `src/providers/` layer that does not exist
+  in this codebase.
+
 ## [0.8.12]
 
 ### Fixed

CLAUDE.md

@@ -15,8 +15,7 @@ Screencast MCP -- MCP server for Windows screen recording, frame sampling, and m
 ## Key paths
 
 - Source: `src/` (TypeScript)
-- Provider adapters: `src/providers/` (implement the `Provider` interface, wired into `ProviderManager`)
-- Tools: `src/tools/`
+- Tools: `src/tools/`; shared helpers: `src/utils/`
 - Package manifest: `package.json` (version source of truth)
 - Tool list: `mcp-tools.json` (enumerates the MCP tools)
 - Docs site: `docs/`
@@ -27,7 +26,7 @@ Screencast MCP -- MCP server for Windows screen recording, frame sampling, and m
 - Use conventional commits (`feat:`, `fix:`, `chore:`, `docs:`)
 - Bump the version in `package.json` in your PR (`npm version <patch|minor|major> --no-git-tag-version`, keeps the lockfile in sync and avoids a stray tag); `release.yml` tags and publishes that version on merge
 - Add a matching entry to `CHANGELOG.md` under the new version heading
-- Provider adapters live in `src/providers/` and implement the `Provider` interface, wired into `ProviderManager`; tools live in `src/tools/`
+- Tools live in `src/tools/` (one `register(server)` per file, wired in `src/index.ts`); pure helpers live in `src/utils/`
 - Keep `mcp-tools.json` in sync with the registered tools
 
 ## Testing

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@tmhs/screencast-mcp",
-  "version": "0.8.12",
+  "version": "0.8.13",
   "description": "MCP server for Windows screen recording, frame sampling, and minimal ffmpeg edits",
   "type": "module",
   "main": "dist/index.js",

src/__tests__/sessions.test.ts

@@ -144,6 +144,16 @@ describe("SessionStore", () => {
     expect(finalIds).toEqual(["a", "b"]);
   });
 
+  it("backs up a corrupt registry to .bak instead of silently discarding it", () => {
+    const p = newPath();
+    writeFileSync(p, "{ not json");
+    const store = new SessionStore(p);
+    store.load();
+    expect(store.list()).toEqual([]);
+    expect(existsSync(`${p}.bak`)).toBe(true);
+    rmSync(`${p}.bak`, { force: true });
+  });
+
   it("reaps a dead recording into a stopped state at boot", () => {
     const p = newPath();
     const a = new SessionStore(p);

src/index.ts

@@ -8,6 +8,7 @@
  * can contain anything on screen, including secrets. See the README threat
  * model.
  */
+import { createRequire } from "node:module";
 import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
 import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
 
@@ -38,9 +39,16 @@ import { register as registerMusicBed } from "./tools/musicBed.js";
 import { register as registerReframe } from "./tools/reframe.js";
 import { register as registerExportPreset } from "./tools/exportPreset.js";
 
+// Resolves from src/ in dev and dist/ in the published package alike; keeps
+// the advertised version from drifting from package.json (the hardcoded
+// string here had already fallen behind twice).
+const { version } = createRequire(import.meta.url)("../package.json") as {
+  version: string;
+};
+
 const server = new McpServer({
   name: "screencast-mcp",
-  version: "0.8.0",
+  version,
 });
 
 registerStartRecording(server);

src/tools/concat.ts

@@ -1,5 +1,5 @@
 import { z } from "zod";
-import { extname } from "node:path";
+import { extname, resolve } from "node:path";
 import { existsSync, writeFileSync, rmSync } from "node:fs";
 import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
 import { errorResponse, okResponse, ScreencastError } from "../utils/errors.js";
@@ -24,19 +24,23 @@ export function register(server: McpServer): void {
     async (args) => {
       try {
         requireFfmpeg();
-        for (const f of args.inputs) {
+        // The concat demuxer resolves relative list entries against the list
+        // file's directory (the OS temp dir), not our cwd, so inputs must be
+        // absolute by the time they are written to the list.
+        const inputs = args.inputs.map((f) => resolve(f));
+        for (const f of inputs) {
           if (!existsSync(f)) throw new ScreencastError(`Input file not found: ${f}`);
         }
-        const ext = extname(args.inputs[0]) || ".mp4";
+        const ext = extname(inputs[0]) || ".mp4";
         const output = resolveOutput(args.output, subdir("edits"), `concat-${stamp()}-${rand()}${ext}`);
         const listFile = tempPath(".txt");
-        writeFileSync(listFile, buildConcatListContent(args.inputs));
+        writeFileSync(listFile, buildConcatListContent(inputs));
         try {
           await runFfmpeg(buildConcatArgs(listFile, output), 10 * 60_000);
         } finally {
           rmSync(listFile, { force: true });
         }
-        return okResponse({ outputPath: output, inputCount: args.inputs.length });
+        return okResponse({ outputPath: output, inputCount: inputs.length });
       } catch (error) {
         return errorResponse(error);
       }

src/tools/startRecording.ts

@@ -136,11 +136,27 @@ export function register(server: McpServer): void {
         const store = getStore();
         store.create(record);
         store.attachChild(id, child);
-        // Keep the on-disk record consistent if the child dies on its own.
-        child.on("exit", () => {
+        // Keep the on-disk record consistent if the child dies on its own. A
+        // non-zero exit is a crash (disk full, encoder failure), not a clean
+        // stop - record it as "failed" with the stderr tail so the caller can
+        // tell the two apart. stop_recording's own update runs after this and
+        // overrides for deliberate stops.
+        child.on("exit", (code) => {
           const cur = store.get(id);
           if (cur && cur.status === "recording") {
-            store.update(id, { status: "stopped", stoppedAt: new Date().toISOString() });
+            if (code !== 0 && code !== null) {
+              store.update(id, {
+                status: "failed",
+                stoppedAt: new Date().toISOString(),
+                error: `ffmpeg exited with code ${code}:\n${stderrTail
+                  .trim()
+                  .split("\n")
+                  .slice(-6)
+                  .join("\n")}`,
+              });
+            } else {
+              store.update(id, { status: "stopped", stoppedAt: new Date().toISOString() });
+            }
           }
           store.detachChild(id);
         });

src/tools/stopRecording.ts

@@ -12,6 +12,14 @@ const inputSchema = {
   sessionId: z.string().min(1).describe("Session id returned by start_recording."),
 };
 
+/** Poll until a pid is gone (process fully exited, file handles released). */
+async function waitForPidExit(pid: number, ms: number): Promise<void> {
+  const deadline = Date.now() + ms;
+  while (Date.now() < deadline && isAlive(pid)) {
+    await new Promise((r) => setTimeout(r, 100));
+  }
+}
+
 /** Wait up to ms for a child to exit; resolve true if it did. */
 function waitForExit(child: ChildProcess, ms: number): Promise<boolean> {
   return new Promise((resolve) => {
@@ -69,22 +77,30 @@ export function register(server: McpServer): void {
           child.stdin.write("q\n");
           child.stdin.end();
           graceful = await waitForExit(child, 8000);
-          if (!graceful && record.pid !== null) killPid(record.pid);
+          if (!graceful && record.pid !== null) {
+            killPid(record.pid);
+            await waitForPidExit(record.pid, 2000);
+          }
         } else if (record.pid !== null && isAlive(record.pid)) {
           // Cross-restart stop: no stdin handle. Terminate by pid. The
           // fragmented-mp4 muxing keeps the partial file playable.
-          if (isFfmpegProcess(record.pid)) killPid(record.pid);
+          if (isFfmpegProcess(record.pid)) {
+            killPid(record.pid);
+            await waitForPidExit(record.pid, 2000);
+          }
         }
         store.detachChild(record.id);
 
-        // Give the muxer a moment to flush the trailer.
-        await new Promise((r) => setTimeout(r, 400));
         const durationSec = await probeDuration(ffprobe, record.outputPath);
 
         const updated = store.update(record.id, {
           status: "stopped",
           stoppedAt: new Date().toISOString(),
           durationSec: durationSec ?? undefined,
+          // A forced kill can transiently mark the record "failed" via the
+          // crash detector in start_recording; a deliberate stop is not a
+          // failure, so clear any such error.
+          error: undefined,
         });
 
         return okResponse({

src/utils/sessions.ts

@@ -11,7 +11,13 @@
  * logic (classifyOrphan) is pure so it is unit-tested without real processes.
  */
 import { spawnSync } from "node:child_process";
-import { readFileSync, writeFileSync, renameSync, existsSync } from "node:fs";
+import {
+  readFileSync,
+  writeFileSync,
+  renameSync,
+  existsSync,
+  copyFileSync,
+} from "node:fs";
 import type { ChildProcess } from "node:child_process";
 import type { Quality } from "./targets.js";
 
@@ -131,13 +137,24 @@ export class SessionStore {
 
   constructor(private readonly path: string) {}
 
-  /** Read the on-disk records, tolerating a missing or corrupt file. */
+  /** Read the on-disk records, tolerating a missing or corrupt file. A corrupt
+   * registry is copied to .bak for inspection instead of silently discarded -
+   * it may hold "recording" entries whose ffmpeg would otherwise never be
+   * reaped. */
   private readDisk(): Map<string, SessionRecord> {
     if (!existsSync(this.path)) return new Map();
     try {
       const data = JSON.parse(readFileSync(this.path, "utf8")) as SessionRecord[];
       return new Map(data.map((r) => [r.id, r]));
     } catch {
+      try {
+        copyFileSync(this.path, `${this.path}.bak`);
+        process.stderr.write(
+          `Session registry was unreadable; saved a copy to ${this.path}.bak and starting fresh.\n`,
+        );
+      } catch {
+        /* best effort */
+      }
       return new Map();
     }
   }