Fix duplicate ids, validate CLI input, harden load/save #2
No files matched your search
@@ -1,7 +1,6 @@
|
|||||||
node_modules/
|
node_modules/
|
||||||
todos.json
|
todos.json
|
||||||
*.tmp
|
*.tmp
|
||||||
*.lock
|
|
||||||
.DS_Store
|
.DS_Store
|
||||||
|
|
||||||
# Personal Claude Code overrides (not shared)
|
# Personal Claude Code overrides (not shared)
|
||||||
|
|||||||
-23
@@ -1,5 +1,4 @@
|
|||||||
#!/usr/bin/env node
|
#!/usr/bin/env node
|
||||||
import { openSync, closeSync, statSync, rmSync } from "node:fs";
|
|
||||||
import { load, save, add, has, 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 FILE = process.env.TODO_FILE ?? "todos.json";
|
||||||
@@ -18,29 +17,7 @@ function parseId(todos, arg) {
|
|||||||
return id;
|
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 (!["ls", "add", "done", "rm"].includes(cmd)) fail(`Unknown command: ${cmd}\n${USAGE}`);
|
||||||
if (cmd !== "ls") lock(FILE);
|
|
||||||
|
|
||||||
let todos;
|
let todos;
|
||||||
try {
|
try {
|
||||||
|
|||||||
+2
-26
@@ -1,7 +1,7 @@
|
|||||||
import { test } from "node:test";
|
import { test } from "node:test";
|
||||||
import assert from "node:assert/strict";
|
import assert from "node:assert/strict";
|
||||||
import { spawnSync, spawn } from "node:child_process";
|
import { spawnSync } from "node:child_process";
|
||||||
import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync, utimesSync } from "node:fs";
|
import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync } from "node:fs";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { join, dirname } from "node:path";
|
import { join, dirname } from "node:path";
|
||||||
import { fileURLToPath } from "node:url";
|
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);
|
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", () => {
|
test("rm on a file with duplicate ids removes only the first of them", () => {
|
||||||
const file = tempFile();
|
const file = tempFile();
|
||||||
writeFileSync(file, JSON.stringify([{ id: 2, text: "b", done: false }, { id: 2, text: "c", done: false }]));
|
writeFileSync(file, JSON.stringify([{ id: 2, text: "b", done: false }, { id: 2, text: "c", done: false }]));
|
||||||
|
|||||||
Reference in new issue
Block a user