refactor(runtime-host)!: make the Runtime Host State Root self-contained

[Astro-Han]

Summary

Runtime Host coordination previously derived from the OS account home: the owner lock lived under the account data directory, the control namespace and Host credentials under the account cache, and the managed deployment record under the account data root. Accounts without a usable home (nobody, HOME=/tmp, a passwd home of /nonexistent) could not start a Runtime Host, and cleaning the account cache destroyed issued access credentials and disconnected locks from the root they protected.

This PR moves the whole coordination surface inside the physical root and bumps the root marker to schema version 2:

• <root>/.maka-host/<rootId>.lock is the single owner lock; .maka-host/artifact-writer-bootstrap.lock is the single Artifact-writer lock.
• .maka-host/state/data holds plugin state, plugin credentials, access credentials and composition; .maka-host/state/deployment/runtime-host-deployment.json is the sole managed deployment transaction record.
• .maka-host/runtime holds disposable registration, startup diagnostics and one-time credential deliveries.
• The account side keeps only a locator (root-location.json) plus service-manager bookkeeping; a missing locator is a discovery failure, not data loss.

Invariants preserved: one Host writer per physical root, rootId stays protocol identity, marker device/inode checks still reject copied directories, no clone support and no global duplicate-identity registry.

prepareRuntimeHostRoot is the single Host-side format admission point. Ordinary startup, activation, update, managed-service launch and deployment cleanup all pass through it, so a schema-1 root migrates automatically and an interrupted migration resumes forward from its durable fence without manual repair. All schema-1 knowledge is clustered in root-upgrade.ts behind inspectStorageRootFormat/withStorageRootUpgrade so the removal below is mechanical.

Refs #5320 (the privileged account-home workaround this replaces), #4712 (the account-cache control namespace this deletes), #1286.

Migration

Schema 2 is a destructive format change: older binaries cannot open a migrated root. Roots written by v0.1.x, cli-v0.1.0-beta.1, v0.2.0-incubating-rc1 or v0.2.0-dev builds migrate automatically on first admission under 0.2.x. Schema-1 support is limited to the 0.2.x line — starting with 0.3 the migration machinery is removed and unmigrated roots are rejected; a root that skips every 0.2.x release must be opened once by a 0.2.x build or restored from backup. Downgrading requires restoring a pre-upgrade copy of the root directory.

Verification

• npm --workspace builds for storage, runtime, runtime-host, cli (and dependents) — clean.
• Focused compiled suites via node --test --test-concurrency=4: storage root-authority (33 pass / 3 platform skips), runtime-host upgrade + two-client + managed + websocket batches, cli service-manager/lifecycle/activation/setup/update batches — all green, including the new interruption-fence matrix, the service-entry migration regression test and the deferred-cleanup-receipt test.
• Docker nobody (passwd home /nonexistent) harness: root resolution, owner acquisition, control-directory preparation, reader contention and second-owner refusal all pass.
• npm run format / npm run lint clean; python3 -m unittest harbor relay tests pass.
• docs/windows-test-inventory.md regenerated; the upgrade suites are added to the windows-recovery lane, which has not run here (no local Windows).
• Not run: full-repository test suite (out of scope per repo rules), Windows-native execution beyond the CI lane, release-to-release upgrade against published packages.

AI use

• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex authored the original candidate-branch commits (Generated-by trailers preserved); Devin rebased the branch onto main, added the namespace flattening and fixed-name bootstrap lock, the service/cleanup format-admission work, the relay workaround removal, the Windows lane update and the new regression tests (Generated-by: Devin trailers).

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above

[jackwener]

Independent agent review — first pass, focused on what blocks. Reviewed at f6dae5f93c2bf80d2b4b1931eee0f4e05fc6a98f. I am an AI agent (executing seat @kabi-opus) publishing through a shared GitHub account; this is an automated review and does not substitute for independent human review. Other lines from different operators are reviewing this PR in parallel; we do not share conclusions.

Two P1s, both inline, both caused by this PR and both blocking. Three of the fifteen checks are red and none of the three is intermittent — I traced each to a cause in this diff.

The three red checks are two defects

package and windows_recovery share one root cause: a union widened here without updating the exhaustive locale map that consumes it, so @maka/desktop does not compile. test is separate: the workflow now executes a cli suite in a lane whose contents are pinned by a policy test that exists to keep that lane off ordinary merges.

They interact, which is worth knowing: test dies at the CI planner step before the Desktop build, so on Linux the compile error is never reached. Fixing the lane question will expose the build failure on every lane rather than only the Windows ones.

What I checked beyond the red, and what held

The claim I most wanted to test is the one the Migration section rests on — that prepareRuntimeHostRoot is the single format admission point and nothing else writes to an unmigrated root. On the paths I sampled it holds:

• connect-or-spawn.ts:369 refuses a legacy root that carries a managed deployment record with managed_root_requires_operator before any preparation, leaving takeover to the managed lifecycle; otherwise it goes through prepareRuntimeHostRoot.
• managed-deployment.ts:654 is format-aware rather than migrating: upgrading returns early, legacy reads the deployment record from the old account-side location, schema 2 from inside the root. That matches the architecture doc's "deployment lookup does not migrate".
• Production callers of prepareRuntimeHostRoot cover update, lifecycle/deployment, activation and setup, which lines up with the paths the summary names.

I sampled rather than exhausted this: 113 files, and with the tree not compiling a deeper audit is premature. I have not verified interruption-fence forward recovery, idempotence of a resumed migration, or the downgrade failure mode — those are the questions I would go to next, and I am flagging them as unexamined rather than clean.

Not covered by me

Windows, entirely — I have no Windows machine, and both Windows lanes are red. The Desktop build failure I confirmed by reading the type and grepping the locale file at this head, not by compiling, because this machine cannot build @maka/ui (@astryxdesign/core 0.5.2 installed against a tree requiring 0.6.1) and the volume is full. I also did not run any suite for this PR.

Given the PR is a draft with the tree in this state, I would rather return for a proper migration-safety pass once it compiles than spend the review on a tree that does not.

简体中文

独立 agent 评审 —— 第一轮,只针对挡路的问题。 在 f6dae5f93c2bf80d2b4b1931eee0f4e05fc6a98f 上评审。我是自动化 agent,不能替代人类独立评审;另有不同 operator 的评审线在并行看这个 PR,结论互不转述。

两条 P1,都在行内,都由本 PR 造成且都挡住合并。 15 项检查中 3 项红,没有一项是间歇性的 —— 我把每一条都追到了本 diff 里的成因。

三条红其实是两个缺陷:package 与 windows_recovery 同源 —— 这里加宽了一个联合类型,却没更新消费它的穷尽语言映射,导致 @maka/desktop 编译不过;test 是另一件事 —— 工作流现在在一条内容被策略测试钉死的通道里执行 cli 套件,而那条策略存在的目的正是让该通道不要出现在普通合并上。
两者还互相影响:test 在 CI planner 那一步就挂,早于 Desktop 构建,所以 Linux 侧根本走不到那个编译错误。把通道问题解决后,构建失败会在所有通道上暴露,而不只是 Windows 两条。

红之外我核了什么、什么站住了:最想验的是 Migration 一节所依赖的那个主张 —— prepareRuntimeHostRoot 是唯一的格式准入点、没有别的地方会写未迁移的 root。在我抽查的路径上它成立:connect-or-spawn.ts:369 对带托管部署记录的 legacy root 直接以 managed_root_requires_operator 拒绝,把接管留给托管生命周期,否则一律走 prepareRuntimeHostRoot;managed-deployment.ts:654 是格式感知而非迁移(upgrading 早退、legacy 从旧的账户侧位置读、schema 2 从 root 内读),与架构文档「deployment lookup 不迁移」一致;prepareRuntimeHostRoot 的生产调用方覆盖 update、lifecycle/deployment、activation、setup,与摘要点名的路径对得上。

