diff --git a/package-lock.json b/package-lock.json index 9d68f4a..dd14a4a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@molecule-ai/mcp-server", - "version": "1.4.0", + "version": "1.4.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@molecule-ai/mcp-server", - "version": "1.4.0", + "version": "1.4.1", "dependencies": { "@modelcontextprotocol/sdk": "^1.12.0", "pino": "^9.6.0", diff --git a/package.json b/package.json index 6bb0976..e31d7f2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@molecule-ai/mcp-server", - "version": "1.4.0", + "version": "1.4.1", "description": "MCP server for Molecule AI Agent Team \u2014 manage workspaces, agents, and skills from any AI coding tool", "type": "module", "exports": { diff --git a/src/__tests__/session-cursor.test.ts b/src/__tests__/session-cursor.test.ts index 6d4ff23..c9a083e 100644 --- a/src/__tests__/session-cursor.test.ts +++ b/src/__tests__/session-cursor.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, rmSync, writeFileSync, readFileSync, readdirSync, existsSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync, readFileSync, readdirSync, existsSync, statSync, chmodSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -126,6 +126,42 @@ describe("CursorStore", () => { expect(JSON.parse(readFileSync(join(dir, "cursor.json"), "utf8"))).toEqual({ "ws-1": "act-100" }); }); + it("set ignores empty / non-string so the round-trip stays total (no throw)", () => { + const a = new CursorStore({ stateDir: dir }); + a.set("ws-1", ""); // no-op, never throws + // @ts-expect-error — JS-caller path that bypasses the type + a.set("ws-1", undefined); + expect(a.size).toBe(0); // nothing stored + a.set("ws-1", "act-1"); + a.save(); + expect(new CursorStore({ stateDir: dir }).load().get("ws-1")).toBe("act-1"); + }); + + it("trySave returns true on success and false+onError on failure", () => { + const ok = new CursorStore({ stateDir: dir }); + ok.set("ws-1", "act-1"); + const errs: unknown[] = []; + expect(ok.trySave((e) => errs.push(e))).toBe(true); + expect(errs).toHaveLength(0); + + // stateDir under a non-existent parent → writeFileSync throws → trySave false. + const bad = new CursorStore({ stateDir: join(dir, "nope", "deeper") }); + bad.set("ws-1", "act-1"); + expect(bad.trySave((e) => errs.push(e))).toBe(false); + expect(errs).toHaveLength(1); + }); + + it("save applies fileMode even over a stale same-PID temp (no 0o644 leak)", () => { + const a = new CursorStore({ stateDir: dir }); // default mode 0o600 + // Simulate a crashed prior save leaving a world-readable temp. + const tmp = join(dir, `cursor.json.tmp.${process.pid}`); + writeFileSync(tmp, "{}", { mode: 0o644 }); + chmodSync(tmp, 0o644); + a.set("ws-1", "act-1"); + a.save(); + expect(statSync(join(dir, "cursor.json")).mode & 0o777).toBe(0o600); + }); + it("unlink removes the backing file and is a no-op when already gone", () => { const a = new CursorStore({ stateDir: dir, sessionKey: "777" }); a.set("ws-1", "act-1"); diff --git a/src/session-cursor.ts b/src/session-cursor.ts index 6e8c9fc..2bb3644 100644 --- a/src/session-cursor.ts +++ b/src/session-cursor.ts @@ -175,6 +175,12 @@ export class CursorStore { } set(workspaceId: string, activityId: string): void { + // Ignore empty/non-string so the in-memory state can't diverge from what + // survives a save→load round-trip (load() drops empty/non-string values). + // A no-op (not a throw): callers like a poll tick advance the cursor via + // `set(ws, newest)` from a `void`-launched loop, where a throw would abort + // the tick before its save() and could wedge the cursor — a skip can't. + if (typeof activityId !== "string" || activityId.length === 0) return; this.cursors.set(workspaceId, activityId); } @@ -193,16 +199,40 @@ export class CursorStore { /** * Atomically persist to disk (temp + rename). The temp name is PID-suffixed * so two writers never collide on the temp path. Throws on write failure — - * the caller (typically a setInterval tick) decides whether to log+swallow. + * the caller decides whether to log+swallow (or use {@link trySave}). */ save(): void { const obj: Record = {}; for (const [k, v] of this.cursors) obj[k] = v; const tmp = `${this.path}.tmp.${process.pid}`; + // Clear any leftover temp from a crashed same-PID save first: writeFileSync + // only applies `mode` when it CREATES the file, so writing over a stale temp + // would silently inherit that file's mode (a 0o644 leak through rename). + try { + unlinkSync(tmp); + } catch { + // No stale temp — the common case. + } writeFileSync(tmp, JSON.stringify(obj, null, 2), { mode: this.fileMode }); renameSync(tmp, this.path); } + /** + * {@link save} wrapped to never throw — for poll-tick / setInterval callers + * where an unhandled rejection would kill the loop. Returns true on success; + * on failure invokes `onError` (if given) and returns false. Adapters should + * prefer this over hand-rolling the try/catch. + */ + trySave(onError?: (err: unknown) => void): boolean { + try { + this.save(); + return true; + } catch (err) { + onError?.(err); + return false; + } + } + /** Remove the backing file. Used by a secondary session on clean exit. No-op if already gone. */ unlink(): void { try {