feat(runtime-host): add durable local deployment ownership - #3767
Conversation
6b202fe to
fd08688
Compare
|
The only failing check on this head is an unrelated Desktop E2E assertion:
This stack changes only Runtime Host operator ownership code. I also attempted to rerun the failed job, but GitHub requires repository admin permission. A maintainer rerun should distinguish the existing Desktop flake from a reproducible regression. 中文摘要当前唯一失败项是无关的 Desktop 菜单刷新 E2E;其余 Desktop E2E 为 64 passed / 1 skipped。本分支只修改 Runtime Host operator ownership。我的权限不能重跑该 job,请维护者重跑以区分已有偶发失败与可复现回归。 |
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this head and found blocking issues.
[P2] Default resolver ignores real process.env, allowing two authority roots
The resolver defaults to options.env?.XDG_DATA_HOME but production calls omit options, so implicit and explicit process.env resolve to different record directories and both claims return applied for the same rootId.
[P2] First-create authority directory not durably published
Recursive mkdir for Maka/runtime-host-ownership is not fsynced at parent entries, so a crash can make a returned applied claim disappear on restart, allowing a second claim.
[P3] Crashed temp owner-record files accumulate
Killed writer before rename leaves ${rootId}.json.*.tmp without recovery.
Checks on fd08688cce are test: failure (unrelated E2E 64 pass / 1 fail).
简体中文
存在权威根定位与持久化两项阻断,另有一项清理 P3。382b24d to
804f4b6
Compare
|
Addressed all three findings on
Verification on this head:
The dependent #3769 commit was rebased onto this head; its combined owner/transfer focused suite is 20 passed. |
804f4b6 to
4a3efbe
Compare
|
Addressed the new P1/P2/P3 findings on
Verification:
|
4a3efbe to
e16e9f8
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Update on e16e9f8139:
Previous 1×P1 + 1×P2 + 1×P3 are now closed. However a new blocking issue was found:
[P2] Non-regular record file can hang the reader indefinitely
local-deployment-owner.ts:527 uses blocking open(O_RDONLY|O_NOFOLLOW) before stat() checking isFile(). A FIFO at that path blocks the open forever, so the check is never reached and readLocalHostDeploymentRecord() hangs, blocking further owner operations under lock. Repro: mkfifo <rootId>.json hangs 500ms vs normal file returns quickly. Fix: use O_NONBLOCK and fstat with non-regular rejection.
Checks on e16e9f8139 are test: queued — code is NO-GO independent of CI.
简体中文
存在 FIFO 阻塞导致的永久挂起。e16e9f8 to
4d12c44
Compare
|
Confirmed and fixed the FIFO blocking issue on The read path now:
The second and third steps preserve protection against a regular-file-to-FIFO substitution between path inspection and open. The regression fixture creates a real FIFO. It also carries a delayed nonblocking writer solely to let the old blocking implementation fail with a bounded duration instead of hanging the test process forever. On the fixed path the FIFO is rejected as Verification:
|
Astro-Han
left a comment
There was a problem hiding this comment.
Update on 4d12c444ae:
No blocking issues. The previous P2 (non-regular record file hanging reader) is now closed via O_NONBLOCK + descriptor stat with regular-file rejection, covering FIFO, TOCTOU, and concurrent release. Prior authority/durability findings remain closed.
Checks on 4d12c444ae are test: queued — code GO, awaiting green.
简体中文
该头未发现阻断。
M4n5ter
left a comment
There was a problem hiding this comment.
Review on 4d12c444ae.
The overall owner/CAS/state-machine design looks sound, and I did not find other merge-relevant issues in transition ordering, normal cross-process serialization, stable-record reads, UTF-8/schema validation, or temporary-file recovery. I found one blocking durability issue and two important issues:
Blocking: retry can report success without re-establishing durability
writeRecord() and removeRecord() correctly return commit_unknown when rename/unlink has completed but the authority-directory fsync fails. An exact retry then observes the desired record (or absence), reduces to unchanged, and returns without another directory sync. This can convert a page-cache observation into successful convergence even though a later crash may still lose the rename or resurrect the released owner.
preparePrivateDirectory() has the same recovery gap: it syncs a parent only when this invocation's mkdir reports created. If a process creates a directory and exits/fails before syncing its parent, a retry or concurrent process treats that visible directory—or an EEXIST result—as already durable and may later return applied while an ancestor entry remains unconfirmed.
The minimal root fix seems to be making durability confirmation itself idempotent:
- before returning a successful
unchanged, sync the authority directory under the same record lock, preservingcommit_unknownif that still fails; - during directory preparation, sync the parent of the first existing boundary and every traversed entry regardless of whether
mkdirreturned success orEEXIST; - fault-inject the first directory sync after
mkdir,rename, andunlink, and require the retry to remain unsuccessful until the barrier succeeds.
The dependent #3769 recovery path should also reapply the exact transition (or call an explicit durability-confirm operation); read + match alone cannot confirm the preceding namespace mutation.
Important: validated caller-owned objects remain mutable across I/O
assertTransition() parses nested values but discards Zod's cloned result. After the first await, the reducer still consumes the original caller-owned transition object. A caller can therefore mutate a shared owner or selected alias while directory/lock I/O is pending, causing the function to persist values that were never validated. An oversized/control-character value can produce a durable record that the next read rejects as invalid_record; later mutation can also make the returned record differ from the serialized bytes.
Please construct a complete canonical parsed clone before any await and use only that snapshot thereafter. A regression test can mutate a shared alias immediately after invocation and assert that both disk and the returned record retain the invocation-time values.
Important: the new suite does not currently provide valid Windows qualification
The resolver test injects platform: 'linux' / 'darwin' while using host-native node:path and hard-coded POSIX expectations, so the test is deterministically broken on a Windows runner. Its only win32 assertion passes authorityRoot, takes the early return, and never exercises the default Windows path. The required CI is Ubuntu-only, and the existing Windows lanes do not run this owner suite, so the green check does not qualify native Windows path, replace/release, or independent-process claim behavior.
Please either make the platform override select path.posix / path.win32, or remove the cross-platform override and use native-platform assertions, then run the focused owner suite in a Windows lane (and preferably macOS as well). I did not find a confirmed production-default Windows bug; the issue here is the broken test/override contract and missing native evidence. The repository's documented Windows limitation around parent-directory power-loss durability remains a qualification boundary rather than a separate finding in this PR.
The simplify audit found no additional merge-gating complexity issue.
简体中文
在 4d12c444ae 上完成复审。
整体 owner/CAS/状态机设计是合理的;在 transition 顺序、正常跨进程串行化、稳定记录读取、UTF-8/schema 校验和临时文件恢复方面,没有发现其他会影响合并的问题。当前有一个 Blocking 和两个 Important:
Blocking:重试可能在未重新建立持久性时返回成功
writeRecord() / removeRecord() 在 rename / unlink 已完成、但 authority 目录 fsync 失败时会正确返回 commit_unknown。然而精确重试观察到目标记录或记录缺失后会归约为 unchanged,并且不再同步目录。这会把 page-cache 中可见的状态升级为成功;后续崩溃仍可能丢失 rename,或让已 release 的旧 owner 复活。
preparePrivateDirectory() 有同一类恢复缺口:只有本次 mkdir 报告 created 时才同步 parent。如果某进程创建目录后在同步 parent 前退出/失败,重试或并发进程会把当前可见目录或 EEXIST 当作已经持久化,之后可能返回 applied,但某一级 ancestor entry 仍未得到确认。
建议让 durability confirmation 本身可幂等重试:
- 在同一 record lock 内,
unchanged成功返回前同步 authority 目录;若仍失败,继续保持commit_unknown; - 目录准备时同步首个既存边界的 parent,并对每一级目录都同步 parent,不区分
mkdir成功还是EEXIST; - 故障注入
mkdir、rename、unlink后的第一次目录同步失败,确认重试只有在 barrier 成功后才能返回成功。
依赖 PR #3769 的恢复路径也需要重放精确 transition,或调用显式 durability-confirm 操作;只做 read + match 不能确认先前的 namespace mutation 已持久化。
Important:校验后的调用方对象仍可在 I/O 期间被修改
assertTransition() 虽然解析了嵌套字段,却丢弃了 Zod 生成的 clone。第一次 await 后,reducer 仍使用调用方原始 transition。调用方可以在目录/锁 I/O 等待期间修改共享的 owner 或 selected alias,导致未经过校验的值被持久化。超长或含控制字符的值会生成下一次读取必然报 invalid_record 的 durable record;稍后的修改还可能让返回的 record 与落盘 bytes 不一致。
应在任何 await 前构造完整、canonical 的 parsed clone,之后只使用该快照。回归测试可在调用后立即修改共享 alias,并断言磁盘与返回值都保持调用时的值。
Important:新增测试目前无法提供有效的 Windows 资格证据
Resolver 测试注入 platform: 'linux' / 'darwin',但仍使用 host-native node:path 并硬编码 POSIX 结果,因此在 Windows runner 上必然失败。唯一的 win32 断言又传入 authorityRoot,提前返回,完全没有覆盖 Windows 默认路径。Required CI 只有 Ubuntu,现有 Windows lane 也不运行该 owner suite,因此绿色 check 不能证明原生 Windows path、replace/release 或独立进程 claim 行为。
建议让 platform override 同时选择 path.posix / path.win32,或者删除跨平台 override、改用原生平台断言;随后至少在 Windows lane 运行 focused owner suite,最好也包含 macOS。没有发现已确认的 Windows production-default bug;这里的问题是测试/override contract 已损坏且缺少 native evidence。仓库已经记录的 Windows parent-directory 断电持久性限制属于 qualification boundary,不作为本 PR 的另一个 finding。
Simplify audit 没有发现额外的 merge-gating complexity 问题。
b584b1d to
3b0a50b
Compare
|
@M4n5ter Addressed the review on
The remaining Linux 简体中文已修复本轮三项问题:所有目录持久化 barrier 都可幂等重试, 依赖 PR #3769 也已重基,恢复逻辑改为重放精确 transition,不再把 当前 Linux 红灯与 current main 自身完全相同,来自 #3784 合并后的既有 Runtime Host 测试/API 编译漂移,不由本 PR 引入。 Generated-by: Codex |
M4n5ter
left a comment
There was a problem hiding this comment.
Re-review of 3b0a50b627 found one Important issue. The previous durability, mutable-input, and cross-platform findings are resolved, and I found no other Blocking/Important issues.
Important — Do not sync above the account-home durability boundary
When the default ~/.local/... hierarchy is missing, preparePrivateDirectory() stops at the existing account home and then syncs dirname(accountHome). On a valid Linux layout such as /home with mode 0711, the account can traverse to its own home but cannot open('/home', 'r').
I reproduced this path returning commit_unknown with an EACCES cause before .local is created. Every retry reaches the same barrier, so owner claims can never succeed for affected accounts.
Please treat accountHome as the stable OS-managed boundary: skip syncing its parent when it is the first existing candidate, while continuing to sync parents for .local and every descendant. This preserves recovery when a prior invocation created .local but failed to sync the home directory. A POSIX regression with an execute-only parent directory would cover the case.
中文
复审 3b0a50b627 后发现一个 Important 问题。此前的持久化重试、可变输入及跨平台问题均已解决,除此之外未发现其他 Blocking/Important 问题。
Important — 不应越过 account home 的持久化边界执行同步
默认的 ~/.local/... 层级尚不存在时,preparePrivateDirectory() 会停在已存在的 account home,随后同步 dirname(accountHome)。在 /home 权限为 0711 这类合法 Linux 布局中,账户可以 traverse 到自己的 home,但不能以只读方式打开 /home。
我直接复现了该路径:在创建 .local 前返回 commit_unknown,底层原因为 EACCES。后续每次重试都会命中同一个 barrier,因此受影响账户永远无法完成 owner claim。
建议将 accountHome 明确定义为由 OS 管理的稳定边界:当它是首个既存 candidate 时,不同步其 parent;但 .local 及以下目录仍逐级同步 parent。这样不会破坏“前一次创建 .local 后同步 home 失败”的恢复保证。建议增加一个 parent 仅有 execute 权限的 POSIX 回归测试。
Generated-by: Codex
3b0a50b to
70bd041
Compare
|
@M4n5ter Confirmed and fixed on The resolver now carries the normalized OS account home as an explicit durability boundary alongside the authority root. Directory preparation skips the parent sync only when the first existing candidate is exactly that boundary. It still syncs:
The POSIX regression creates an account home beneath an execute-only parent, first proves that Verification:
简体中文已确认并修复:account home 现在是明确的 OS-managed durability boundary。首次既存边界正好是 home 时,不再同步其 parent;但创建 POSIX 回归先证明 execute-only parent 无法只读打开并返回 Generated-by: Codex |
M4n5ter
left a comment
There was a problem hiding this comment.
Re-reviewed 70bd041860: the account-home durability-boundary finding is resolved. I found no remaining Blocking/Important issues; correctness and design are acceptable.
Summary
ownedandtransferringstates so claim, same-owner selection, transfer commit/rollback, and release are idempotent and crash-observable.npxinvocations and ordinary remote Clients at the durable owner seam.This PR is the durable owner/transfer foundation only. It does not retire a Host, inspect or persist
HostEpoch/PID, launch a successor, infer owner authority from compatibility, or add Desktop/CLI/TUI/systemd/remote adapters.#3766 has merged. This branch is rebased onto its merge commit, so the diff against
maincontains only the durable owner/transfer foundation.Refs #3231
Design context: #3709
中文摘要
owned/transferring状态,使 claim、同 owner 选择、transfer commit/rollback 与 release 可幂等重试并可在崩溃后观察。npxinvocation 和普通 remote Client。本 PR 只提供 durable owner/transfer 基础。它不负责 Host retirement,不持久化
HostEpoch/PID,不启动 successor,不从 compatibility 推断 owner authority,也不增加 Desktop/CLI/TUI/systemd/remote adapter。依赖 #3766;在前置 PR 合并前,这个 stacked Draft 相对
main的 diff 会包含其第一笔 commit。Verification
npm --workspace @maka/runtime-host run buildgit diff --checkpassed.AI use
Select exactly one:
Tool(s) and scope: Codex helped design and implement the durable owner record, CAS transitions, persistence, and cross-process tests under user direction. The affected commit includes a
Generated-by: Codextrailer.Checklist
Does this PR entail a change in behavior?