这是抽查不是穷尽:113 个文件,而且在树都编译不过的状态下深入审为时过早。我没有验证中断 fence 的前滚恢复、重入迁移的幂等性、以及降级时的失败形态 —— 这三点是我接下来会去查的,现在把它们标为"未检查"而不是"没问题"。

未覆盖:Windows,完全没有 —— 我没有 Windows 机器,而两条 Windows 通道都是红的。Desktop 构建失败是我在这个 head 上读类型 + grep 语言文件确认的,不是编译出来的,因为本机构建不了 @maka/ui(装的是 @astryxdesign/core 0.5.2,而树要 0.6.1)且磁盘已满。本 PR 我也没有跑过任何套件。

鉴于这是 draft 且树处于当前状态,我更愿意等它能编译之后再回来做一次像样的迁移安全审查,而不是把这轮评审花在一棵编译不过的树上。

[jackwener]

[P1] Widening this union broke the Desktop build, and the string users would see for the new state was never written.

apps/desktop/src/renderer/locales/settings-projects-copy.ts:243 declares

managementError: Record<RuntimeHostServiceErrorCode | 'unknown', string>;

an exhaustive map over this union. Adding root_requires_migration therefore obliges every locale block to carry it. None do — I grepped that file at f6dae5f93c2bf80d2b4b1931eee0f4e05fc6a98f and the identifier appears zero times, against five managementError blocks. @maka/desktop build:main fails with TS2741 at lines 568, 906 and 1261 plus TS2322 at 1369, which is what takes down both package and windows_recovery.

Worth separating the two halves. The compile error is mechanical. The other half is not: this PR's headline is the schema-1 to schema-2 migration, and the sentence that tells a user their State Root needs migrating does not exist in any language. Whatever copy closes this has to be written, not generated.

On why it reached CI: your Verification lists builds for storage, runtime, runtime-host and cli — apps/desktop is not among them. On the Linux side test died at the CI planner step before it ever reached the Desktop build, so only the two Windows lanes surfaced it. That is also why the lane policy failure below matters beyond its own red: it masked this one.

简体中文

把这个联合类型加宽之后,Desktop 构建挂了,而这个新状态给用户看的那句话一句都没写。

apps/desktop/src/renderer/locales/settings-projects-copy.ts:243 把 managementError 声明成 Record<RuntimeHostServiceErrorCode | 'unknown', string> —— 对这个联合的穷尽映射。于是加入 root_requires_migration 就要求每个语言块都补上它。一个都没补:我在 f6dae5f93 上 grep 过,这个标识符在该文件里出现 0 次,而 managementError 块有 5 个。@maka/desktop 的 build:main 因此报 TS2741(568/906/1261 行)与 TS2322(1369 行),package 与 windows_recovery 两条红都源于此。

两半要分开看:编译错误是机械的;另一半不是 —— 本 PR 的主线就是 schema-1 到 schema-2 的迁移,而**"你的 State Root 需要迁移"这句话在任何语言里都不存在**。补这段文案需要人写,不是生成。

为什么会漏到 CI:你 Verification 里列的构建是 storage / runtime / runtime-host / cli,apps/desktop 不在其中;而 Linux 的 test 在 CI planner 那一步就挂了,根本没走到 Desktop 构建,所以只有两条 Windows 通道把它照出来。这也是下面那条通道策略问题的额外代价:它把这一条遮住了。

[Astro-Han]

Fixed in 62d24ba83 — root_requires_migration copy added to all three locale blocks in settings-projects-copy.ts (en: 'This State Root predates this version. Run an update or activation to migrate it.', zh-CN/zh-TW siblings hand-written, not generated); tsc -p tsconfig.main.json is clean for that file. And yes — the verification gap was the real defect here: apps/desktop was never in the built set, so the exhaustive Record did its job only in CI.

[jackwener]

[P1] These entries put the recovery lane back on ordinary merges, which ci-workflow-policy.test.mjs pins against on purpose.

scripts/ci-workflow-policy.test.mjs:704 derives the lane's executed workspaces from the packages/*/dist/ paths its steps run and asserts the set exactly:

assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']);

Line 269 of this workflow now runs packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js, so the set becomes ['cli', 'runtime', 'runtime-host', 'storage'] and test fails. The filter additions here are the other half of the same move.

The test states why it is pinned, and the reason is a cost argument rather than bookkeeping:

These suites are ordinary TypeScript, so test runs them on Linux on every pull request and fails first; naming their sources here only bought a second, slower red. Listing one again would put this lane back on most merges, so it fails here.

So this is a decision about which lane owns the upgrade suites, not a line to patch. Two coherent answers: keep the cli upgrade suites in test where they already run on Linux and leave this lane to the three workspaces it was narrowed to; or argue that these particular suites need real Windows and change the policy — in which case the assertion and the comment above it have to move with it, carrying the new rationale. What cannot hold is the current state, where the workflow and the policy that guards it disagree.

I have no view on which answer is right; it is a CI-cost tradeoff for whoever owns that lane.

简体中文

这些条目会让 recovery 通道重新在大多数合并上被触发,而 ci-workflow-policy.test.mjs 正是为此把它钉死的。

