fix(settings-local): never steal writer locks

This commit is contained in:
Tianyi Cui
2026-07-31 01:14:43 +08:00
parent f9f8148e79
commit afb05b4049
9 changed files with 36 additions and 75 deletions

View File

@@ -2,5 +2,5 @@
# side as of the last confirmed-consistent state. Both languages carry equal authority;
# after editing either side, bring the other along and re-record with:
# pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-30-settings-write-path-integrity.md
2026-07-30-settings-write-path-integrity.md: 07bd095162879c8e7866846cf562f6a13307e5fc
2026-07-30-settings-write-path-integrity.zh.md: 5d02177073d482b61750d7bdfbbd0866bc227a6a
2026-07-30-settings-write-path-integrity.md: f6b39ebc323e635945d2eae049c0d29e94c390b6
2026-07-30-settings-write-path-integrity.zh.md: de60cc05751893d1875f2a68a8b357c5572940ad

View File

@@ -14,7 +14,7 @@ Review found the provider's write path could destroy state it never observed, an
**One operation chain, and every write is a read-modify-write.** Watcher refreshes and persists from every namespace queue share a single settled chain, and `persistSection` begins by reconciling the on-disk text into the seam — publishing any unobserved difference first — before rendering against that fresh text. A write can no longer resurrect a stale document, and an on-disk document that turned invalid fails the write loud rather than being overwritten (the reload path keeps its warn-and-keep-last-good policy; the shared `reconcileFromDisk` throws and each caller picks its policy). The watcher's `ready` signal queues one extra reconcile, closing the startup gap between the initial load and the watcher becoming active.
**Writes hold a `wx`-created `<file>.lock` sibling.** The read-render-rename cycle runs under a cross-process writer lock with exponential backoff, a 2 s acquisition deadline, and stale takeover after 5 s (a crashed holder, broken with a warning). Readers never lock — the rename commit is atomic — so contention is writer-only and resolves in milliseconds. The lock constants are protocol invariants, not config: a holder rewrites one small document, so the deadline and stale age derive from that bound, not from deployment taste.
**Writes hold a `wx`-created `<file>.lock` sibling.** The read-render-rename cycle runs under a cross-process writer lock with exponential backoff and a 2 s acquisition deadline. Readers never lock because rename commits atomically. A contender never removes a lock it did not create: age cannot distinguish an abandoned lock from a slow live holder, and deleting by age can also remove a successor acquired between inspection and deletion. Contention therefore rejects at the deadline, leaving an abandoned lock for explicit operator recovery.
**Observer disposal is quiescent.** Watchers carry an `active` flag checked when a queued invocation would start, so a disposer that ran while the invocation waited prevents the start entirely; started invocations register in a service-level `pendingTails` set that the dispose drain awaits beside the write queues. The `settings/updated` fan-out contains a returned thenable's rejection through the same listener diagnostic as a sync throw, and the event contract now states that the `INVARIANT` rethrow serves synchronous listeners only — invariant companions must stay sync, which the shipped companion already is.
@@ -24,7 +24,8 @@ Review found the provider's write path could destroy state it never observed, an
## Alternatives considered
- **`proper-lockfile` instead of a hand-rolled lock** — the dependency-over-hand-rolling policy was weighed: the library is barely maintained, its stale/retry policy is broader than this one-file protocol needs, and the shipped lock is ~40 lines with deterministic tests (including injected `EEXIST`/`stat` races). The policy favors dependencies that delete owned code; this one would replace 40 explained lines with an opaque peer.
- **`proper-lockfile` instead of a hand-rolled lock** — the dependency-over-hand-rolling policy was weighed: the library is barely maintained, its stale/retry policy is broader than this one-file protocol needs, and the shipped lock is a small exclusive-create/backoff loop with deterministic tests. The policy favors dependencies that delete owned code; this one would replace explained local behavior with an opaque peer.
- **Age-based stale-lock takeover** — age is not ownership. A slow holder may legitimately cross the threshold, and an inspector can delete a successor's newly acquired lock after the old holder releases. Failing closed preserves mutual exclusion; recovery is explicit because only the operator can establish that no writer remains.
- **Revision/CAS instead of a lock** — rename cannot express compare-and-swap, so a CAS needs a version sidecar or content re-hash and a retry loop in every writer; the lock achieves the same serialization with one primitive and keeps readers free.
- **Merging external edits into the in-flight write's own section** — the seam merges patches over the state visible at call time, so a same-namespace external edit racing a write still resolves last-write-wins; folding it in would need three-way merge semantics no consumer has asked for. The write publishes the external state first, so the loser is at least observed before being superseded.
- **Declaring async `settings/updated` listeners unsupported** — the typed signature is `void` and lint flags misused promises, but an unlinted JS plugin can still register an async listener; a contract note cannot un-throw an unhandled rejection, so containment is the only defense that holds at runtime.
@@ -32,4 +33,4 @@ Review found the provider's write path could destroy state it never observed, an
## Consequences
`update()` gained a documented failure mode (lock deadline, invalid on-disk document) and the rejection messages carry `$`-rooted paths. Remaining, documented in the provider README: same-namespace concurrent edits stay last-write-wins (no per-value merge or revision check), a watcher event the OS never delivers leaves the cache stale until the next signal or write, and comments inside replaced arrays or attached inline to changed scalar values go with the value they described. The [user-settings seam note](2026-07-28-user-settings-seam.md)'s deferred-lockfile alternative is superseded by this note. The same defect classes exist in `dsh-credentials-local` (two chains over one `.env`, cached whole-file write-back, post-persist emit) and in the `llm/adapters-updated` fan-out on the stacked branches; those fixes belong to the PRs that introduce the packages and follow this template on merge-up.
`update()` has documented failure modes for lock-deadline expiry and an invalid on-disk document, and rejection messages carry `$`-rooted paths. An abandoned lock blocks writes until an operator confirms that no writer owns it and removes the sidecar. Remaining, documented in the provider README: same-namespace concurrent edits stay last-write-wins (no per-value merge or revision check), a watcher event the OS never delivers leaves the cache stale until the next signal or write, and comments inside replaced arrays or attached inline to changed scalar values go with the value they described. The [user-settings seam note](2026-07-28-user-settings-seam.md)'s deferred-lockfile alternative is superseded by this note.

