diff --git a/CHANGELOG.md b/CHANGELOG.md index 96cf7aa..760a828 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CLAUDE.md b/CLAUDE.md index 17d48f0..16ce05d 100644 --- a/CLAUDE.md +++ b/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 --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 diff --git a/package-lock.json b/package-lock.json index bfa3bc2..2ef4115 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@tmhs/screencast-mcp", - "version": "0.8.12", + "version": "0.8.13", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@tmhs/screencast-mcp", - "version": "0.8.12", + "version": "0.8.13", "license": "CC-BY-NC-ND-4.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.12.1", @@ -879,9 +879,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -899,9 +896,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -919,9 +913,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -939,9 +930,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -959,9 +947,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -979,9 +964,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2783,9 +2765,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2807,9 +2786,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2831,9 +2807,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2855,9 +2828,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/package.json b/package.json index 1c30973..ba3900c 100644 --- a/package.json +++ b/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", diff --git a/src/__tests__/sessions.test.ts b/src/__tests__/sessions.test.ts index aa640a8..c9dcf06 100644 --- a/src/__tests__/sessions.test.ts +++ b/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); diff --git a/src/index.ts b/src/index.ts index 143e1f2..74faa7e 100644 --- a/src/index.ts +++ b/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); diff --git a/src/tools/concat.ts b/src/tools/concat.ts index 59c9f1d..5a59d31 100644 --- a/src/tools/concat.ts +++ b/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); } diff --git a/src/tools/startRecording.ts b/src/tools/startRecording.ts index eebd05a..ee65906 100644 --- a/src/tools/startRecording.ts +++ b/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); }); diff --git a/src/tools/stopRecording.ts b/src/tools/stopRecording.ts index 70fd460..cf5fffd 100644 --- a/src/tools/stopRecording.ts +++ b/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 { + 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 { 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({ diff --git a/src/utils/sessions.ts b/src/utils/sessions.ts index bf47764..3a517b6 100644 --- a/src/utils/sessions.ts +++ b/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 { 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(); } }