scripts/ci-workflow-policy.test.mjs:704 从该通道步骤实际运行的 packages/*/dist/ 路径推导出它执行的 workspace,并精确断言其集合为 ['runtime','runtime-host','storage']。本工作流第 269 行现在会运行 packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js,集合因此变成 ['cli','runtime','runtime-host','storage'],test 随之失败;这里新增的路径过滤条目是同一动作的另一半。

测试把钉死的理由写明了,而且是成本论证不是记账:这些套件是普通 TypeScript,test 每个 PR 都会在 Linux 上先跑先红;把它们的源码列进这条通道只买到"第二次、更慢的红",而且会让这条通道回到大多数合并上。

所以这是"升级套件归哪条通道"的决定,不是补一行能了的事。两种自洽的答案:要么把 cli 的升级套件留在 test(它们本来就在那儿跑),这条通道维持被收窄后的三个 workspace;要么论证这些套件确实需要真实 Windows,那就连同断言与其上方的注释一起改,把新理由写进去。不能成立的是当前状态:工作流和守护它的策略互相矛盾。

哪个答案对我没有立场 —— 这是那条通道的持有者要做的 CI 成本权衡。

[Astro-Han]

Resolved by moving the policy side in 62d24ba83: the assertion now admits cli and the comment above it carries the new rationale. These suites execute win32-branching production code — the task-launcher artifact gate and in-root lock semantics — that Linux test cannot reach; the 3/3 deterministic failure on real Windows reported below is the evidence they belong here. The lane's criterion is 'needs real Windows', not workspace bookkeeping.

[zhiiw]

Independent review. Conclusions bind to f6dae5f93c2bf80d2b4b1931eee0f4e05fc6a98f (still Draft; CI test / package / windows_recovery red on this head). I ran the parts CI could not: this review was done on a real Windows machine, the lane this PR extends but the author could not execute.

Verified locally (Windows 11, Node 24.18.1 — the version CI pins):

• packages/storage root-migration 1/1; packages/runtime-host root-upgrade 14/15; packages/cli runtime-host-root-upgrade 1/4 — details below.
• root-authority + artifact-writer-lock + managed-deployment suites: 43 pass / 2 fail / 11 platform-skipped; the 2 failures analyzed below.
• tsc -b clean for storage / runtime-host / cli; apps/desktop tsc -p tsconfig.main.json fails exactly as CI (the locale errors).

The design reads correctly. The migration machinery is the shape it should be: the marker fence rejects business access mid-upgrade (legacy_root_requires_migration for v1 and for v2-with-upgrade), the plan file is bound to the transaction and never re-derived, staging is restaged at most once, a previously-present source can never be reinterpreted as empty, and the managed path fails closed in the right order (in my probes the marker stayed at schema 1 when the update was not admitted). Legacy Windows paths are byte-faithful to the base (AppData/Local/Maka for both cache and durable, state-root-owners, and the bootstrap-lock sha256(dev:ino) naming matches the old computation). Zero-residue scan of every removed symbol found no stale production consumer; the schema-1 decode and legacy read paths survive where migration needs them; the account-home locator remaining is inherent, and the doc says so.

Findings (inline where the diff allows):

1. [P1] The head does not compile its own desktop bundle — root_requires_migration joined RuntimeHostServiceErrorCode but the three locale records in settings-projects-copy.ts were not updated, so package and windows_recovery are red at the same tsc step. (Inline at the new union member.)
2. [P1] The recovery lane now contradicts the CI policy test — windows-recovery.yml runs packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js, while scripts/ci-workflow-policy.test.mjs (untouched by this PR) pins the lane's executed workspaces to ['runtime', 'runtime-host', 'storage']. That contradiction is the red test lane. (Inline at the workflow line.)
3. [P2] The three new cli successor-upgrade e2e tests cannot pass on Windows as written — deterministic 3/3 on real Windows: the staged fake package carries no Windows task-launcher artifact, so the update refuses with update_incomplete ("Maka does not include the Windows Runtime Host task launcher"). These are exactly the tests added to the lane that "has not run here". Fixture gap, not a production-ordering bug — the marker correctly stayed at schema 1. (Inline at the test.)
4. [P2] A live root can no longer be moved on Windows — keeps one owner when a live root moves behind a new alias fails 3/3 in isolation on this head and passes on the base: with the owner lock at <root>/.maka-host/<rootId>.lock, Windows forbids renaming a directory with an open lock file inside it. The invariant (one owner survives a move) still matters; the platform reality changed. Either record that live roots are immovable on Windows (docs + test adaptation), or keep an external lock reference. (Inline at the lock path.)

Honest noise report: in full-file runs on my machine I also saw transient EPERM-on-rename flakes (torn upgrade completion record…, serializes identity repair…, and two one-offs in the first combined run). Each passes in isolation and the base shows the same file-rename pattern, so I treat them as my machine (real user session, Defender/indexer), not as PR evidence. The CI lane is the arbiter once it can build.

COMMENT, not APPROVE: findings 1–4 are unresolved. All four are small; the architecture underneath is sound.

---

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

[zhiiw]

[P1] Adding root_requires_migration to RuntimeHostServiceErrorCode without the copy key breaks the desktop build: apps/desktop/src/renderer/locales/settings-projects-copy.ts keeps three Record<RuntimeHostServiceErrorCode | 'unknown', string> maps (lines 568, 906, 1261 per CI) that now miss this member, so tsc -p tsconfig.main.json fails and the package / windows_recovery lanes are red. Reproduced locally. Minimal fix: add the root_requires_migration copy to the three locale records (and the zh-CN/zh-TW siblings).

[Astro-Han]

Fixed in 62d24ba83 — root_requires_migration copy added to all three locale records (en / zh-CN / zh-TW); tsc -p tsconfig.main.json clean for settings-projects-copy.ts.

[zhiiw]

[P1] This step adds packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js to the lane, but scripts/ci-workflow-policy.test.mjs ("the recovery lane leaves the suites it executes to the required test lane", untouched by this PR) pins the lane's executed workspaces to ['runtime', 'runtime-host', 'storage'] — the policy's stated rationale being that ordinary TypeScript suites already run on Linux in the required test lane and re-running them here only buys a second, slower red. That contradiction is the current test failure. One side has to move: either the cli-workspace e2e relocates (e.g. under runtime-host), or the policy test and its rationale are amended to admit cli here.

[Astro-Han]

Resolved by amending the policy in 62d24ba83: the executed-workspace assertion now includes cli, with the rationale comment updated. These suites exercise win32-only production gates (task-launcher artifact, in-root lock semantics) that the Linux test lane cannot reach — the deterministic 3/3 Windows failure below demonstrates exactly that.

[zhiiw]

[P2] These three scenarios fail deterministically on real Windows (3/3, Node 24.18.1, this head): the staged fake successor package contains no Windows task-launcher artifact, so the update refuses with update_incomplete ("Maka does not include the Windows Runtime Host task launcher", runtime-host-windows-task-launcher-artifact.ts:70). The resume-under-failure contracts these tests pin are exactly what the recovery lane was extended to prove, so they need to pass there: give the staged package a stub launcher under native/runtime-host-windows-task-launcher/prebuilds/win32-x64/, or gate the three cases off win32 with the reason recorded. (Verified the production ordering is safe meanwhile: the marker stays at schema 1 when the update is not admitted.)

[Astro-Han]

Fixed in 62d24ba83 — both staged fake-package layouts now carry a stub native/runtime-host-windows-task-launcher/prebuilds/win32-x64/maka-runtime-host-task-launcher.exe, so the win32 launcher gate passes on real Windows. Chose the stub over gating precisely because the resume-under-failure contracts are what this lane exists to prove.

[zhiiw]

[P2] With the owner lock now inside the root, Windows can no longer rename a live root directory at all: keeps one owner when a live root moves behind a new alias fails deterministically here (3/3 isolated, EPERM renaming the root while .maka-host/<rootId>.lock is open) and passes on the base (account-home lock allowed the move). The invariant the test protects — one owner survives the move — is still real; what changed is that the move itself is impossible while the owner is alive on Windows. If immovable-while-live is the accepted semantic, say so in docs/architecture/runtime-host-architecture.md and adapt the test to close the owner before moving on win32; if moves of live roots must keep working, the lock reference has to stay outside the root.

[Astro-Han]

Accepted semantic, documented in 62d24ba83. With the owner lock inside <root>/.maka-host/, a live root cannot be renamed on Windows while the lock is held — that is a direct consequence of self-containment (the lock must travel with the root it protects), and both architecture docs now say so. The test keeps the same invariant on win32 via a junction alias instead of renaming a locked directory: the physical root still shares one owner.

[jackwener]

Second round at 62d0995b4da12b41cff6a05d381d3fef81f57e74. I am an AI agent (executing seat @kabi-opus) publishing through a shared GitHub account; this is an automated review and does not substitute for independent human review. A separate line from a different operator is reviewing this in parallel and covers the Windows side; we do not share conclusions.

Round 1's two P1s are resolved. One new P1, inline. I also owe you the migration-safety pass I deferred last round, and it comes out clean — details below.

Round 1 closed out

The locale map now carries root_requires_migration in the three blocks that were missing it, and windows_recovery is green where it was red, which is the empirical half. The lane-policy conflict was resolved the second way I described — the policy changed and the reasoning moved into the assertion with it, rather than the number being edited to match. Whether "every suite in this lane exercises code that branches on win32" is the right membership rule is a call for whoever owns that lane, but it is now written down where the next person will find it, which was the part that mattered.

The migration-safety questions I deferred

Last round I marked three things unexamined rather than clean. Having read them at this head, all three hold, and for better reasons than I expected:

Interruption fence, forward recovery. The plan is durable and re-read, never re-derived: root-upgrade.ts treats an upgrade marker as proof that the plan file is part of the transaction record. A resumed migration therefore continues with the same plan rather than re-inspecting sources that the first pass may already have consumed. Stale upgrade-* staging directories from other sessions are removed, and a legacy writer admitted before the fence still has its OS lock released explicitly.

Re-entrant idempotence. The loop re-enters on completedSnapshot(state, transaction.id) and, when that hits, calls assertCommittedState — which does not trust the completion record. It requires both directories, actually reads the access credential file and the plugin composition store, and cross-checks the deployment record against what completion attested, on the stated grounds that "an attested record that went missing is silent authority loss". A corrupt completed snapshot restages exactly once before throwing, so a bad copy degrades instead of wedging. I went looking specifically for the post-rename, pre-commit crash window; it lands in the committed branch and validates rather than restaging, so the migrated state is not destroyed by a retry.

Downgrade. Fail-closed, and I checked it against the old code rather than the description: the base's isRootMarker requires marker.schemaVersion === STORAGE_ROOT_MARKER_SCHEMA_VERSION, which is 1 there. A schema-2 marker fails that guard, so an older binary refuses the root instead of operating on a layout it does not understand. The destructive-downgrade note in the Migration section describes real behaviour.

The new finding

Inline, on access-credential-store.ts. Short version: a wrap added for the migration's benefit flattens ten distinct decode rejections into "corrupt" and breaks an existing assertion deterministically. test was still running when I finished, so it has not registered yet.

Evidence

@maka/core, storage, mcp, runtime, runtime-host rebuilt from cleaned dist at this head. storage root-authority: 32 pass, 3 platform skips, 0 fail. runtime-host root-upgrade: 15/15. Full runtime-host suite: 1980 tests, 1966 pass, 12 skipped, 2 fail — one is the new finding above; the other is WorkHub v2 keeps its attachment and browser tool ceiling visible in direct and Code Mode, which fails identically on the merge base in this same environment and is tracked as #5388, still open.

Not covered by me

Windows, entirely — no machine, and the parallel line owns that. I did not re-examine the 113 files from round 1 beyond the areas above; this round was scoped to the two P1 fixes and the three migration questions I had left open.

简体中文

在 62d0995b4da12b41cff6a05d381d3fef81f57e74 上的第二轮。第一轮两条 P1 已解决;新增一条 P1,在行内。 另外我补上了上一轮推迟的迁移安全核查,三条都站得住。

第一轮收口:locale 表补齐了缺失的三处 root_requires_migration,windows_recovery 由红转绿(这是实证的一半);通道策略走的是我说的第二条路 —— 改策略并把理由随断言一起写下来,而不是把数字改成与现状相符。"该通道每个套件都走 win32 分支"是不是正确的成员判据,由那条通道的持有者定;但它现在写在下一个人找得到的地方,这才是要紧的。

我推迟的三个迁移安全问题,读完之后都成立:①中断 fence 前滚 —— plan 是持久的、只读不重导;升级标记即证明 plan 文件属于事务记录,重入时不会重新探查可能已被消耗的旧源;其它会话遗留的 upgrade-* 暂存目录会被清掉;fence 发布前被准入的旧写者其 OS 锁会被显式释放。②重入幂等 —— 重入先看 completedSnapshot,命中则走 assertCommittedState,而它不信任完成记录:要求两个目录都在、真的去读凭据文件与插件组合存储、并把部署记录与完成态所声称的相互对照(理由写明:"被声称存在却消失的记录是静默的权威丢失");完成快照损坏时只重暂存一次即抛,降级而不卡死。我专门去找 rename 之后、commit 之前的崩溃窗口:重入会落进 committed 分支做校验而不是重暂存,已迁移状态不会被重试摧毁。③降级 —— fail-closed,而且我是对着旧代码核的:base 的 isRootMarker 要求 schemaVersion === 1,schema-2 的 marker 过不了这道守卫,旧二进制会拒绝该 root 而不是在看不懂的布局上操作。

新发现见行内(access-credential-store.ts):一个为迁移而加的包装,把十种不同的解码拒绝压成了"corrupt",并确定性地打断了一条既有断言。我收工时 test 还在跑,所以尚未反映出来。

证据:本 head 清产物重建 core/storage/mcp/runtime/runtime-host。storage 的 root-authority 32 通过 3 平台跳过 0 失败;runtime-host 的 root-upgrade 15/15;runtime-host 全量 1980 条:1966 通过、12 跳过、2 失败 —— 一条即上面的新发现,另一条是 WorkHub v2 keeps its attachment and browser tool ceiling visible…,在同一环境下于 merge base 上同样失败,即仍开着的 #5388。

未覆盖:Windows 完全未覆盖(无机器,且由并行线负责);第一轮那 113 个文件我也没有在上述范围之外重审 —— 本轮范围限定在两条 P1 的修复与我此前留空的三个迁移问题。

[jackwener]

[P1] This wrap flattens every decode rejection into "corrupt", and it breaks an existing assertion deterministically.

authenticated-websocket.test.ts:763 asserts on the precise reason:

/Pre-association Runtime Host access files cannot declare capability owners/u

That error is still thrown, but it is now the cause of a generic outer error, and the regexp only sees the outer message. The test fails 3 runs out of 3 in isolation here:

AssertionError: The input did not match the regular expression …
Input: 'Error: Runtime Host access file is corrupt: /…/runtime-host-access.json'
  [cause]: Error: Pre-association Runtime Host access files cannot declare capability owners

This PR does touch that test file, but only to adapt resolveRootControlNamespace(...) call sites to the new signature; the assertion was not swept for the new wrap, and there is no access file is corrupt assertion anywhere in the suite.

The test is the symptom. The part worth deciding is what the wrap does to diagnosis. decodeAccessFile raises at least ten distinct, specific errors — Unsupported Runtime Host access file, Invalid Runtime Host access file, Duplicate Runtime Host access credential identity, Duplicate Runtime Host pending credential principal, A Session Guest cannot be granted multiple Sessions, Duplicate Runtime Host Turn access request identity, and this one, among others. All of them now reach an operator as Runtime Host access file is corrupt: <path>.

Several of those are not corruption. "Pre-association files cannot declare capability owners" is an ordering violation; the duplicate-identity ones are state violations. In a credential store the word "corrupt" points at deletion or restore-from-backup, which is a destructive response to a file that parsed fine and was rejected on policy. cause keeps the real reason reachable programmatically, but only for a reader that prints causes.

The motivation is legitimate — assertCommittedState reads this file during migration and wants a uniform, path-bearing signal — so the answer is probably not to drop the wrap. Two shapes that keep it without losing the diagnosis: wrap only genuine parse failures, letting decodeAccessFile's named errors through unchanged; or keep the single catch and put the cause's message in the surface text, so the path and the reason travel together.

简体中文

这个包装把所有解码拒绝都压成了"corrupt",并且确定性地打断了一条既有断言。

authenticated-websocket.test.ts:763 断言的是精确原因(Pre-association Runtime Host access files cannot declare capability owners)。该错误仍然会抛出,但现在成了外层通用错误的 cause,而正则只看得到外层消息。我在本机单独跑该文件,3 次挂 3 次。

本 PR 确实改动了这个测试文件,但只是把 resolveRootControlNamespace(...) 的调用点适配到新签名;这条断言没有随新包装一起清扫,而且整个套件里也没有任何针对 access file is corrupt 的断言。

测试只是症状。真正要定的是这个包装对诊断做了什么。 decodeAccessFile 至少会抛出十种各不相同的具体错误(Unsupported…、Invalid…、Duplicate … credential identity、Duplicate … pending credential principal、A Session Guest cannot be granted multiple Sessions、Duplicate … Turn access request identity 等),现在它们到达运维手里全都是同一句 Runtime Host access file is corrupt: <path>。

其中若干根本不是损坏:"Pre-association 文件不能声明 capability owner"是顺序违规,重复身份那几条是状态违规。在一个凭据存储里,"corrupt"这个词指向的是删除或从备份恢复 —— 而对一个解析正常、只是被策略拒绝的文件采取这种处置是破坏性的。cause 让真实原因在程序上仍可获取,但前提是读取方会打印 cause。

动机是正当的 —— 迁移里的 assertCommittedState 会读这个文件,需要一个统一的、带路径的信号 —— 所以答案多半不是去掉包装。两种能保住包装又不丢诊断的形状:只包装真正的解析失败,让 decodeAccessFile 的具名错误原样透出;或者保留这一处 catch,但把 cause 的消息放进外层文本,让路径和原因一起走。

[Astro-Han]

Fixed in b3ba58b3b, taking the hybrid of your two shapes: only genuine parse failures get the "corrupt" label (which is now honest — the file did not parse), while every decodeAccessFile rejection keeps its precise message with the path appended as a suffix (${error.message}: ${path}). The ordering and state violations keep their names, the path still travels with every failure, and the pinned assertion matches again — authenticated-websocket.test.js passes locally.

The same shape in validateDeploymentSource had a worse edge you would have caught next: its decoder throws the typed invalid_config error, and the plain-Error wrap was silently dropping the wire code. It now rethrows the typed error with the path appended to the message, code preserved.

[zhiiw]

Re-review of the increment f6dae5f9..62d0995b. Conclusions bind to 62d0995b4da12b41cff6a05d381d3fef81f57e74 (CI: windows_recovery green; test red — one failure, see inline; package still running at review time).

All four findings from my previous review are closed, each verified on the same real Windows machine that produced them (Node 24.18.1):

1. Locale break: the three locale records now carry the new codes (plus root_migration_busy / root_unavailable), and tsc -p tsconfig.main.json --noEmit is clean locally.
2. Policy contradiction: resolved by amending the policy test with the rationale written into it ("every suite in this lane exercises code that branches on win32 … the cli upgrade suite earns its place the same way"); scripts/ci-workflow-policy.test.mjs passes 46/46 locally. The reasoning is sound — the cli suite drives the Windows task-launcher gate and the in-root owner lock, which the Linux lane cannot reach.
3. cli upgrade e2e fixtures: the staged packages now carry a stub task-launcher prebuild; the three scenarios that failed 3/3 on my machine now pass (cli root-upgrade 4/4).
4. Live-root move on Windows: the test now places the junction beside the live root on win32 (the invariant — one owner across the alias — is still exercised), and the architecture doc records the semantic ("Windows cannot rename the directory while the owner holds the lock; release the owner before moving a live root"). root-authority.test.js full-file run: green locally, including keeps one owner…. This closes the loop the honest way: the platform fact is documented rather than silently absorbed.

Migration suites on this head, locally: root-migration + root-upgrade + cli root-upgrade + root-authority — 0 failures (8 platform skips). The EPERM flakes I reported from my machine did not reappear; they stay classified as my environment.

One new break on this head, inline: the access file is corrupt wrap in readAccessCredentialFile changes the top-level message an existing test matches against — test is red on exactly that. One-line test fix or message-preserving wrap.

COMMENT, not APPROVE: the inline break is unresolved. Everything I reported on the previous head is closed with evidence.

---

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

[zhiiw]

[P1] This wrap changed the top-level error message, and an existing test pins the inner one: authenticated-websocket.test.ts ("capability-provider credentials retain a Host-verified Client owner identity") does assert.rejects(…, /Pre-association Runtime Host access files cannot declare capability owners/), which matches the outer message only. The test lane is red on exactly this, and it reproduces locally. Minimal fix: keep the original message on the thrown error (e.g. put "corrupt: path" detail in cause or a suffix after the original message), or update the test's regex to the wrapped message — the former preserves every other caller's message expectations.

[Astro-Han]

Fixed in b3ba58b3b with the message-preserving shape you suggested: parse failures keep the "corrupt: <path>" text (accurate there — the file did not parse), and decode rejections surface as <original reason>: \<path\>, so the pinned regex matches the original message again and the path still reaches the operator. Verified locally: authenticated-websocket.test.js green.

[jackwener]

[P2] The symlink allowance defeats the socket/FIFO skip this commit adds.

The comment two lines up says entries cp cannot carry are skipped "instead of wedging the copy", but the predicate is lstatSync, so it classifies the link, not its target. isSymbolicLink() returns true for a symlink pointing at a socket or FIFO, the entry is admitted, and dereference: true then makes cp copy the target — the exact case the filter is here to exclude.

I reproduced it on this commit's options, outside the repo so nothing else is in play:

src/realfifo   (FIFO)
src/linkfifo -> realfifo
src/file.txt

cp(src, dst, { recursive: true, dereference: true, filter /* as written here */ })
→ ERR_FS_CP_FIFO_PIPE: Cannot copy a FIFO pipe: cp returned EINVAL
   (cannot copy a FIFO pipe: dst/linkfifo)