View File

@@ -18,7 +18,7 @@ YAML 写入则整体替换 namespace 节点,把分节内的每条注释都删
**单一操作链,且每次写入都是读-改-写。**watcher 的刷新与来自各 namespace 队列的持久化共享同一条结算链;`persistSection` 会先把磁盘上的文本对账进 seam——任何未被观察到的差异都先发布出去——然后才对照这份新鲜文本渲染。写入不再可能复活一份陈旧文档磁盘上已变非法的文档会让写入响亮失败而不是被覆盖重载路径保持其“告警并保留最后可用值”策略共享的 `reconcileFromDisk` 抛错各调用方自选策略。watcher 的 `ready` 信号会额外排入一次对账,弥合初始加载与 watcher 生效之间的启动缺口。
**写入持有以 `wx` 创建的同目录 `<file>.lock`。**读-渲染-rename 循环在一把跨进程写锁下运行指数退避2 s 获取截止时间、5 s 后陈旧接管(持有者已崩溃;打破旧锁时给出告警)。读方从不加锁——rename 提交是原子的——因此竞争只发生在写方之间,毫秒级即可化解。锁的各项常量是协议不变式,不是配置:持有者只是重写一份小文档,截止时间与陈旧时限都从这一上界推得,而非出自部署偏好
**写入持有以 `wx` 创建的同目录 `<file>.lock`。**读-渲染-rename 循环在一把跨进程写锁下运行,采用指数退避2 s 获取期限。读方从不加锁,因为 rename 会原子提交。竞争者绝不移除并非由自己创建的锁:锁龄无法区分遗留锁与仍存活的慢速持有者,按锁龄删除还可能移除检查与删除之间由后继者取得的新锁。因此,竞争会在期限到达时拒绝写入,把遗留锁留给操作者显式恢复
**观察者 dispose 达到完全停稳。**watcher 携带一个 `active` 标志,排队的调用即将启动时先检查它,因此在调用等待期间已经运行过的释放器能让这次启动彻底不发生;已启动的调用会登记进服务级的 `pendingTails` 集合dispose 排空除了等待各写队列,还会等待该集合。`settings/updated` 扇出会把监听器返回的 thenable 的 rejection 收容进与同步抛错相同的监听器诊断;事件契约现已写明 `INVARIANT` 重抛只服务同步监听器——不变式配套插件必须保持同步,而已交付的那个配套插件本就是同步的。
@@ -28,7 +28,8 @@ YAML 写入则整体替换 namespace 节点,把分节内的每条注释都删
## 曾考虑的替代方案
- **用 `proper-lockfile` 取代手写锁**——按“依赖优先于手写”政策做过权衡:该库几乎无人维护,其陈旧/重试策略比这个单文件协议所需的更宽泛,而已交付的锁约 40 行并带确定性测试(含注入的 `EEXIST`/`stat` 竞态)。该政策偏向能删除自有代码的依赖;这个依赖只会把 40 行带解释的代码换成一个不透明的等价物。
- **用 `proper-lockfile` 取代手写锁**——按“依赖优先于手写”政策做过权衡:该库几乎无人维护,其陈旧/重试策略比这个单文件协议所需的更宽泛,而已交付的锁只是一个较小的独占创建/退避循环,带确定性测试。该政策偏向能删除自有代码的依赖;这个依赖只会把解释清楚的本地行为换成一个不透明的等价物。
- **按锁龄接管陈旧锁**——锁龄不等于所有权。慢速持有者可能合理地跨过阈值,而旧持有者释放后,检查方还可能删除后继者新取得的锁。以失败收口可以保住互斥性;恢复必须显式进行,因为只有操作者才能确认已无写入方存活。
- **用修订号/CAS 取代锁**——rename 表达不了 compare-and-swap因此 CAS 需要一个版本伴随文件或内容重哈希,外加每个写方里的一个重试循环;锁用一个原语实现同样的串行化,还让读方完全免锁。
- **把外部编辑合并进正在进行的写入自身的分节**——seam 是在调用时刻可见的状态之上合并 patch 的,因此与写入竞态的同 namespace 外部编辑仍按后写胜出解决;要把外部编辑并进来,需要三方合并语义,而没有任何消费方提出过这种需求。写入会先发布外部状态,落败一方至少在被取代之前被观察到。
- **宣布不支持异步 `settings/updated` 监听器**——类型签名是 `void`lint 也会标记误用的 promise但未经 lint 的 JS 插件仍能注册异步监听器;契约里的一句说明无法收回已经抛出的 unhandled rejection收容是唯一在运行时守得住的防线。
@@ -36,6 +37,4 @@ YAML 写入则整体替换 namespace 节点,把分节内的每条注释都删
## 后果
`update()` 有了成文的失败模式(锁截止时间到期、磁盘文档非法)rejection 消息携带以 `$` 为根的路径。仍然存在、且已记录在提供方 README 中的有:同 namespace 并发编辑仍是后写胜出没有逐值合并也没有修订号检查OS 从未投递的 watcher 事件会让缓存保持陈旧,直到下一个信号或下一次写入;被替换数组内部的注释、以及行内附着在被改标量值上的注释,会随其描述的值一起消失。
[用户设置 seam note](2026-07-28-user-settings-seam.md)里“延后锁文件”那条替代方案已被本 note 取代。同类缺陷还存在于 `dsh-credentials-local`(两条链共用一个 `.env`、按缓存整文件写回、持久化之后才发事件)与堆叠分支上的 `llm/adapters-updated` 扇出这些修复归引入相应包package的那些 PRPull Request所有向上合并时按本模板处理。
`update()` 对锁获取期限到达与磁盘文档非法都有成文的失败模式rejection 消息携带以 `$` 为根的路径。遗留锁会阻塞写入,直到操作者确认没有写入方拥有它并移除伴随文件。仍然存在、且已记录在提供方 README 中的有:同 namespace 并发编辑仍是后写胜出没有逐值合并也没有修订号检查OS 从未投递的 watcher 事件会让缓存保持陈旧,直到下一个信号或下一次写入;被替换数组内部的注释、以及行内附着在被改标量值上的注释,会随其描述的值一起消失。[用户设置 seam note](2026-07-28-user-settings-seam.md)里“延后锁文件”那条替代方案已被本 note 取代。