Replace the driver install lock primitive: directory-rename cannot make claim-and-verify atomic, and the inode guard is not portable
还没有人认领这个 Issue。
评估
- 难度
- 5/5
- 预计耗时
- 一周以上
- 新手友好度
- 35/100
- Issue 类型
- 重构
- 描述清晰度
- 描述清楚
- 活跃度
- 活跃
- 技术栈
- typescript
- 领域
- tooling
调研方向
先从 packages/drivers/src/resolve.ts 中的 withInstallLock、claimStaleLock、isStaleLock、releaseInstallLock、installLockPath 和 heldLockPath 开始,然后阅读 packages/drivers/test/install-lock.test.ts 以及与 #1201 相关的行为。使用所提议的 O_EXCL sentinel 方向,并加入多进程控制。完成意味着测试覆盖获取、声明过期 lock、身份安全的释放,以及已识别的竞争缺口,并且不依赖 inode 比较。
由索引模型根据 Issue 内容生成。
描述
Summary
The cross-process driver install lock added in #1201 uses a lock directory plus rename as its primitive, with an inode comparison covering one window the ownership token cannot. Three independent reviewers, across three review rounds, converged on the same structural conclusion: that primitive cannot make "confirm this is still the lock I judged stale" and "claim it" a single atomic step, and the inode fallback rests on a guarantee that is not portable.
This issue tracks replacing the primitive. It is deliberately not a bug report against #1201 — that PR is a large net improvement over main, where there is no cross-process lock at all — but it should not be mistaken for a proof of mutual exclusion, and the remaining gaps should not be patched further in place.
Why replace rather than repair
Three rounds of fixes on this protocol each closed real races and then exposed new ones in the compensating branches themselves:
- Round 1 added the lock. Review found stale cleanup could delete a peer's fresh lock.
- Round 2 made the claim a
rename(atomic, one winner) and added a pre-check plus a post-rename verify-and-restore. Review found the restore branch leaves the lock pathname free, so a third contender can take it, the restore fails, and the moved live lock is deleted — admitting two owners. - Round 2 also added an inode check for the
mkdir→owner.jsonwindow the token cannot cover. Review foundst_inois not a portable identity.
That pattern — each compensating branch generating the next finding — is the signature of a protocol being asked for a guarantee it cannot give, rather than of a fixable bug.
The specific gaps
1. Claim is not atomic with the staleness verdict. Nothing makes "read the owner record" and "rename the directory" one operation. The current code narrows the window from both sides (re-read before, verify what was actually moved after, restore on mismatch) but cannot close it.
2. The restore branch opens its own window. Between the mismatched rename and the restore, the lock pathname is unoccupied and a third process can create it. The restore then fails and the moved live lock is deleted.
3. Inode identity is not portable. fs.statSync(lockDir).ino is a per-filesystem identity. On Windows (untested on that PR), on overlay and network filesystems, and where a deleted directory's inode is promptly reused, the comparison can always-match (deleting a successor's live lock) or never-match (leaking our own). The failure is silent either way, and it degrades to exactly the pathname deletion it was added to prevent.
4. The lock wait outlasts only one peer. Each process counts its deadline from its own start, so with three or more simultaneous contenders the third's deadline can expire mid-install and it falls through to an unlocked performInstall over the same tree.
Proposed direction
An O_EXCL sentinel file whose content identifies the acquisition, replacing both the directory rename and the inode check:
open(path, "wx")is atomic and failsEEXISTwhen held — the same portable exclusion the lock directory gives — but the file has content, so the acquisition identity travels with the lock itself.- Claiming a stale lock becomes read-identity-then-conditionally-replace against a single object, rather than a verdict about one path followed by a rename of another.
- Release compares the identity in the file, so no inode comparison is needed and the
mkdir→owner.jsonwindow disappears: the identity is written by the same atomic operation that takes the lock.
Deriving the lock wait from the number of observed contenders, or re-checking readiness rather than falling through on timeout, should be considered alongside it (gap 4).
This wants its own PR, its own tests — including a multi-process control that demonstrates the assertion bites, as #1201's does — and its own review. It should not be appended to #1201.
Evidence
Six review threads on #1201, from three independent reviewers, all reaching this conclusion:
| Reviewer | Finding | Thread |
|---|---|---|
| codex | Block new acquisitions while restoring a stale-lock claim | https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888887305 |
| kilo-code | releaseInstallLock's inode check relies on an unverified cross-platform guarantee |
https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888899280 |
| cubic | Two acquisitions look identical when a stale lock has no readable owner.json |
https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888905549 |
| cubic | Restoration rename can fail; the catch then deletes the moved live lock | https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888905551 |
| cubic | Release safety depends on a platform-dependent, unverified inode premise | https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888905561 |
| codex | Do not use reusable inodes as lock ownership | https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888928250 |
Related but separate, also open on #1201 and not covered by this redesign:
- Lock wait outlasts only one peer (gap 4 above): https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888905558
- A killed CLI leaves npm running in a detached process group, so pid-based staleness reclaims a lock whose work is still mutating the tree: https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888887310
- Readiness is checked outside the cross-process lock, so a forced repair can race an observer: https://github.com/AltimateAI/altimate-code/pull/1201#discussion_r3888887308
Scope
withInstallLock,claimStaleLock,isStaleLock,releaseInstallLock,installLockPath/heldLockPathinpackages/drivers/src/resolve.tspackages/drivers/test/install-lock.test.ts- The
external_directorypermission patterns inpackages/opencode/src/altimate/tools/warehouse-install-driver.ts, if the sentinel changes what paths are written
Relates to #1202.
- 主要语言
- TypeScript
- 星标
- 813
- 派生
- 134
- 平均合并
- 2 天 3 小时
- 30 天内合并 PR
- 65
贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
AltimateAI/altimate-code 的其他 Issue
-
难度 2/5 1-3 小时 新手友好度 70/100
AltimateAI/altimate-code#1359 ·
-
难度 2/5 1-3 小时 新手友好度 84/100
AltimateAI/altimate-code#1323 ·
-
难度 2/5 1-3 小时 新手友好度 86/100
AltimateAI/altimate-code#1288 ·
-
难度 1/5 1 小时以内 新手友好度 92/100
AltimateAI/altimate-code#1285 ·
-
privacy: Altimate Base consent dialog no longer discloses persistent per-installation identifier 未关闭
难度 1/5 1 小时以内 新手友好度 88/100
AltimateAI/altimate-code#1284 ·
查看 AltimateAI/altimate-code 的全部 Issue
相似的 Issue
-
bug(cli): hapi doctor inline-media prints a fabricated B:\ helper-script path in packaged installs 未关闭
难度 2/5 1-3 小时 新手友好度 70/100
-
Crush 未关闭
难度 1/5 1 小时以内 新手友好度 85/100
catppuccin/catppuccin#3125 ·
-
难度 1/5 1 小时以内 新手友好度 90/100
ElementsProject/cln-application#167 · 1 条评论 · 1 个 reaction ·
-
难度 2/5 1-3 小时 新手友好度 75/100
Quantco/pnpm-licenses#17 ·
-
难度 2/5 1-3 小时 新手友好度 75/100