realfifo is skipped correctly. One level of symlink indirection is enough to get past the guard. This matters here specifically because the same commit makes symlinked legacy sources legal (present() moved from lstat to stat) and turns on dereference, so symlinks in a legacy data directory are now an expected shape rather than an exotic one — and a host data directory is exactly where sockets live, which is why the skip exists.

Failure mode is fail-closed, not data loss: the cp rejection carries ERR_FS_CP_FIFO_PIPE, so sourceGone stays false and stageSnapshot throws "repair or remove the failing entry, then retry". But the upgrade is blocked until an operator finds the entry, and the message names the source directory, not the offending path, so there is nothing to act on directly.

Minimal fix — resolve through the link before deciding, which also disposes of dangling symlinks:

const link = lstatSync(entry);
const kind = link.isSymbolicLink() ? statSync(entry, { throwIfNoEntry: false }) : link;
if (!kind) return false;               // dangling symlink
return kind.isFile() || kind.isDirectory();

I ran that exact predicate against the same fixture plus a linkdir -> realdir case: the FIFO symlink is skipped, the directory symlink is still followed and copied, and the regular file is unaffected. Dropping isSymbolicLink() from the allow-list on its own would not work, since a symlinked directory must be admitted for dereference to follow it.

Two smaller notes on the same predicate, neither blocking:

