fix(usage-log): canonicalize queue key to prevent concurrent writes via path aliases

writeQueues was keyed by the raw workspaceDir-derived path before any
realpath resolution.  Two callers using different spellings of the same
physical directory (a symlink and its target, or a relative vs absolute
path) therefore produced separate queue entries and both entered
appendRecord concurrently.

Inside appendRecord, withFileLock calls resolveNormalizedFilePath which
uses fs.realpath on the directory; both spellings resolve to the same
normalised path.  If one chain is already in fn() — its entry set in
HELD_LOCKS — the second chain's acquireFileLock sees HELD_LOCKS hit for
the same normalised path and re-entrantly joins it.  Both callbacks then
execute the read-modify-write cycle concurrently, and whichever writes
last overwrites the first, silently dropping one entry per collision.

Fix: call fs.realpath(memoryDir) immediately after fs.mkdir and use the
canonical path as both the writeQueues key and the appendRecord file
argument.  A single canonical key means all in-process writers for the
same physical file are serialised through one queue regardless of how
the workspace path was spelled by the caller.

Test: symlink tmpDir to a second name and interleave concurrent
recordTokenUsage calls across both spellings.  Asserts all N records
survive — regression guard for the path-alias queue split.
This commit is contained in:
jiarung 2026-03-15 08:07:32 +00:00
parent 9944231ff4
commit 9f05b36834
2 changed files with 39 additions and 1 deletions

View File

@ -215,6 +215,36 @@ describe("recordTokenUsage", () => {
expect(lockExists).toBe(false);
});
it("different path spellings for the same workspace share one queue — no record is lost", async () => {
// Symlink tmpDir → another name so the same physical directory has two
// spellings. Without queue-key canonicalisation both spellings create
// independent writeQueues entries; when one chain holds the file lock
// (HELD_LOCKS set) the other re-entrantly joins it and both execute the
// read-modify-write cycle concurrently, silently dropping entries.
const symlinkDir = `${tmpDir}-symlink`;
await fs.symlink(tmpDir, symlinkDir);
try {
// Mix canonical and symlink paths across concurrent writes.
const N = 6;
await Promise.all(
Array.from({ length: N }, (_, i) =>
recordTokenUsage({
workspaceDir: i % 2 === 0 ? tmpDir : symlinkDir,
label: "llm_output",
usage: { input: i + 1, output: 1, total: i + 2 },
}),
),
);
// All N records must survive — none may be lost to a concurrent
// read-modify-write collision.
const records = JSON.parse(await fs.readFile(usageFile, "utf-8"));
expect(records).toHaveLength(N);
} finally {
await fs.unlink(symlinkDir).catch(() => {});
}
});
it("serialises concurrent writes — no record is lost", async () => {
const N = 20;
await Promise.all(

View File

@ -129,8 +129,16 @@ export async function recordTokenUsage(params: {
}
const memoryDir = path.join(params.workspaceDir, "memory");
const file = path.join(memoryDir, "token-usage.json");
await fs.mkdir(memoryDir, { recursive: true });
// Canonicalize before keying writeQueues so that different path spellings
// for the same physical directory (e.g. a symlink vs its target) share a
// single in-process queue. Without this, two spellings produce separate
// queue entries and both call appendRecord concurrently; when
// withFileLock's HELD_LOCKS map then resolves both to the same normalised
// path the second caller re-entrantly joins the first — allowing concurrent
// read-modify-write cycles that silently drop entries.
const realMemoryDir = await fs.realpath(memoryDir).catch(() => memoryDir);
const file = path.join(realMemoryDir, "token-usage.json");
const entry: TokenUsageRecord = {
id: makeId(),