From 130ffe0d13d54504847cb40a1e658929c863a5af Mon Sep 17 00:00:00 2001 From: christian-westerkam Date: Wed, 21 Oct 2026 00:56:16 +0200 Subject: [PATCH] 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 }]));