• lstatSync throws if an entry disappears between readdir and the filter. That rejection lands in the same catch and is reported as "repair or remove the failing entry", which is misleading for a race.
• It is a synchronous stat per entry inside an async copy, so a large legacy tree blocks the loop.

简体中文

这条 [P2] 是:放行 symlink 的写法把本次提交新加的 socket/FIFO 跳过逻辑绕过去了。

上面两行注释说要跳过 cp 搬不动的条目"以免把拷贝卡死",但判据是 lstatSync,判的是链接本身而不是它指向的东西。指向 socket/FIFO 的符号链接在 isSymbolicLink() 上为真,于是被放行,接着 dereference: true 让 cp 去拷贝目标——正是这个过滤器要排除的情况。

我在仓库外用本提交的同一组参数复现过:src/realfifo(FIFO)被正确跳过,而 src/linkfifo -> realfifo 会让 cp 抛 ERR_FS_CP_FIFO_PIPE。一层符号链接就够绕过去了。

这在本提交里尤其相关:同一个提交把 symlink 的 legacy 源变成合法(present() 从 lstat 改成 stat)并打开了 dereference,所以 legacy 数据目录里出现符号链接已经是预期形态;而 host 数据目录正是 socket 会出现的地方——这也正是这个跳过逻辑存在的原因。

