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 <noreply@anthropic.com>

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", "  ");

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", () => {