cxs is a local-first CLI for searching Codex session logs. It is designed for progressive retrieval: find the right session first, then read
docs(sync-lock): 标记 stale lock 清理为 best-effort mitigation
之前的 commit 187c5d9 在 tryRemoveStaleLock 加了二次比对,但这并不是
原子 TOCTOU 修复:在二次读取与 path-based rmSync 之间仍有残余 race
窗口,另一个进程可能在该窗口内删并替换 lock,导致当前进程仍可能
rmSync 别人的新 lock。要做真正原子需 OS-level flock 或 inode-pinned
unlink,代价不划算 — 接受 best-effort 表述。
- sync-lock.ts: 改写 tryRemoveStaleLock 注释,明确说明这是 mitigation
而非原子修复,以及为什么对 cxs 低并发场景可接受
- docs/CODE_QUALITY_REVIEW_2026-04-27.md: P0-3 标记为 ⚠️ best-effort
mitigation 并解释工程决策,避免后人以为已"彻底修复"
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Entire-Checkpoint: 6ed014a6a4bc
+15
-6
+6
-2
docs/CODE_QUALITY_REVIEW_2026-04-27.md
+6
-2
docs/CODE_QUALITY_REVIEW_2026-04-27.md
···
129
129
130
130
`getCurrentSessions()` 已经采用这种结构，其他函数应对齐。
131
131
132
-
### P0-3：sync lock stale 清理存在竞争窗口
132
+
### P0-3：sync lock stale 清理存在竞争窗口 ⚠️ best-effort mitigation
133
133
134
-
当前逻辑遇到已有 lock 后会读 lock info，判断 pid 不存在就删除 lock 文件。这里有一个竞态：读到 stale lock 之后、删除之前，另一个进程可能已经创建了新 lock；当前进程可能误删别人的新 lock。
134
+
> 状态：已在 commit `187c5d9` 部分缓解 — 引入 `tryRemoveStaleLock` 在删除前二次比对 `pid + createdAt`。但这是 best-effort,**不是原子 TOCTOU 修复**：在二次读取与 path-based `rmSync` 之间仍有残余 race 窗口,另一个进程可能在该窗口内删除并替换 lock,导致当前进程仍可能 `rmSync` 别人的新 lock。Node/Bun 无 inode-pinned unlink,要做真正原子需引入 OS-level flock(native bindings)。
135
+
>
136
+
> 工程决策：cxs 的 sync 是低并发异常路径,残余 race 窗口极窄,接受 best-effort 表述并在 `sync-lock.ts:tryRemoveStaleLock` 注释里明确标注。如未来观察到锁损坏,再考虑引入 flock 或换 rename-based 抓取。
137
+
138
+
原文（保留作为背景）：当前逻辑遇到已有 lock 后会读 lock info，判断 pid 不存在就删除 lock 文件。这里有一个竞态：读到 stale lock 之后、删除之前，另一个进程可能已经创建了新 lock；当前进程可能误删别人的新 lock。
135
139
136
140
建议删除前重新读取并比对 `pid + createdAt`，只删除自己刚判断过的那份 lock。
137
141
+9
-4
sync-lock.ts
+9
-4
sync-lock.ts
···
68
68
removeLockIfPresent(lockPath);
69
69
}
70
70
71
-
// Why: avoid TOCTOU when clearing a stale lock. Between our `existing` read
72
-
// and the actual `rmSync` call, another sync process can re-create the lock.
73
-
// Re-read and compare pid+createdAt: only delete the lock file if it's still
74
-
// the same one we judged dead. Exported for unit tests.
71
+
// Best-effort mitigation, NOT an atomic TOCTOU fix. The race window between
72
+
// our initial read and rmSync is narrowed (we re-read and bail if the lock
73
+
// no longer matches `expected`), but a residual window remains: between our
74
+
// re-read and the path-based rmSync, another process can still delete the
75
+
// stale lock and create a fresh one — we'd then unlink that fresh lock by
76
+
// path. node:fs has no inode-pinned unlink, and we don't take an OS-level
77
+
// flock, so this is the best we can do without native bindings. Acceptable
78
+
// for cxs's low-concurrency sync flow; revisit if we ever observe corruption.
79
+
// Exported for unit tests.
75
80
export function tryRemoveStaleLock(lockPath: string, expected: SyncLockInfo): boolean {
76
81
const reChecked = readLockInfo(lockPath);
77
82
if (!reChecked) return true;