后果是 fail-closed、不丢数据:错误码是 ERR_FS_CP_FIFO_PIPE,sourceGone 为假,stageSnapshot 抛"repair or remove the failing entry"。但升级被挡住,而且提示里给的是源目录、不是出问题的那个条目,运维照着这句话没法直接动手。

最小修法是在判定前先穿过链接(顺带把悬空链接也处理掉),见上面的代码块。我用同一组夹具加上 linkdir -> realdir 跑过那段判据:FIFO 链接被跳过,目录链接仍被跟随并拷贝,普通文件不受影响。单纯把 isSymbolicLink() 从白名单里去掉是不行的——符号链接目录必须被放行,dereference 才会去跟随它。

另有两点同一判据上的小问题,都不阻塞:lstatSync 在条目于 readdir 与 filter 之间消失时会抛,落到同一个 catch 里被报成"repair or remove the failing entry",对竞态而言有误导;以及它是异步拷贝里的同步 stat,legacy 树大时会阻塞事件循环。

[jackwener]

Independently reproduced by a second automated reviewer seat, which did not read this comment.

That seat ran its own probe against the production cp options and reported the same three outcomes I did — direct FIFO skipped, symlink-to-FIFO throwing ERR_FS_CP_FIFO_PIPE, failure fail-closed and recoverable — and arrived at the same minimal fix (if lstat says symlink, stat the target and admit only file/directory). It also named a concrete way the shape arises in a legacy data/ directory: leftover IPC pipes, or a tool that creates a fifo and then a symlink to it.

One observation it added that I had not stated: a symlink to an ordinary file outside the source is now materialized into the snapshot. That follows from dereference: true and is arguably the intended meaning of a self-contained root, so we both treat it as a deliberate boundary rather than a defect — but it is worth being explicit that the upgrade now pulls external file content into durable state.

Recording this because the two lines are independent: different reviewer seat, its own probe, written before reading this thread. It does not make the finding more true, but it does mean it is not resting on one reading of the code.

简体中文

这条已由另一个自动化审查席位独立复现,该席位没有读本条评论。

它用生产同款 cp 参数自己跑了探针,得到与我相同的三个结果(直接 FIFO 被跳过;指向 FIFO 的符号链接抛 ERR_FS_CP_FIFO_PIPE;失败是 fail-closed、可恢复),并给出同一个最小修法(lstat 判定为符号链接时,stat 目标,只放行文件/目录)。它还指出了这种形状在 legacy data/ 里的具体来源:遗留的 IPC 管道,或某个工具先建 fifo 再建指向它的符号链接。

它补充了一点我没写的:指向源目录之外普通文件的符号链接,现在会被实体化进快照。这是 dereference: true 的直接结果,也可能正是"自包含根"的本意,所以我们都把它当作有意的边界而非缺陷——但值得明说:升级现在会把外部文件内容拉进持久状态。

记录这一点是因为两条线相互独立:不同审查席位、各自的探针、写在读到本线程之前。这不会让结论更真,但说明它不是只建立在一个人对代码的一种读法上。

[jackwener]

Re-review of exact head 9f432637d0d0aba55ee4e8c789a5ab879b890ea5. My conclusions from 62d0995b4 are void; this is a fresh pass. Not approving: one open [P2], posted inline, and the required checks are not terminal yet (test, package, windows_recovery were still running when I looked).

The finding I raised last round is genuinely fixed, not relabelled. readAccessCredentialFile now reserves "corrupt" for SyntaxError and lets every other decode rejection keep its own message with the path appended (access-credential-store.ts:283-289, commit b3ba58b3b). The authenticated-websocket suite passes 20/20 on three consecutive local runs, including the Pre-association Runtime Host access files cannot declare capability owners assertion that failed 3/3 for me on the previous head — same machine, so this is a before/after comparison rather than a change of environment. I am not treating that as closing the finding: it was raised independently by a second reviewer on Windows, and the independent re-check belongs to that line, not to me.

guard committed state against marker loss (6c25fa803) holds up. I checked the ordering it depends on rather than the claim: ensureRootMarker runs during root resolution, before any repository creates .maka-host/state, so "a formed state directory implies this root was marked and committed before" is not a false positive for a fresh root that crashed during bootstrap. Both new refusals fail closed with invalid_marker and an actionable message. The hardlink fallback in publishMarkerFile cleans up its staged temp file and still syncs the directory.

Scope check: the delta from the previous head is 21 files, +461/−87, across four fix commits, on the same merge-base (846f4fbaa) — no rebase. I found no unrelated feature work riding along in it.

What I did not cover: Windows behaviour of any kind, including the windows_recovery job and the live-root/junction path — I have no Windows machine, and that ground is covered independently by another reviewer. I also did not run the packaged installer or a real legacy-root migration end to end; my evidence is source reading plus the targeted suites and the standalone cp probe described inline.

---

Automated review notice: This review was produced by an AI agent (Claude Opus 5) and published through the shared jackwener account. It is not an independent human review and does not replace one.

简体中文

重审 exact head 9f432637d,上一轮 62d0995b4 的结论全部作废。不批准:有一条 P2,且必需检查尚未到终态(test、package、windows_recovery 我查看时仍在运行)。

上一轮我提的那条确实修好了,不是换了个说法:readAccessCredentialFile 现在只把 SyntaxError 叫 corrupt,其余 decode 拒绝保留各自 message 并附路径。本机 authenticated-websocket 连跑三次 20/20,上一个 head 上挂 3/3 的那条断言现在过了——同一台机器,前后对照。但我不把这当作该发现的闭合:那条是由另一位审查者在 Windows 上独立命中的,独立复核应由那条线给,不是我。

guard committed state against marker loss 站得住:我核的是它依赖的顺序而不是它的说法——ensureRootMarker 在根解析期间运行,早于任何仓库创建 .maka-host/state,所以"存在成形的 state 目录即说明此根此前被标记并提交过"对启动中途崩溃的新根不构成误判。两条新拒绝都 fail-closed。

范围:相对上一个 head 的增量是 21 文件 +461/−87,四个 fix 提交,merge-base 未变(846f4fbaa),没有 rebase;其中没有夹带无关功能。

未覆盖:任何 Windows 行为(含 windows_recovery 与活 root / junction 路径)——我没有 Windows 机器,该面由另一条线独立覆盖;也没有跑打包安装器或真实 legacy root 的端到端迁移。

[jackwener]

Review of exact head 9f432637d0d0aba55ee4e8c789a5ab879b890ea5.

I am not approving this head. Required test is still in_progress on this SHA (package likewise; windows_recovery is completed/success). This is an Astro-Han draft; I will not merge it.

I did not read other reviews. Round-1 conclusions on 62d0995b4 are discarded. This round versus that SHA is four fix commits, 21 files, +461/−87. The whole pull request versus merge-base 846f4fbaa is still the 125-file schema-2 move.

What this change is doing

The problem is real. Host coordination used to live under the OS account home and cache, so an account without a usable home could not start a Runtime Host, and wiping the cache destroyed issued credentials and disconnected locks from the root they protected. Putting the owner lock, credentials, plugin state and the managed deployment record inside the physical root, with the account side reduced to a locator, is the smallest shape that makes the root self-contained.

prepareRuntimeHostRoot is the single admission point. Schema 2 is a destructive format change; that is documented, including the 0.2.x-only migration window. I walked the upgrade fence (plan file + session.begin before staging, restage-once of a corrupt committed snapshot, locator written only after the snapshot is in place, cleanup failure after commit() must not report the upgrade as failed).

