From ba173761326c65dab472bf7beb74b70fe38aefdc Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:13:51 +0200 Subject: [PATCH 1/6] Fix duplicate ids, validate CLI input, harden load/save - add() uses max id + 1 so ids are never reused after rm - done/rm reject missing, non-numeric and unknown ids; add rejects empty text - load() gives a readable error on bad JSON or non-list data - save() writes a temp file and renames it - CLI checks the command before loading and no longer relies on process.exit for ls - Add regression tests for all of the above Co-Authored-By: Claude Opus 5.5 --- src/cli.js | 59 +++++++++++++++++++++++++--------------- src/todo.js | 22 ++++++++++++--- test/cli.test.js | 69 +++++++++++++++++++++++++++++++++++++++++++++++ test/todo.test.js | 42 ++++++++++++++++++++++++++++- 4 files changed, 166 insertions(+), 26 deletions(-) create mode 100644 test/cli.test.js diff --git a/src/cli.js b/src/cli.js index 90ffe77..85d7f28 100755 --- a/src/cli.js +++ b/src/cli.js @@ -1,28 +1,45 @@ #!/usr/bin/env node -import { load, save, add, complete, remove, format } from "./todo.js"; +import { load, save, add, has, complete, remove, format } from "./todo.js"; const FILE = process.env.TODO_FILE ?? "todos.json"; -const [cmd, ...args] = process.argv.slice(2); -let todos = load(FILE); +const USAGE = "Usage: todo [ls | add | done | rm ]"; +const [cmd = "ls", ...args] = process.argv.slice(2); -switch (cmd) { - case "add": - todos = add(todos, args.join(" ")); - break; - case "done": - todos = complete(todos, Number(args[0])); - break; - case "rm": - todos = remove(todos, Number(args[0])); - break; - case "ls": - case undefined: - console.log(format(todos)); - process.exit(0); - default: - console.error(`Unknown command: ${cmd}\nUsage: todo [ls | add | done | rm ]`); - process.exit(1); +function fail(message) { + console.error(message); + process.exit(1); } -save(FILE, todos); +function parseId(todos, arg) { + const id = Number(arg); + if (arg === undefined || !Number.isInteger(id)) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); + if (!has(todos, id)) fail(`No todo with id ${id}`); + return id; +} + +if (!["ls", "add", "done", "rm"].includes(cmd)) fail(`Unknown command: ${cmd}\n${USAGE}`); + +let todos; +try { + todos = load(FILE); +} catch (err) { + fail(err.message); +} + +switch (cmd) { + case "add": { + const text = args.join(" ").trim(); + if (!text) fail(`Missing todo text\n${USAGE}`); + todos = add(todos, text); + break; + } + case "done": + todos = complete(todos, parseId(todos, args[0])); + break; + case "rm": + todos = remove(todos, parseId(todos, args[0])); + break; +} + +if (cmd !== "ls") save(FILE, todos); console.log(format(todos)); diff --git a/src/todo.js b/src/todo.js index efb8343..c9e6e94 100644 --- a/src/todo.js +++ b/src/todo.js @@ -1,19 +1,33 @@ -import { readFileSync, writeFileSync, existsSync } from "node:fs"; +import { readFileSync, writeFileSync, renameSync, existsSync } from "node:fs"; export function load(path) { if (!existsSync(path)) return []; - return JSON.parse(readFileSync(path, "utf8")); + let todos; + try { + todos = JSON.parse(readFileSync(path, "utf8")); + } catch (err) { + throw new Error(`Could not read ${path}: ${err.message}`); + } + if (!Array.isArray(todos)) throw new Error(`Could not read ${path}: expected a list of todos`); + return todos; } +// Write to a temp file and rename, so an interrupted write never leaves `path` truncated. export function save(path, todos) { - writeFileSync(path, JSON.stringify(todos, null, 2) + "\n"); + const tmp = `${path}.tmp`; + writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n"); + renameSync(tmp, path); } export function add(todos, text) { - const id = todos.length + 1; + const id = Math.max(0, ...todos.map((t) => t.id)) + 1; return [...todos, { id, text, done: false }]; } +export function has(todos, id) { + return todos.some((t) => t.id === id); +} + export function complete(todos, id) { return todos.map((t) => (t.id === id ? { ...t, done: true } : t)); } diff --git a/test/cli.test.js b/test/cli.test.js new file mode 100644 index 0000000..4e73029 --- /dev/null +++ b/test/cli.test.js @@ -0,0 +1,69 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync, readFileSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const CLI = fileURLToPath(new URL("../src/cli.js", import.meta.url)); + +function run(file, ...args) { + return spawnSync(process.execPath, [CLI, ...args], { + env: { ...process.env, TODO_FILE: file }, + encoding: "utf8", + }); +} + +const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json"); + +test("ls with no file prints the empty message and writes nothing", () => { + const file = tempFile(); + const res = run(file, "ls"); + assert.equal(res.status, 0); + assert.equal(res.stdout.trim(), "Nothing to do."); + assert.equal(existsSync(file), false); +}); + +test("add with no text fails without saving", () => { + const file = tempFile(); + const res = run(file, "add", " "); + assert.equal(res.status, 1); + assert.match(res.stderr, /Missing todo text/); + assert.equal(existsSync(file), false); +}); + +for (const [label, args, message] of [ + ["missing id", [], /Invalid id: \(missing\)/], + ["non-numeric id", ["abc"], /Invalid id: abc/], + ["unknown id", ["99"], /No todo with id 99/], +]) { + for (const cmd of ["done", "rm"]) { + test(`${cmd} with ${label} fails and leaves the file unchanged`, () => { + const file = tempFile(); + run(file, "add", "a"); + const before = readFileSync(file, "utf8"); + const res = run(file, cmd, ...args); + assert.equal(res.status, 1); + assert.match(res.stderr, message); + assert.equal(readFileSync(file, "utf8"), before); + }); + } +} + +test("unknown command prints usage even when the file is corrupt", () => { + const file = tempFile(); + writeFileSync(file, "{ not json"); + const res = run(file, "--help"); + assert.equal(res.status, 1); + assert.match(res.stderr, /Unknown command: --help\nUsage:/); +}); + +test("a corrupt file gives a readable error instead of a stack trace", () => { + const file = tempFile(); + writeFileSync(file, "{ not json"); + const res = run(file, "ls"); + assert.equal(res.status, 1); + assert.match(res.stderr, /^Could not read /); + assert.doesNotMatch(res.stderr, /at .*\.js:\d+/); +}); diff --git a/test/todo.test.js b/test/todo.test.js index 01687e7..04beccd 100644 --- a/test/todo.test.js +++ b/test/todo.test.js @@ -1,12 +1,28 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { add, complete, remove, format } from "../src/todo.js"; +import { mkdtempSync, writeFileSync, existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { load, save, add, has, complete, remove, format } from "../src/todo.js"; + +const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json"); test("add appends an open todo", () => { const todos = add([], "buy milk"); assert.deepEqual(todos, [{ id: 1, text: "buy milk", done: false }]); }); +test("add after a remove does not reuse an existing id", () => { + const todos = add(remove(add(add([], "a"), "b"), 1), "c"); + assert.deepEqual(todos.map((t) => t.id), [2, 3]); +}); + +test("has reports whether an id exists", () => { + const todos = add([], "a"); + assert.equal(has(todos, 1), true); + assert.equal(has(todos, 2), false); +}); + test("complete marks only the matching todo", () => { const todos = complete(add(add([], "a"), "b"), 2); assert.equal(todos[0].done, false); @@ -21,3 +37,27 @@ test("remove drops the matching todo", () => { test("format renders an empty list", () => { assert.equal(format([]), "Nothing to do."); }); + +test("load returns an empty list for a missing file", () => { + assert.deepEqual(load(tempFile()), []); +}); + +test("save then load round-trips and leaves no temp file", () => { + const file = tempFile(); + const todos = add([], "a"); + save(file, todos); + assert.deepEqual(load(file), todos); + assert.equal(existsSync(`${file}.tmp`), false); +}); + +test("load rejects invalid JSON with a readable error", () => { + const file = tempFile(); + writeFileSync(file, "{ not json"); + assert.throws(() => load(file), /Could not read .*todos\.json/); +}); + +test("load rejects JSON that is not a list", () => { + const file = tempFile(); + writeFileSync(file, "{}"); + assert.throws(() => load(file), /expected a list of todos/); +}); From 8dc519443818d5eceee16a01e5f65a6711ce75c4 Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:26:22 +0200 Subject: [PATCH 2/6] Address review: safer save, stricter load and id parsing - save() follows symlinks, keeps permissions, uses a unique temp file and cleans it up on failure - load() rejects entries that aren't {id, text, done} - add() finds the max id with reduce (no spread limit) - CLI accepts only plain digits as ids and reports save errors readably - Ignore *.tmp Co-Authored-By: Claude Opus 5.5 --- .gitignore | 1 + src/cli.js | 10 ++++++++-- src/todo.js | 27 ++++++++++++++++++++------ test/cli.test.js | 22 ++++++++++++++++++++-- test/todo.test.js | 48 ++++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 95 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index 285cf65..605e1fb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ node_modules/ todos.json +*.tmp .DS_Store # Personal Claude Code overrides (not shared) diff --git a/src/cli.js b/src/cli.js index 85d7f28..bdcc4ba 100755 --- a/src/cli.js +++ b/src/cli.js @@ -11,8 +11,8 @@ function fail(message) { } function parseId(todos, arg) { + if (!/^\d+$/.test(arg ?? "")) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); const id = Number(arg); - if (arg === undefined || !Number.isInteger(id)) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); if (!has(todos, id)) fail(`No todo with id ${id}`); return id; } @@ -41,5 +41,11 @@ switch (cmd) { break; } -if (cmd !== "ls") save(FILE, todos); +if (cmd !== "ls") { + try { + save(FILE, todos); + } catch (err) { + fail(err.message); + } +} console.log(format(todos)); diff --git a/src/todo.js b/src/todo.js index c9e6e94..b1a4a65 100644 --- a/src/todo.js +++ b/src/todo.js @@ -1,4 +1,8 @@ -import { readFileSync, writeFileSync, renameSync, existsSync } from "node:fs"; +import { readFileSync, writeFileSync, renameSync, existsSync, realpathSync, statSync, chmodSync, rmSync } from "node:fs"; +import { randomBytes } from "node:crypto"; + +const isTodo = (t) => + t !== null && typeof t === "object" && Number.isInteger(t.id) && typeof t.text === "string" && typeof t.done === "boolean"; export function load(path) { if (!existsSync(path)) return []; @@ -9,18 +13,29 @@ export function load(path) { throw new Error(`Could not read ${path}: ${err.message}`); } if (!Array.isArray(todos)) throw new Error(`Could not read ${path}: expected a list of todos`); + const bad = todos.findIndex((t) => !isTodo(t)); + if (bad !== -1) throw new Error(`Could not read ${path}: entry ${bad + 1} is not a valid todo`); return todos; } -// Write to a temp file and rename, so an interrupted write never leaves `path` truncated. +// Write a uniquely named temp file next to the real file (following symlinks), copy over +// its permissions, then rename it into place, so an interrupted write never truncates it. export function save(path, todos) { - const tmp = `${path}.tmp`; - writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n"); - renameSync(tmp, path); + const exists = existsSync(path); + const target = exists ? realpathSync(path) : path; + const tmp = `${target}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`; + try { + writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n"); + if (exists) chmodSync(tmp, statSync(target).mode); + renameSync(tmp, target); + } catch (err) { + rmSync(tmp, { force: true }); + throw new Error(`Could not save ${path}: ${err.message}`); + } } export function add(todos, text) { - const id = Math.max(0, ...todos.map((t) => t.id)) + 1; + const id = todos.reduce((max, t) => Math.max(max, t.id), 0) + 1; return [...todos, { id, text, done: false }]; } diff --git a/test/cli.test.js b/test/cli.test.js index 4e73029..46ead6c 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -1,9 +1,9 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdtempSync, writeFileSync, readFileSync, existsSync } from "node:fs"; +import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; const CLI = fileURLToPath(new URL("../src/cli.js", import.meta.url)); @@ -36,6 +36,11 @@ test("add with no text fails without saving", () => { for (const [label, args, message] of [ ["missing id", [], /Invalid id: \(missing\)/], ["non-numeric id", ["abc"], /Invalid id: abc/], + ["hex id", ["0x1"], /Invalid id: 0x1/], + ["exponent id", ["1e0"], /Invalid id: 1e0/], + ["decimal id", ["1.0"], /Invalid id: 1\.0/], + ["padded id", [" 1 "], /Invalid id: 1 /], + ["empty id", [""], /Invalid id: \n/], ["unknown id", ["99"], /No todo with id 99/], ]) { for (const cmd of ["done", "rm"]) { @@ -59,6 +64,19 @@ test("unknown command prints usage even when the file is corrupt", () => { assert.match(res.stderr, /Unknown command: --help\nUsage:/); }); +test("a failed save gives a readable error instead of a stack trace", () => { + const file = tempFile(); + chmodSync(dirname(file), 0o500); + try { + const res = run(file, "add", "a"); + assert.equal(res.status, 1); + assert.match(res.stderr, /^Could not save /); + assert.doesNotMatch(res.stderr, /at .*\.js:\d+/); + } finally { + chmodSync(dirname(file), 0o700); + } +}); + test("a corrupt file gives a readable error instead of a stack trace", () => { const file = tempFile(); writeFileSync(file, "{ not json"); diff --git a/test/todo.test.js b/test/todo.test.js index 04beccd..1ddeea8 100644 --- a/test/todo.test.js +++ b/test/todo.test.js @@ -1,8 +1,8 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, writeFileSync, existsSync } from "node:fs"; +import { mkdtempSync, writeFileSync, readdirSync, symlinkSync, lstatSync, statSync, chmodSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, dirname } from "node:path"; import { load, save, add, has, complete, remove, format } from "../src/todo.js"; const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json"); @@ -42,12 +42,46 @@ test("load returns an empty list for a missing file", () => { assert.deepEqual(load(tempFile()), []); }); +test("add handles a very large list", () => { + const todos = Array.from({ length: 200_000 }, (_, i) => ({ id: i + 1, text: "x", done: false })); + assert.equal(add(todos, "y").at(-1).id, 200_001); +}); + test("save then load round-trips and leaves no temp file", () => { const file = tempFile(); const todos = add([], "a"); save(file, todos); assert.deepEqual(load(file), todos); - assert.equal(existsSync(`${file}.tmp`), false); + assert.deepEqual(readdirSync(dirname(file)), ["todos.json"]); +}); + +test("save writes through a symlink instead of replacing it", () => { + const real = tempFile(); + const link = join(dirname(tempFile()), "link.json"); + save(real, []); + symlinkSync(real, link); + save(link, add([], "a")); + assert.equal(lstatSync(link).isSymbolicLink(), true); + assert.deepEqual(load(real).map((t) => t.text), ["a"]); +}); + +test("save keeps the file's permissions", () => { + const file = tempFile(); + save(file, []); + chmodSync(file, 0o600); + save(file, add([], "a")); + assert.equal(statSync(file).mode & 0o777, 0o600); +}); + +test("save reports a readable error and cleans up when it can't write", () => { + const file = tempFile(); + chmodSync(dirname(file), 0o500); + try { + assert.throws(() => save(file, []), /Could not save .*todos\.json/); + assert.deepEqual(readdirSync(dirname(file)), []); + } finally { + chmodSync(dirname(file), 0o700); + } }); test("load rejects invalid JSON with a readable error", () => { @@ -61,3 +95,11 @@ test("load rejects JSON that is not a list", () => { writeFileSync(file, "{}"); assert.throws(() => load(file), /expected a list of todos/); }); + +for (const entry of ["null", "1", '{"text":"a","done":false}', '{"id":"1","text":"a","done":false}']) { + test(`load rejects an invalid entry: ${entry}`, () => { + const file = tempFile(); + writeFileSync(file, `[${entry}]`); + assert.throws(() => load(file), /entry 1 is not a valid todo/); + }); +} From 089fc94e16538c5def7ad4005f846c488e3acaaa Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:33:46 +0200 Subject: [PATCH 3/6] Address review round 2: duplicate ids, read-only files, dangling links, safe ids - load() rejects duplicate ids (left by the old add bug) and ids that aren't safe positive integers - save() refuses files the user can't write, and follows dangling symlinks to their target - CLI rejects ids beyond Number.MAX_SAFE_INTEGER - Skip permission-based tests when running as root Co-Authored-By: Claude Opus 5.5 --- src/cli.js | 2 +- src/todo.js | 34 ++++++++++++++++++++++++++++------ test/cli.test.js | 3 ++- test/todo.test.js | 36 ++++++++++++++++++++++++++++++++++-- 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/cli.js b/src/cli.js index bdcc4ba..9a0a914 100755 --- a/src/cli.js +++ b/src/cli.js @@ -11,8 +11,8 @@ function fail(message) { } function parseId(todos, arg) { - if (!/^\d+$/.test(arg ?? "")) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); const id = Number(arg); + if (!/^\d+$/.test(arg ?? "") || !Number.isSafeInteger(id)) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); if (!has(todos, id)) fail(`No todo with id ${id}`); return id; } diff --git a/src/todo.js b/src/todo.js index b1a4a65..3cb1518 100644 --- a/src/todo.js +++ b/src/todo.js @@ -1,8 +1,24 @@ -import { readFileSync, writeFileSync, renameSync, existsSync, realpathSync, statSync, chmodSync, rmSync } from "node:fs"; +import { + readFileSync, writeFileSync, renameSync, existsSync, lstatSync, readlinkSync, statSync, chmodSync, accessSync, rmSync, constants, +} from "node:fs"; +import { dirname, resolve } from "node:path"; import { randomBytes } from "node:crypto"; +// Ids are positive and stay below MAX_SAFE_INTEGER so add()'s max + 1 is always exact. +const isId = (id) => Number.isSafeInteger(id) && id > 0 && id < Number.MAX_SAFE_INTEGER; + const isTodo = (t) => - t !== null && typeof t === "object" && Number.isInteger(t.id) && typeof t.text === "string" && typeof t.done === "boolean"; + t !== null && typeof t === "object" && isId(t.id) && typeof t.text === "string" && typeof t.done === "boolean"; + +// Follow symlinks by hand so a dangling link resolves to the file it points at. +function resolveLinks(path) { + let target = path; + for (let hops = 0; lstatSync(target, { throwIfNoEntry: false })?.isSymbolicLink(); hops++) { + if (hops === 40) throw new Error("too many levels of symbolic links"); + target = resolve(dirname(target), readlinkSync(target)); + } + return target; +} export function load(path) { if (!existsSync(path)) return []; @@ -15,21 +31,27 @@ export function load(path) { if (!Array.isArray(todos)) throw new Error(`Could not read ${path}: expected a list of todos`); const bad = todos.findIndex((t) => !isTodo(t)); if (bad !== -1) throw new Error(`Could not read ${path}: entry ${bad + 1} is not a valid todo`); + const seen = new Set(); + const dup = todos.find((t) => seen.size === seen.add(t.id).size); + if (dup) throw new Error(`Could not read ${path}: id ${dup.id} is used more than once; renumber it by hand`); return todos; } // Write a uniquely named temp file next to the real file (following symlinks), copy over // its permissions, then rename it into place, so an interrupted write never truncates it. +// A file the user can't write (e.g. chmod 444) is refused, as a plain write would be. export function save(path, todos) { - const exists = existsSync(path); - const target = exists ? realpathSync(path) : path; - const tmp = `${target}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`; + let tmp; try { + const target = resolveLinks(path); + const exists = existsSync(target); + if (exists) accessSync(target, constants.W_OK); + tmp = `${target}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`; writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n"); if (exists) chmodSync(tmp, statSync(target).mode); renameSync(tmp, target); } catch (err) { - rmSync(tmp, { force: true }); + if (tmp) rmSync(tmp, { force: true }); throw new Error(`Could not save ${path}: ${err.message}`); } } diff --git a/test/cli.test.js b/test/cli.test.js index 46ead6c..350b05a 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -41,6 +41,7 @@ for (const [label, args, message] of [ ["decimal id", ["1.0"], /Invalid id: 1\.0/], ["padded id", [" 1 "], /Invalid id: 1 /], ["empty id", [""], /Invalid id: \n/], + ["unsafe id", ["99999999999999999999"], /Invalid id: 99999999999999999999/], ["unknown id", ["99"], /No todo with id 99/], ]) { for (const cmd of ["done", "rm"]) { @@ -64,7 +65,7 @@ test("unknown command prints usage even when the file is corrupt", () => { assert.match(res.stderr, /Unknown command: --help\nUsage:/); }); -test("a failed save gives a readable error instead of a stack trace", () => { +test("a failed save gives a readable error instead of a stack trace", { skip: process.getuid?.() === 0 && "root ignores file permissions" }, () => { const file = tempFile(); chmodSync(dirname(file), 0o500); try { diff --git a/test/todo.test.js b/test/todo.test.js index 1ddeea8..ea23327 100644 --- a/test/todo.test.js +++ b/test/todo.test.js @@ -6,6 +6,7 @@ import { join, dirname } from "node:path"; import { load, save, add, has, complete, remove, format } from "../src/todo.js"; const tempFile = () => join(mkdtempSync(join(tmpdir(), "todo-")), "todos.json"); +const isRoot = process.getuid?.() === 0; test("add appends an open todo", () => { const todos = add([], "buy milk"); @@ -65,6 +66,29 @@ test("save writes through a symlink instead of replacing it", () => { assert.deepEqual(load(real).map((t) => t.text), ["a"]); }); +test("load rejects duplicate ids left by the old add() bug", () => { + const file = tempFile(); + writeFileSync(file, JSON.stringify([{ id: 2, text: "b", done: false }, { id: 2, text: "c", done: false }])); + assert.throws(() => load(file), /id 2 is used more than once/); +}); + +test("save through a dangling symlink creates the target and keeps the link", () => { + const dir = dirname(tempFile()); + const link = join(dir, "link.json"); + symlinkSync("real.json", link); + save(link, add([], "a")); + assert.equal(lstatSync(link).isSymbolicLink(), true); + assert.deepEqual(load(join(dir, "real.json")).map((t) => t.text), ["a"]); +}); + +test("save refuses a read-only file", { skip: isRoot && "root ignores file permissions" }, () => { + const file = tempFile(); + save(file, []); + chmodSync(file, 0o444); + assert.throws(() => save(file, add([], "a")), /Could not save .*todos\.json/); + assert.deepEqual(load(file), []); +}); + test("save keeps the file's permissions", () => { const file = tempFile(); save(file, []); @@ -73,7 +97,7 @@ test("save keeps the file's permissions", () => { assert.equal(statSync(file).mode & 0o777, 0o600); }); -test("save reports a readable error and cleans up when it can't write", () => { +test("save reports a readable error and cleans up when it can't write", { skip: isRoot && "root ignores file permissions" }, () => { const file = tempFile(); chmodSync(dirname(file), 0o500); try { @@ -96,7 +120,15 @@ test("load rejects JSON that is not a list", () => { assert.throws(() => load(file), /expected a list of todos/); }); -for (const entry of ["null", "1", '{"text":"a","done":false}', '{"id":"1","text":"a","done":false}']) { +for (const entry of [ + "null", + "1", + '{"text":"a","done":false}', + '{"id":"1","text":"a","done":false}', + '{"id":0,"text":"a","done":false}', + '{"id":-1,"text":"a","done":false}', + `{"id":${Number.MAX_SAFE_INTEGER},"text":"a","done":false}`, +]) { test(`load rejects an invalid entry: ${entry}`, () => { const file = tempFile(); writeFileSync(file, `[${entry}]`); From f09d20cb846eab09a3c6db843e600ec7f2415135 Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:41:37 +0200 Subject: [PATCH 4/6] Address review round 3: id limit, private temp files, canonical ids - add() refuses to create an id that load() would reject; the CLI reports it - save() creates the temp file with the real file's mode (and O_EXCL), so a private list is never briefly world-readable; one stat call; symlink resolution inlined; comment states the rename trade-offs - CLI rejects ids with leading zeros and id 0 - Tests for trimming, the bare 'todo' default, and the id limit Co-Authored-By: Claude Opus 5.5 --- src/cli.js | 8 ++++++-- src/todo.js | 38 +++++++++++++++++++------------------- test/cli.test.js | 26 ++++++++++++++++++++++++++ test/todo.test.js | 4 ++++ 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/cli.js b/src/cli.js index 9a0a914..638c469 100755 --- a/src/cli.js +++ b/src/cli.js @@ -12,7 +12,7 @@ function fail(message) { function parseId(todos, arg) { const id = Number(arg); - if (!/^\d+$/.test(arg ?? "") || !Number.isSafeInteger(id)) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); + if (!/^[1-9]\d*$/.test(arg ?? "") || !Number.isSafeInteger(id)) fail(`Invalid id: ${arg ?? "(missing)"}\n${USAGE}`); if (!has(todos, id)) fail(`No todo with id ${id}`); return id; } @@ -30,7 +30,11 @@ switch (cmd) { case "add": { const text = args.join(" ").trim(); if (!text) fail(`Missing todo text\n${USAGE}`); - todos = add(todos, text); + try { + todos = add(todos, text); + } catch (err) { + fail(err.message); + } break; } case "done": diff --git a/src/todo.js b/src/todo.js index 3cb1518..5d3f754 100644 --- a/src/todo.js +++ b/src/todo.js @@ -4,22 +4,12 @@ import { import { dirname, resolve } from "node:path"; import { randomBytes } from "node:crypto"; -// Ids are positive and stay below MAX_SAFE_INTEGER so add()'s max + 1 is always exact. +// Ids are positive and below MAX_SAFE_INTEGER, so max + 1 in add() is always exact. const isId = (id) => Number.isSafeInteger(id) && id > 0 && id < Number.MAX_SAFE_INTEGER; const isTodo = (t) => t !== null && typeof t === "object" && isId(t.id) && typeof t.text === "string" && typeof t.done === "boolean"; -// Follow symlinks by hand so a dangling link resolves to the file it points at. -function resolveLinks(path) { - let target = path; - for (let hops = 0; lstatSync(target, { throwIfNoEntry: false })?.isSymbolicLink(); hops++) { - if (hops === 40) throw new Error("too many levels of symbolic links"); - target = resolve(dirname(target), readlinkSync(target)); - } - return target; -} - export function load(path) { if (!existsSync(path)) return []; let todos; @@ -37,18 +27,27 @@ export function load(path) { return todos; } -// Write a uniquely named temp file next to the real file (following symlinks), copy over -// its permissions, then rename it into place, so an interrupted write never truncates it. -// A file the user can't write (e.g. chmod 444) is refused, as a plain write would be. +// Atomic save: write a uniquely named temp file next to the real file, then rename it into +// place, so an interrupted write never truncates the list. +// - Symlinks are followed by hand, so a dangling link gets its target created, not replaced. +// - The temp file is created with the real file's permissions, so a private list is never +// readable by others, even mid-write. A file the user can't write (chmod 444) is refused. +// - Trade-offs of renaming: the directory must be writable, hard links to the old file are +// not updated, and the new file is owned by whoever ran the save. export function save(path, todos) { let tmp; try { - const target = resolveLinks(path); - const exists = existsSync(target); - if (exists) accessSync(target, constants.W_OK); + let target = path; + for (let hops = 0; lstatSync(target, { throwIfNoEntry: false })?.isSymbolicLink(); hops++) { + if (hops === 40) throw new Error("too many levels of symbolic links"); + target = resolve(dirname(target), readlinkSync(target)); + } + const stat = statSync(target, { throwIfNoEntry: false }); + if (stat) accessSync(target, constants.W_OK); + const mode = stat ? stat.mode & 0o777 : 0o666; tmp = `${target}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`; - writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n"); - if (exists) chmodSync(tmp, statSync(target).mode); + writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n", { mode, flag: "wx" }); + if (stat) chmodSync(tmp, mode); // undo the umask, so the mode matches exactly renameSync(tmp, target); } catch (err) { if (tmp) rmSync(tmp, { force: true }); @@ -58,6 +57,7 @@ export function save(path, todos) { export function add(todos, text) { const id = todos.reduce((max, t) => Math.max(max, t.id), 0) + 1; + if (!isId(id)) throw new Error("No ids left: the highest id is already the largest allowed"); return [...todos, { id, text, done: false }]; } diff --git a/test/cli.test.js b/test/cli.test.js index 350b05a..82d60d2 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -25,6 +25,30 @@ test("ls with no file prints the empty message and writes nothing", () => { assert.equal(existsSync(file), false); }); +test("no command lists the todos", () => { + const file = tempFile(); + run(file, "add", "a"); + const res = run(file); + assert.equal(res.status, 0); + assert.equal(res.stdout.trim(), "[ ] 1. a"); +}); + +test("add trims surrounding whitespace from the text", () => { + const file = tempFile(); + run(file, "add", " buy", "milk "); + assert.deepEqual(JSON.parse(readFileSync(file, "utf8")), [{ id: 1, text: "buy milk", done: false }]); +}); + +test("add fails readably when no ids are left, without saving", () => { + const file = tempFile(); + const before = JSON.stringify([{ id: Number.MAX_SAFE_INTEGER - 1, text: "a", done: false }]); + writeFileSync(file, before); + const res = run(file, "add", "b"); + assert.equal(res.status, 1); + assert.match(res.stderr, /^No ids left/); + assert.equal(readFileSync(file, "utf8"), before); +}); + test("add with no text fails without saving", () => { const file = tempFile(); const res = run(file, "add", " "); @@ -42,6 +66,8 @@ for (const [label, args, message] of [ ["padded id", [" 1 "], /Invalid id: 1 /], ["empty id", [""], /Invalid id: \n/], ["unsafe id", ["99999999999999999999"], /Invalid id: 99999999999999999999/], + ["leading-zero id", ["01"], /Invalid id: 01/], + ["zero id", ["0"], /Invalid id: 0/], ["unknown id", ["99"], /No todo with id 99/], ]) { for (const cmd of ["done", "rm"]) { diff --git a/test/todo.test.js b/test/todo.test.js index ea23327..ff67b4d 100644 --- a/test/todo.test.js +++ b/test/todo.test.js @@ -43,6 +43,10 @@ test("load returns an empty list for a missing file", () => { assert.deepEqual(load(tempFile()), []); }); +test("add refuses an id that load() would reject", () => { + assert.throws(() => add([{ id: Number.MAX_SAFE_INTEGER - 1, text: "a", done: false }], "b"), /No ids left/); +}); + test("add handles a very large list", () => { const todos = Array.from({ length: 200_000 }, (_, i) => ({ id: i + 1, text: "x", done: false })); assert.equal(add(todos, "y").at(-1).id, 200_001); From dab492da1e249aeccc73dc02b1437b2fca5771f4 Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:49:47 +0200 Subject: [PATCH 5/6] Lock during updates, repair duplicate ids, keep file identity on save - CLI holds a lock file from load to save, so concurrent commands can't lose an update; stale locks from crashed runs are taken over - load() renumbers duplicate ids from the old add() bug instead of refusing the file - save() keeps owner, group and setgid on the temp file, and falls back to an in-place write for hard-linked files, files whose owner/group can't be kept, and directories that aren't writable Co-Authored-By: Claude Opus 5.5 --- .gitignore | 1 + src/cli.js | 23 +++++++++++++++++++++ src/todo.js | 51 +++++++++++++++++++++++++++++++++++++---------- test/cli.test.js | 36 +++++++++++++++++++++++++++++++-- test/todo.test.js | 34 +++++++++++++++++++++++++++---- 5 files changed, 128 insertions(+), 17 deletions(-) diff --git a/.gitignore b/.gitignore index 605e1fb..0c537b5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ node_modules/ todos.json *.tmp +*.lock .DS_Store # Personal Claude Code overrides (not shared) diff --git a/src/cli.js b/src/cli.js index 638c469..92902d0 100755 --- a/src/cli.js +++ b/src/cli.js @@ -1,4 +1,5 @@ #!/usr/bin/env node +import { openSync, closeSync, statSync, rmSync } from "node:fs"; import { load, save, add, has, complete, remove, format } from "./todo.js"; const FILE = process.env.TODO_FILE ?? "todos.json"; @@ -17,7 +18,29 @@ function parseId(todos, arg) { return id; } +// Hold a lock file from load to save, so two commands running at once can't lose an update. +// A lock older than 10s is left over from a crashed run and is taken over. +function lock(path) { + const lockPath = `${path}.lock`; + const deadline = Date.now() + 5000; + for (;;) { + try { + closeSync(openSync(lockPath, "wx")); + process.on("exit", () => rmSync(lockPath, { force: true })); + return; + } catch (err) { + if (err.code === "EACCES" || err.code === "EPERM") return; // read-only directory: can't lock + if (err.code !== "EEXIST") fail(`Could not lock ${path}: ${err.message}`); + } + const mtime = statSync(lockPath, { throwIfNoEntry: false })?.mtimeMs; + if (mtime !== undefined && Date.now() - mtime > 10_000) rmSync(lockPath, { force: true }); + else if (Date.now() > deadline) fail(`${path} is busy: another todo command holds ${lockPath} (delete it if none is running)`); + else Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 25); + } +} + if (!["ls", "add", "done", "rm"].includes(cmd)) fail(`Unknown command: ${cmd}\n${USAGE}`); +if (cmd !== "ls") lock(FILE); let todos; try { diff --git a/src/todo.js b/src/todo.js index 5d3f754..267188e 100644 --- a/src/todo.js +++ b/src/todo.js @@ -1,5 +1,6 @@ import { - readFileSync, writeFileSync, renameSync, existsSync, lstatSync, readlinkSync, statSync, chmodSync, accessSync, rmSync, constants, + readFileSync, writeFileSync, renameSync, existsSync, lstatSync, readlinkSync, statSync, chmodSync, chownSync, accessSync, rmSync, + constants, } from "node:fs"; import { dirname, resolve } from "node:path"; import { randomBytes } from "node:crypto"; @@ -21,20 +22,30 @@ export function load(path) { if (!Array.isArray(todos)) throw new Error(`Could not read ${path}: expected a list of todos`); const bad = todos.findIndex((t) => !isTodo(t)); if (bad !== -1) throw new Error(`Could not read ${path}: entry ${bad + 1} is not a valid todo`); + // Older versions could give two todos the same id. Keep the first and renumber the rest + // after the current highest id, so every todo can be targeted on its own again. const seen = new Set(); - const dup = todos.find((t) => seen.size === seen.add(t.id).size); - if (dup) throw new Error(`Could not read ${path}: id ${dup.id} is used more than once; renumber it by hand`); - return todos; + let next = todos.reduce((max, t) => Math.max(max, t.id), 0); + return todos.map((t) => { + if (!seen.has(t.id)) { + seen.add(t.id); + return t; + } + if (!isId(++next)) throw new Error(`Could not read ${path}: no ids left to renumber duplicate id ${t.id}`); + return { ...t, id: next }; + }); } // Atomic save: write a uniquely named temp file next to the real file, then rename it into // place, so an interrupted write never truncates the list. // - Symlinks are followed by hand, so a dangling link gets its target created, not replaced. -// - The temp file is created with the real file's permissions, so a private list is never -// readable by others, even mid-write. A file the user can't write (chmod 444) is refused. -// - Trade-offs of renaming: the directory must be writable, hard links to the old file are -// not updated, and the new file is owned by whoever ran the save. +// - The temp file gets the real file's permissions from the start (so a private list is never +// readable by others mid-write) and its owner and group. +// - A file the user can't write (chmod 444) is refused, as a plain write would be. +// - Where a rename can't keep the file's identity (other hard links, an owner or group we +// can't assign, a directory we can't write), it falls back to a plain in-place write. export function save(path, todos) { + const data = JSON.stringify(todos, null, 2) + "\n"; let tmp; try { let target = path; @@ -44,10 +55,28 @@ export function save(path, todos) { } const stat = statSync(target, { throwIfNoEntry: false }); if (stat) accessSync(target, constants.W_OK); - const mode = stat ? stat.mode & 0o777 : 0o666; + const writeInPlace = () => { + if (tmp) rmSync(tmp, { force: true }); + tmp = undefined; + writeFileSync(target, data); + }; + if (stat?.nlink > 1) return writeInPlace(); + const mode = stat ? stat.mode & 0o7777 : 0o666; tmp = `${target}.${process.pid}.${randomBytes(4).toString("hex")}.tmp`; - writeFileSync(tmp, JSON.stringify(todos, null, 2) + "\n", { mode, flag: "wx" }); - if (stat) chmodSync(tmp, mode); // undo the umask, so the mode matches exactly + try { + writeFileSync(tmp, data, { mode, flag: "wx" }); + } catch (err) { + if (stat && (err.code === "EACCES" || err.code === "EPERM")) return writeInPlace(); + throw err; + } + if (stat) { + try { + chownSync(tmp, stat.uid, stat.gid); + } catch { + return writeInPlace(); + } + chmodSync(tmp, mode); // after chown, which can clear setgid; also undoes the umask + } renameSync(tmp, target); } catch (err) { if (tmp) rmSync(tmp, { force: true }); diff --git a/test/cli.test.js b/test/cli.test.js index 82d60d2..3a8d8e1 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -1,7 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; -import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync } from "node:fs"; +import { spawnSync, spawn } from "node:child_process"; +import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync, utimesSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; @@ -49,6 +49,38 @@ test("add fails readably when no ids are left, without saving", () => { assert.equal(readFileSync(file, "utf8"), before); }); +test("concurrent adds don't lose updates or reuse ids", async () => { + const file = tempFile(); + const runs = Array.from({ length: 12 }, (_, i) => + new Promise((done) => { + const child = spawn(process.execPath, [CLI, "add", `t${i}`], { env: { ...process.env, TODO_FILE: file } }); + child.on("exit", done); + })); + assert.deepEqual(await Promise.all(runs), Array(12).fill(0)); + const todos = JSON.parse(readFileSync(file, "utf8")); + assert.equal(todos.length, 12); + assert.deepEqual(todos.map((t) => t.id).sort((a, b) => a - b), Array.from({ length: 12 }, (_, i) => i + 1)); + assert.equal(existsSync(`${file}.lock`), false); +}); + +test("a stale lock from a crashed run is taken over", () => { + const file = tempFile(); + writeFileSync(`${file}.lock`, ""); + const old = new Date(Date.now() - 60_000); + utimesSync(`${file}.lock`, old, old); + const res = run(file, "add", "a"); + assert.equal(res.status, 0, res.stderr); + assert.equal(existsSync(`${file}.lock`), false); +}); + +test("rm on a file with duplicate ids removes only the first of them", () => { + const file = tempFile(); + writeFileSync(file, JSON.stringify([{ id: 2, text: "b", done: false }, { id: 2, text: "c", done: false }])); + const res = run(file, "rm", "2"); + assert.equal(res.status, 0, res.stderr); + assert.equal(res.stdout.trim(), "[ ] 3. c"); +}); + test("add with no text fails without saving", () => { const file = tempFile(); const res = run(file, "add", " "); diff --git a/test/todo.test.js b/test/todo.test.js index ff67b4d..af2cfd7 100644 --- a/test/todo.test.js +++ b/test/todo.test.js @@ -1,6 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, writeFileSync, readdirSync, symlinkSync, lstatSync, statSync, chmodSync } from "node:fs"; +import { mkdtempSync, writeFileSync, readdirSync, symlinkSync, linkSync, lstatSync, statSync, chmodSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, dirname } from "node:path"; import { load, save, add, has, complete, remove, format } from "../src/todo.js"; @@ -70,10 +70,36 @@ test("save writes through a symlink instead of replacing it", () => { assert.deepEqual(load(real).map((t) => t.text), ["a"]); }); -test("load rejects duplicate ids left by the old add() bug", () => { +test("load renumbers duplicate ids left by the old add() bug", () => { const file = tempFile(); - writeFileSync(file, JSON.stringify([{ id: 2, text: "b", done: false }, { id: 2, text: "c", done: false }])); - assert.throws(() => load(file), /id 2 is used more than once/); + writeFileSync(file, JSON.stringify([ + { id: 2, text: "b", done: false }, + { id: 2, text: "c", done: true }, + { id: 3, text: "d", done: false }, + ])); + assert.deepEqual(load(file).map((t) => [t.id, t.text, t.done]), [[2, "b", false], [4, "c", true], [3, "d", false]]); +}); + +test("save writes in place when the file has other hard links", () => { + const file = tempFile(); + const other = join(dirname(file), "other.json"); + save(file, []); + linkSync(file, other); + save(file, add([], "a")); + assert.deepEqual(load(other).map((t) => t.text), ["a"]); +}); + +test("save writes in place when only the file, not its directory, is writable", { skip: isRoot && "root ignores file permissions" }, () => { + const file = tempFile(); + save(file, []); + chmodSync(dirname(file), 0o500); + try { + save(file, add([], "a")); + assert.deepEqual(load(file).map((t) => t.text), ["a"]); + assert.deepEqual(readdirSync(dirname(file)), ["todos.json"]); + } finally { + chmodSync(dirname(file), 0o700); + } }); test("save through a dangling symlink creates the target and keeps the link", () => { From 130ffe0d13d54504847cb40a1e658929c863a5af Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:56:16 +0200 Subject: [PATCH 6/6] Drop the lock file: too much machinery for a personal CLI Concurrent runs of the CLI can again lose an update; saves stay atomic. Co-Authored-By: Claude Opus 5.5 --- .gitignore | 1 - src/cli.js | 23 ----------------------- test/cli.test.js | 28 ++-------------------------- 3 files changed, 2 insertions(+), 50 deletions(-) diff --git a/.gitignore b/.gitignore index 0c537b5..605e1fb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,6 @@ node_modules/ todos.json *.tmp -*.lock .DS_Store # Personal Claude Code overrides (not shared) diff --git a/src/cli.js b/src/cli.js index 92902d0..638c469 100755 --- a/src/cli.js +++ b/src/cli.js @@ -1,5 +1,4 @@ #!/usr/bin/env node -import { openSync, closeSync, statSync, rmSync } from "node:fs"; import { load, save, add, has, complete, remove, format } from "./todo.js"; const FILE = process.env.TODO_FILE ?? "todos.json"; @@ -18,29 +17,7 @@ function parseId(todos, arg) { return id; } -// Hold a lock file from load to save, so two commands running at once can't lose an update. -// A lock older than 10s is left over from a crashed run and is taken over. -function lock(path) { - const lockPath = `${path}.lock`; - const deadline = Date.now() + 5000; - for (;;) { - try { - closeSync(openSync(lockPath, "wx")); - process.on("exit", () => rmSync(lockPath, { force: true })); - return; - } catch (err) { - if (err.code === "EACCES" || err.code === "EPERM") return; // read-only directory: can't lock - if (err.code !== "EEXIST") fail(`Could not lock ${path}: ${err.message}`); - } - const mtime = statSync(lockPath, { throwIfNoEntry: false })?.mtimeMs; - if (mtime !== undefined && Date.now() - mtime > 10_000) rmSync(lockPath, { force: true }); - else if (Date.now() > deadline) fail(`${path} is busy: another todo command holds ${lockPath} (delete it if none is running)`); - else Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 25); - } -} - if (!["ls", "add", "done", "rm"].includes(cmd)) fail(`Unknown command: ${cmd}\n${USAGE}`); -if (cmd !== "ls") lock(FILE); let todos; try { diff --git a/test/cli.test.js b/test/cli.test.js index 3a8d8e1..9d9d5b8 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -1,7 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { spawnSync, spawn } from "node:child_process"; -import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync, utimesSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; @@ -49,30 +49,6 @@ test("add fails readably when no ids are left, without saving", () => { assert.equal(readFileSync(file, "utf8"), before); }); -test("concurrent adds don't lose updates or reuse ids", async () => { - const file = tempFile(); - const runs = Array.from({ length: 12 }, (_, i) => - new Promise((done) => { - const child = spawn(process.execPath, [CLI, "add", `t${i}`], { env: { ...process.env, TODO_FILE: file } }); - child.on("exit", done); - })); - assert.deepEqual(await Promise.all(runs), Array(12).fill(0)); - const todos = JSON.parse(readFileSync(file, "utf8")); - assert.equal(todos.length, 12); - assert.deepEqual(todos.map((t) => t.id).sort((a, b) => a - b), Array.from({ length: 12 }, (_, i) => i + 1)); - assert.equal(existsSync(`${file}.lock`), false); -}); - -test("a stale lock from a crashed run is taken over", () => { - const file = tempFile(); - writeFileSync(`${file}.lock`, ""); - const old = new Date(Date.now() - 60_000); - utimesSync(`${file}.lock`, old, old); - const res = run(file, "add", "a"); - assert.equal(res.status, 0, res.stderr); - assert.equal(existsSync(`${file}.lock`), false); -}); - test("rm on a file with duplicate ids removes only the first of them", () => { const file = tempFile(); writeFileSync(file, JSON.stringify([{ id: 2, text: "b", done: false }, { id: 2, text: "c", done: false }]));