The named-refusal collapse

readAccessCredentialFile on this head labels a file corrupt only for SyntaxError from JSON.parse. Other Errors keep their message, with the path appended. decodeAccessFile throws ordinary Errors for the policy/ordering refusals (including Pre-association Runtime Host access files cannot declare capability owners). The websocket test at authenticated-websocket.test.ts:761-763 still asserts that named string, which would not match Runtime Host access file is corrupt.

I did not run that suite here (workspace tsc for @maka/runtime-host does not build on this machine without a matching @maka/runtime build). The pin is the production catch plus that assertion. Wrapping every decode Error in a new Error still drops any future typed class; today those refusals are already plain Errors.

This round's other three commits

These are crash-window and leak fixes, not a second feature.

• Marker-loss guards: a formed .maka-host/state with a missing or freshly re-minted schema-1 marker now fails invalid_marker instead of minting a new rootId or staging empty sources over committed state. Two root-authority tests pin that.
• connectOrSpawn folds prelude failures into startup_failed and still lets an aborted signal throw. connect-or-spawn-env.test.ts pins both.
• CLI update frames fold remaining storage-root codes through managedRuntimeHostErrorCode so internal codes do not go on the wire.

P2 — this pull request — exclusive marker create can leave a husk that later looks like success

packages/storage/src/marker-file.ts:101-123

On filesystems where link(temp, marker) fails with EPERM/ENOTSUP/ENOSYS, publication falls back to open(markerPath, 'wx') then write/sync. If that write throws, the handler closes the handle and rethrows without unlinking markerPath. The next publication: 'create' tries link first, sees EEXIST, and returns already_exists without writing contents.

So a failed or crashed exclusive create leaves an empty or partial marker that later attempts treat as a successful existing marker. The hardlink path does not have this shape: it only links after the temp file is fully written. I found no test that drives the no-hardlink fallback.

Minimal fix: delete markerPath if the exclusive write fails, and do not treat a marker that cannot be decoded as already_exists.

Verification bounds

• Walked: admission/upgrade fence, credential read, the four commits after 62d0995b4, marker publication, connect-or-spawn prelude.
• Did not walk every one of the 125 files at equal depth.
• Did not run focused suites or full test:dist. Required test is not terminal on this SHA.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

简体中文

只绑 9f432637d。test 仍在跑，不 approve，不合并。

问题是真的：协调面从账户 home/cache 挪进物理 root。readAccessCredentialFile 现在只有 JSON.parse 的 SyntaxError 才叫 corrupt，具名拒绝还在，websocket 测试仍断言那句 Pre-association 原文。

本轮另外三笔是崩溃窗口/泄漏，不是第二套功能。P2：无硬链接回退 wx 创建 marker 后写失败不删文件，下次当成 already_exists。最小修法：写失败就删掉这个 husk，解不出的 marker 不能当成功。

[jackwener]

P2 on this head. If link falls back to open(markerPath, 'wx') and the write throws, this catch closes the handle and rethrows without unlinking markerPath.

The next publication: 'create' then hits link → EEXIST → already_exists and never writes contents. A failed exclusive create therefore leaves an empty or partial marker that later looks like a successful existing one. The hardlink path does not: it only links after the temp file is complete.

Delete markerPath on write failure, and do not treat an undecodable marker as already_exists.

[jackwener]

Follow-up to review 5248780043, still on exact head 9f432637d0d0aba55ee4e8c789a5ab879b890ea5. I am not changing that review.

The husk is real: on the no-hardlink fallback, a failed exclusive write leaves markerPath in place, and I still found no test that drives that path. What I got wrong is the caller-level consequence.

I re-read every publication: 'create' call site (ensureRootMarker, bindStateRootComposition, createWorkspaceMarker). None of them treat already_exists as a successful marker:

• ensureRootMarker ignores the publish result and always readPersistedRootMarker. If the husk is already there, lstat(markerPath) succeeds first and it never reaches publish; parseRootMarker('') fails closed as invalid_marker.
• Composition only returns early on published; otherwise it reads and assertMatchingComposition, which cannot decode a husk.
• Workspace identity ignores the publish result, then readWorkspaceMarker, same fail-closed.

The two publication: 'replace' call sites use rename and never take the wx fallback.

So publishMarkerFile can still return already_exists for an empty or partial file. At every real caller that is a wedge (the root or marker is stuck until someone deletes the zero-byte file), not silent success. The minimal fix is unchanged: unlink on exclusive-write failure, and do not treat an undecodable marker as already_exists.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

简体中文

仍绑 9f432637d。不改原 review。husk 是真的，无硬链接回退仍无测试。错的是后果：三个 create 调用点都不会把 already_exists 当成功，空文件解不开，是 fail-closed 卡住，要人删掉那个零字节文件，不是 silent success。最小修法不变。

[zhiiw]

Round-2 review. Conclusions bind to 9f432637d0d0aba55ee4e8c789a5ab879b890ea5 (still Draft; at review time every check is terminal green, Validate installed CLI Eval skipped). The true increment over the previously reviewed head is 62d0995b..9f432637d — 21 files, +461/−87, four fix commits; I verified the increment directly rather than the PR-total delta, and found no scope creep.

The previously unresolved break is genuinely fixed, in the better of the two proposed directions. Only a real parse failure (SyntaxError) is now labeled "corrupt"; the named decode rejections keep their own reason with the path appended, so a policy/ordering rejection no longer invites a destructive remedy. The previously failing test (capability-provider credentials retain a Host-verified Client owner identity) passes locally.

All four of my round-1 findings remain closed on this head, each re-verified on the same real Windows machine (Node 24.18.1): locale records + desktop tsc clean; the amended CI policy test passes 46/46; the three cli successor-upgrade e2e pass 4/4 with the stub launcher; the win32 junction-beside-live-root adaptation passes and the doc sentence is present. Migration suites (storage root-migration, runtime-host root-upgrade, cli root-upgrade, root-authority) plus connect-or-spawn-env: 0 failures locally (8 platform skips).

The new surfaces in this increment read sound: the lost-marker guards fail closed in both places (a formed state/ next to a missing or legacy-declared marker refuses to mint a fresh identity); startup failures fold into a diagnosable startup_failed with abort precedence; the hardlink-less-filesystem fallback; the migration staging hardening (dereference + non-file filtering + hardenTree before validation reads); the legacy-source re-inspection under the admitted locks, which closes the "attested source vanished" hole.

Attribution note for this machine: the cli service-manager / update-reconciliation / setup failures and the file-update-lock inherited-descriptor failure that appear on my Windows box reproduce identically on the PR's base — pre-existing local platform gaps (directory fsync, fork fd inheritance), not this PR's account. Every test this PR adds or touches passes locally.

Scope note: I did not re-adjudicate the other seats' open findings. Nothing unresolved in mine.

---

Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

[jackwener]

Required checks on exact head 9f432637d0d0aba55ee4e8c789a5ab879b890ea5 are now terminal: test, package, and windows_recovery are completed/success. Validate installed CLI Eval is skipped. The pull request is still a draft.

I am still not approving. The marker-husk P2 is open, and this pass was an adversarial re-review, not an approve-if-green instruction. I will not merge it.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

简体中文

仍绑 9f432637d。必需检查已终态全绿。仍不 approve：marker husk 的 P2 还开着，这轮也没有“没 P0–P2 就批”的指令。draft，不合并。

[jackwener]

[P2] The "attested artifact went missing" check covers the deployment record but not the two artifacts read on these two lines.

Eleven lines below, this function makes an explicit argument: the completion record attests whether staging had a deployment record, and "an attested record that went missing is silent authority loss". That reasoning applies just as well to the access credential file and the Plugin Composition — but neither is attested, and both reads here are ENOENT-tolerant, so a committed snapshot that lost them passes.

• readAccessCredentialFile returns createAccessCredentialFile([]) on ENOENT (access-credential-store.ts:265).
• HostPluginCompositionStore.read() returns undefined on ENOENT (plugin-composition-store.ts:68).
• Both results are discarded here; the calls only assert "parses if present".
• completionRecordSchema carries migrationId and deploymentRecord only (root-upgrade.ts:558-565), so there is nothing to compare against.

So a snapshot whose data/ directory survived but whose runtime-host-access.json did not is accepted as committed, and the upgrade proceeds having silently dropped every issued credential. The same snapshot missing its deployment record is correctly rejected.

The tolerance itself is right and cannot simply be removed: a legacy root that never issued credentials genuinely has no access file, which is exactly why the deployment record needed an attestation bit rather than a presence check. The gap is that the bit was only minted for one of the three artifacts.

This is not hypothetical under this function's own threat model. The comment above completedSnapshot — "The completion record only proves the staged copy; check what survived" — states that the snapshot may degrade between staging and this validation. That is the window the deployment check defends and these two lines do not.

Minimal fix, matching the existing shape: record accessFile and pluginComposition booleans alongside deploymentRecord when the completion record is written (root-upgrade.ts:408-414, where both artifacts have just been read), and reject here when an attested artifact is absent.

I have not driven this as a fault-injection probe; it is read from the source and from the two ENOENT branches, so treat it as P2 rather than a demonstrated failure. Rated P2 and not P1 for the same reason the deployment check is not P0: it needs the snapshot to degrade after staging.

简体中文

[P2] "被登记过的产物消失了"这条检查只保护了 deployment record,没有保护这两行读的两个产物。

本函数往下十一行有一句明确的论证:完成记录登记了暂存时是否存在 deployment record,而**"被登记过的记录消失了,就是静默的权限丢失"。这个论证对访问凭据文件和 Plugin Composition 同样成立——但这两个都没有被登记,而且这里两次读都容忍 ENOENT,所以丢了它们的已提交快照会通过校验**。

• readAccessCredentialFile 在 ENOENT 时返回空凭据文件(access-credential-store.ts:265);
• HostPluginCompositionStore.read() 在 ENOENT 时返回 undefined(plugin-composition-store.ts:68);
• 两者返回值在这里都被丢弃,调用只断言了"存在则能解析";
• completionRecordSchema 只有 migrationId 与 deploymentRecord,没有可供比对的登记位。

于是:data/ 目录还在、但 runtime-host-access.json 没了的快照会被当作已提交接受,升级继续,所有已签发的凭据被静默丢弃;而同一个快照如果丢的是 deployment record,则会被正确拒绝。

容忍 ENOENT 本身是对的、不能直接删掉:从未签发过凭据的 legacy root 确实没有访问文件——这正是 deployment record 当初需要一个登记位而不是存在性检查的原因。缺口在于这个位只为三个产物中的一个铸了。

在本函数自己的威胁模型下这不是假设:completedSnapshot 上方写着**"完成记录只证明暂存副本;要检查活下来的是什么"**,即明说快照可能在暂存与本校验之间退化。那正是 deployment 检查所防、而这两行不防的窗口。

最小修法(沿用既有形状):写完成记录时(root-upgrade.ts:408-414,那里刚好刚读过这两个产物)一并记下 accessFile 与 pluginComposition 两个布尔,并在此处对"登记过却缺失"的产物拒绝。

我没有用故障注入探针跑过,这条是读源码和那两个 ENOENT 分支得出的,所以按 P2 记而不是已演示的故障。定 P2 不定 P1 的理由与 deployment 检查不是 P0 相同:它需要快照在暂存之后退化。

[jackwener]

Scope note, so this is not mistaken for a regression in the four fix commits pushed since the previous head.

assertCommittedState is untouched by 62d0995b4..9f432637d — the four commits do not go near it. It is in scope for this pull request only in the sense that the whole file is: packages/runtime-host/src/root-upgrade.ts does not exist at the merge-base 846f4fbaa and is introduced by 916b7a774 within this branch.

So: new code that this pull request adds, not something the latest round broke. I am raising it now because this round is where I read the file, not because it changed. A second reviewer seat independently confirmed the mechanism (discarded return values, ENOENT treated as an empty artifact) and made the same scope point.

简体中文

范围说明,免得把它误当成本轮四个 fix 提交引入的回归。

assertCommittedState 不在 62d0995b4..9f432637d 的改动范围内,那四个提交没碰它。它属于本 PR 的范围,只是因为整个文件都是本 PR 新增的:packages/runtime-host/src/root-upgrade.ts 在 merge-base 846f4fbaa 上并不存在,由本分支内的 916b7a774 引入。

也就是说:这是本 PR 新增的代码,不是最近这一轮改坏的。我现在才提,是因为这一轮才读到这个文件,不是因为它变了。另一个审查席位独立确认了该机制(返回值被丢弃、ENOENT 被当作空产物),并给出了同样的范围判断。

[jackwener]

Reviewed exact head 9f432637d0d0aba55ee4e8c789a5ab879b890ea5.

P2 — the no-hardlink fallback can permanently wedge a valid State Root

packages/storage/src/marker-file.ts:97-123 first writes and fsyncs a temporary marker, but on EPERM / ENOTSUP / ENOSYS from link() it opens the authoritative marker path with wx and writes the bytes in place. A partial write, I/O error, process crash, or power interruption can therefore leave a truncated final marker. The temporary file is then removed, and every retry sees EEXIST and returns already_exists; nothing replaces the husk.

This is reachable on the exFAT/network filesystems that the new fallback explicitly intends to support. I fault-injected the fallback final handle so it wrote one byte and raised EIO: the final marker contained only {, and a clean retry returned already_exists while leaving { untouched. Through the production State Root composition caller (state-root-composition.ts:54-84), first bind returned composition_io_failed; retry returned invalid_composition against the same retained byte. An established root is then fail-closed until the marker is manually removed. This is a current regression introduced by the second-round filesystem-support expansion, not silent success or data loss.

Please never write the authoritative marker in place. The smallest sound options are to reject filesystems without the required atomic primitive, or serialize the fallback with a per-marker OS/advisory lock, recheck absence while holding it, and atomically rename the already-written/fsynced temporary file. Add an interruption test at this fallback publication boundary and prove retry recovery; the current marker tests cover temporary-file write/sync/close failures only.

What I independently verified as holding

• Credential decoding now labels only malformed JSON as corrupt. Unsupported schema, duplicate identity, Session Guest ordering violations, oversize, and raw I/O failures retain distinct classifications.
• Real child processes killed with SIGKILL immediately after upgrade-fence publication, committed-state rename, and ready-marker publication all recovered in a fresh process, preserving the exact root id and migrated canary bytes.
• The dependency-order Core → Storage → MCP → Runtime → Runtime Host → Computer Use → Eval → CLI builds passed. Focused migration/authority/credential suites passed 100 tests, with 3 platform skips. All exact-head required checks are terminal and successful.

Required conclusion: the central self-contained-root transaction is directionally sound, but this revision is not ready to merge while the P2 remains. No production deletion or deeper architectural refactor is required for this finding; preserve the existing temporary-file authority and repair only the unsupported-hardlink publication mechanism. I found no low-value test to delete—the missing test is the fallback interruption/reentry case. Residual verification is the repaired behavior on real exFAT/network filesystems and independent human review of this material persistence/availability change.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.