[fxl112233]

Summary

Managed dependency installation needs proof that the entire producer process tree has exited before Storage can publish or remove staging. Add an internal offline npm ci runner using Windows atomic Job supervision and Linux bubblewrap PID namespaces, with distinct completed, settled-failure and unsettled results. Cancellation, deadlines and detected quotas share the settlement boundary; unsettled staging is retained.

Use explicit runtime paths, private npm configuration/cache, disabled lifecycle scripts, bounded redacted output and symlink-aware inventory. Test real offline npm, receipt reuse and settled-failure Storage handoff. Supporting commits fix Windows PTY resource cleanup, registration replacement retries, plugin directory synchronization and test fixture lifetimes/portability.

Refs #4326

Verification

• All 11 Linux workspaces: 12,043 passed, 43 skipped, 0 failed (12,086 total), including complete Runtime and Runtime Host suites. Whole-repository Linux build passed.
• Windows complete Runtime Host: 1,891 passed, 56 skipped, 0 failed, with natural exit. Windows PTY input/PowerShell gates: 5 passed; Storage path/lock/recovery gates: 74 passed, 12 skipped.
• Real producer/npm + Storage authority/crash: Windows 44 passed, 3 skipped; Linux 41 passed, 6 skipped. Includes NTFS ADS checks and real Linux .bin links/namespace teardown.
• Windows Rust launcher: 55 passed; strict clippy, build and rustfmt passed. New real-Job tests admit a closed-stdio descendant, reject breakaway, and prove root-first exit/cancellation drain before writes stop. AppContainer admission remains separately tested as refusal in this environment.
• Lint, format, typecheck, ASF headers, Desktop/UI knip, Windows inventory and required Linux sandbox smoke passed. Dependency patches apply cleanly; the exported commit series reproduces the final Git tree from the pinned baseline.
• Real Electron E2E on Linux/Xvfb: 38 passed, one worker, zero retries, 13.3 minutes.

Linux evidence comes from an unprivileged Debian Docker environment with a tini subreaper, Node 24.18.1 and npm 11.19.0. The broad Windows Runtime diagnostic suite is not green: 143 named failures reproduce on the unmodified baseline, 9 previously failing cases pass current isolated reruns, and one record is a file-level failure. No full Windows Runtime success is claimed. Release/signing pipelines, live Eval egress images and Storybook geometry experiments were not replayed locally; remote CI remains authoritative.

Review focus

This is the producer foundation only. It installs no production capability/consumer and changes no external protocol or schema. Bundled runtime identity/supply chain, online registry policy, packaging, full Host quarantine/lifecycle integration and Desktop enablement remain separate work under #4326. An unsettled result must not become ordinary provision() rejection. Linux's read-only host mount is not the complete future hermetic filesystem policy; macOS explicitly refuses execution.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented code, tests, documentation and this draft, and ran the recorded local checks. Every authored commit contains Generated-by: OpenAI Codex; retain the trailer when squashing. Human review is required before submission.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally — Linux workspaces and Windows platform gates; broader Windows diagnostic exceptions are recorded above

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[hqhq1025]

Reviewed exact head a23a4509c2cc464da6e9fa4bb052585541770ef5. This change adds the offline managed-dependency producer foundation, including Linux PID-namespace supervision, Windows Job/AppContainer settlement, quota scanning, and the Storage handoff contract. I found one P2 issue, attached inline, so the technical result is NO-GO pending a fix.

Validation completed: build:test; full typecheck, lint, format, ASF headers, Windows inventory, and diff check; producer unit tests (12 passed, 1 platform skip); Storage (1,242 passed, 11 skipped); Runtime Host (1,932 passed, 20 skipped, 1 failed). The single Runtime Host failure and the native Linux producer integration failures are environmental on this runner: bubblewrap cannot configure loopback (RTM_NEWADDR: Operation not permitted). Hosted test, package, windows_recovery, and windows_sandbox_w0_protocol are green. The head cleanly merges with current main fa25b3d018380d1893602af832756f2bb35a4a89.

I could not independently run the native Windows supervisor tests or a privileged Linux bubblewrap integration on this host.

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

[hqhq1025]

[P2] Start the deadline and cancellation boundary before preparation. The listener and timer are installed only here, after the cache seed has been recursively scanned and copied and after the staging tree has been scanned. On this exact head, an 8,000-entry seed made timeoutMs: 1 return completed after 2,625 ms; an abort scheduled at 1 ms returned cancelled only after 2,348 ms, without ever launching. With the accepted 250,000-entry / 2 GiB inputs, preparation can keep a caller blocked well past its requested deadline. Please make the seed scan/copy, staging preparation, and final scan observe the same lifecycle deadline/cancellation signal, and add a large-seed regression that fails if an expired timeout can still report success.

[fxl112233]

Automated follow-up by OpenAI Codex on behalf of fxl112233.

Addressed in 08182f0. The deadline and abort listener now start before preparation and remain active through final inventory. Monotonic checkpoints cover seed/staging scans, directory enumeration, cache-copy entries, and staging writes. The supervisor receives only the remaining time budget. In-flight filesystem work and supervisor settlement are still awaited; an unconfirmed supervisor still returns unsettled. Individual filesystem operations are cooperatively bounded, not forcibly interrupted.

Regression coverage includes an 8,000-entry seed with a 1 ms timeout/cancellation, interruption after copying actually starts, and expiry/cancellation during final inventory. The original implementation failed the three initial regression tests; the fixed producer suite passes 16 tests with 1 platform skip, including the additional interrupted-copy regression. Runtime Host and referenced dependencies compile; repository lint/format, ASF headers, Windows inventory, and diff checks pass.

Full local Runtime Host run (Windows, concurrency 4, per-test timeout 180 s): 1,894 passed, 56 skipped, 1 failed. The sole failure was the unchanged owned-candidate election test, an exited owned Candidate permits one real successor in the same election; rerunning its complete test file passed all 11 tests. This is consistent with a timing-sensitive failure, not a claimed all-green initial run. Applying the repository dependency patches also removed the prior local suite stall. Native Linux producer integration was not run on this Windows host.

On this exact new head, windows_recovery and windows_sandbox_w0_protocol have passed; CI test and Windows packaging are still running. Please take another look at the lifecycle fix once checks finish.

[hqhq1025]

Reviewed exact head 08182f0036801f8c0fcb5150d78c8b6ba8fe49a6. The previous P2 is fixed, and I found no remaining P0-P3 issues.

The producer now establishes one monotonic deadline and cancellation boundary before cache discovery, copying, staging inspection, process supervision, and final inventory. Filesystem traversal and copy filters checkpoint that boundary, the launcher receives only the remaining time, and in-flight scans settle before the function returns. The new large-cache and final-inventory regressions confirm that timeout/cancellation stop preparation before launch and do not leave copying active after return.

Validation completed: build:test; full typecheck, lint, format, ASF headers, Windows inventory, and diff check; managed-dependency producer tests (16 passed, 1 platform skip); hosted test, package, windows_recovery, and windows_sandbox_w0_protocol; and a clean merge with current main 4a42aaeab4a7435a84c7d512e81b4d803f9ee0aa. Runtime Host full tests were 1,930 passed, 20 skipped, 1 failed, and 6 cancelled. The Bash sandbox failure reproduces on exact base because this runner cannot establish the required sandbox, and the six resumable-peer-stream cancellations also reproduce unchanged on exact base f109ccde910dd8a42d35c6a72e60cef91815819f.

I did not independently execute the native Windows supervisor or a privileged Linux bubblewrap integration on this host.

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

[Astro-Han]

Comment-only review of the supervision design. No approval or change request is implied.

What I verified

Focused on the supervisor, settlement boundary and resource lifecycle.

• No spin-loop, no unbounded retry. There is deliberately no restart/backoff policy — one npm ci attempt per call, which is correct for a producer whose staging is exclusive and whose lockfile already pins the tree. The monitor (managed-dependency-producer.ts:210-222) serializes its scans and only arms the next setTimeout inside .finally(), so the "non-overlapping 250 ms scan" claim in docs/architecture/...zh-CN.md §7 holds. windows_launcher.rs::wait_for_empty_job sleeps 10 ms between polls with a hard deadline; settle_job bounds the drain grace to 5 s then terminates and fails closed; control/registration.ts retries rename at most 5 times with 10/20/40/80/160 ms backoff and only for EPERM/EACCES/EBUSY on win32.
• Settlement boundary is genuinely two-sided. managed-dependency-process.ts only reports settled when the launcher exits 0 and a structurally valid report is read (Linux: exit-code from bubblewrap's --json-status-fd 3 must equal the process exit code; Windows: the strict exitCode,failed,settled key set in decodeProducerProcessResult). I checked the exit-code key against upstream bubblewrap (bubblewrap.c:469: { "exit-code": %i }) — it is correct, and taking documents.at(-1) correctly skips bubblewrap's multi-part setup status object. A signal-terminated child, a status overflow, or a missing/mismatched report all become unsettled.
• Failure containment holds. An adapter throw, a rejected launch, or any preparation error cannot escape runManagedDependencyProducer as a rejection; every path returns a discriminated result. producer_supervisor.rs:57-62 keeps the cancellation event alive via an Arc held by the blocked reader thread, so the WAIT_OBJECT_0 + 1 wake cannot be a stale/reused handle if npm exits before the control stream closes — that race is real and is handled correctly.
• Shutdown ordering is sound. finally at managed-dependency-producer.ts:259-263 stops the monitor, clears its timer and awaits the in-flight scan before the final inventory; the outer finally then clears the deadline timer and removes the abort listener. managed-dependency-linux-init.ts:33-40 maps every teardown route (stdin byte, stdin EOF, spawn error, child exit, signal-kill) onto a concrete exit code, and PID-1 exit makes the kernel kill and reap the namespace, so the missing wait() reaping is not a leak.
• Windows stdin ownership is correctly split: windows_launcher.rs::create_appcontainer_child(producer_stdin) replaces the AppContainer child's stdin with NUL while the supervisor retains the Host pipe, so npm cannot consume a cancellation byte.
• Crash/restart test coverage is real, not mocked, for the part that matters: windows_job_tests.rs runs against a genuine atomic-bound Job with a live descendant that has closed stdio and proves no writes occur after settlement for both root-first exit and injected cancellation, plus a real breakaway refusal. scripts/managed-dependency-producer.integration.test.mjs covers detached descendants, cancellation teardown, owner death and a native execution timeout.
• Repo conventions: ASF headers present on all 8 new files; docs/windows-test-inventory.md arithmetic is consistent (27→30 gap, 95→98 total, matching the 3 new skip declarations in oauth-execution-authority.test.ts); @maka/storage is already a runtime-host dependency, so the stable-storage import is legal; no Biome rule in the allowlist is tripped.

Minor findings

1. managed-dependency-process.ts:59,220-223 — the control temp directory is never reclaimed on unsettled or pre-spawn-failure results.

const controlRoot = await mkdtemp(join(tmpdir(), 'maka-producer-control-'));
...
} finally {
  if (confirmed) await rm(controlRoot, { recursive: true, force: true });
}

confirmed = result.settled, so every unsettled outcome — cancellation, an unprovable exit code, a killed launcher, i.e. exactly the crash paths this PR exists to model — leaves a maka-producer-control-* directory in os.tmpdir() forever. On Windows it also retains launch.json (the full launch manifest with absolute staging/npm/cache paths) and any partial result.json. The same leak occurs if the Windows manifest writeFile throws, since confirmed is still false. Nothing records the path, so neither the caller nor an operator can reclaim it.

This may be intentional forensic retention mirroring the staging rule, but unlike staging it is undocumented (§7 says nothing about it) and has no bound or reclamation path. Either always clean it up on the pre-spawn error paths and document the unsettled retention, or return the retained path in the diagnostic.

2. producer_supervisor.rs:42-54,76-78 + main.rs:63-71 — pre-child supervisor failures are reported as unsettled.

request.validate(), the create_new report open and CreateEventW all fail before any child exists, and run() returns Err, which main turns into ExitCode::FAILURE with no report file. managed-dependency-process.ts:202 then treats any non-zero launcher exit as settled: false → { kind: 'unsettled', reason: 'supervision' }.

The consequence is that a provably settled, no-child failure (rejected request, unwritable report path, event-creation failure) is indistinguishable from a lost process tree and forces indefinite staging retention under the PR's own contract. The trigger is narrow today — the TS side clamps timeoutMs into MIN..MAX, dedupes readRoots, and requireDirectory already rejects non-canonical separators that validate_path would reject — but it is a real classification hole, and a launcher that is killed mid-flight lands in the same bucket by design. Smallest sound fix: write {"settled":true,"exitCode":null,"failed":true} into the report for the three pre-child error paths so the host can tell "never started" from "could not prove teardown".

3. managed-dependency-producer.ts:210-222 — the quota monitor's cost scales with the tree it is policing.

measureProducerTree(input.projectRoot, ...) walks the entire staging tree (npm output, scratch, copied cache) and the next scan is armed 250 ms after the previous one completes. At the documented defaults (250,000 entries / 2 GiB) a scan can take seconds, so scans become effectively back-to-back full lstat+opendir walks competing for I/O with the npm ci they are supervising. Worth either a minimum-interval floor when a scan exceeds the interval, or cheaper incremental accounting. §7 documents "no overlap" but not that the interval is scan-duration-bound.

Nits

• managed-dependency-producer.ts:247-254,264-267 — the 64 KiB window keeps the head of npm's output, but npm ci prints its actionable npm error ... at the end. Keeping a bounded head+tail would make diagnostic much more useful. Relatedly, outputTruncated is computed but never surfaced in the result, so a caller cannot tell the diagnostic was cut.
• managed-dependency-producer.ts:256-258 — } catch { result = { settled: false }; } discards the adapter error entirely; a genuine adapter bug (TypeError, bad path) becomes unsettled with no evidence. redactSecrets(error.message) into the diagnostic would cost nothing and matches what the preparation phase already does at :202.
• experiments/windows-sandbox/launcher/src/main.rs:76-79 — the usage string still reads [--atomic|--broker-validate] <request.json> and does not mention --producer-supervise or --producer-breakaway-probe (it also predates --self-probe, so this is pre-existing drift).
• plugin-composition-store.ts / plugin-package-store.ts — consolidating onto @maka/storage/stable-storage's syncDirectory is the right call, but note that the shared helper is a no-op on win32 while both removed local implementations ran open(dir,'r') + sync() unconditionally. That is a silent Windows durability delta for plugin publication; it is disclosed in the PR body but not in the diff, so it deserves an explicit line in the description. Nine local duplicates remain and are candidates for the same consolidation (control/registration.ts:118, control/startup-diagnostic.ts:363, client/client-instance-identity.ts:95, peer-mesh/store.ts:743, peer-reachability/publisher.ts:282, operator/local-deployment-owner.ts:818, server/access-credential-store.ts:591, server/skill-catalog-repository.ts:1905, server/skill-catalog-transaction.ts:1173) — check each, since several are error-tolerant variants rather than drop-in equivalents.
• Test hygiene: windows_job_tests.rs::run_settlement_case builds its maka-job-drain-* root inline and only calls remove_dir_all on the success path, so any failing assert! leaks the fixture directory (the TS tests use t.after(...) for this).
• Test gaps worth one more case each: no test for finding #2 (launcher starts, exits non-zero, writes no report), no test asserting the control-dir lifecycle from finding #1, and the "non-overlapping scan" invariant is asserted only in prose — the unit tests mock launch, so a scan that outlives its 250 ms window is not exercised.

Required conclusion

1. Is the current solution optimal for the actual problem? Largely yes. It reuses launch_appcontainer_settled/settle_job rather than adding a parallel launch path, and the settled/unsettled split is the right contract for the Storage handoff. The reservations are the two resource-lifecycle gaps above, not the design.
2. Production code that can be deleted: the nine remaining local syncDirectory duplicates in runtime-host (fold into @maka/storage/stable-storage after checking each variant). Beyond that, none identified — managed-dependency-linux-init.ts and decodeProducerProcessResult are both load-bearing.
3. Low-quality tests that can be deleted: none identified. windows_job_tests.rs and the real-npm ci integration tests are the strongest evidence in the PR; registration-replacement.test.ts is well-constructed (it asserts no unlink of the old snapshot and the exact retry counts of 3/6/1 per platform).
4. Is a deeper refactor required? No. The ownership and protocol boundaries are in the right places; PR 4's Host lifecycle/quarantine wiring is the natural next seam.
5. Ready to merge? Not my call — I am commenting only. Findings #1 and #2 should be addressed or explicitly accepted first.
6. Residual risks / verification gaps: Windows behavior is attested by the author's local runs and is not reproducible here; macOS correctly refuses before touching staging. On unsettled results, staging and the Windows AppContainer profile/ACL ledger are retained with no reclamation path, so repeated crashes accumulate both. I could not run the suites, so all green-check claims are unverified by me.

Review-relevant risks and approval boundary

No user-visible behavior, public protocol, schema, release, packaging or licensing surface is changed by this diff — the modules are not wired into server/index.ts and no capability or consumer is installed. The plugin syncDirectory consolidation does change Windows durability behavior for plugin publication (see nits), and the retained Windows launch.json keeps absolute host paths in os.tmpdir() (mode 0o600, created in a 0o700 mkdtemp directory, so exposure is limited). Neither is a protected-area change requiring independent human review beyond the normal process.

AI review is not independent human review and is not an authorization to merge. A maintainer makes the final merge decision.

[me2seeks]

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

Summary

Supervises managed dependency producers in the Windows sandbox launcher: a new producer_supervisor.rs opens the private report file before starting any child (AppContainer receives grants only to runtime/staging roots, never the control directory — documented in place), and the Cancellation handle is held until EOF/cancellation with Drop closing it, eliminating the close/reuse race if npm finishes before the control stream closes. TS-side runtime-host composition/canonical-projection tests, the ACL ledger, broker pipe, protocol, and windows job tests move in step, plus the zh-CN storage-authority doc and Windows test inventory. The issue (producers running unsupervised, reports visible to the sandboxed child, handle lifetime races) is real, and the layering (supervisor above the existing settled launcher) is right. CI is fully green: package, test, windows_recovery, windows_sandbox_w0_protocol.

Findings

1. [P3] run() calls create_new(true) for the report — a second supervised run for the same producer fails with "file exists" rather than superseding, which is conservative and correct for audit, but confirm a test pins the intended behavior when a prior report exists (supersede vs fail), since a wedged producer that retries would hit this path.
2. [P3] CloseHandle in Drop on a raw usize handle: if the supervisor thread panics between event creation and reader spawn, the handle still closes correctly — good — but the raw-handle-as-usize pattern bypasses ownership types; a windows::Win32::Foundation::HANDLE-typed newtype would make the invariant compile-time. Rust-experiments scope, non-blocking.

Verdict

merge-ready — careful handle/report ordering with the sandbox grant boundary documented and fully green Windows CI; two P3 notes only.

[github-actions]

This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed.

Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the pinned label.

[Astro-Han]

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

I reviewed a456ef71efd222ccc6e8fa7ac4d89f347764c6fd against the previously reviewed 08182f00 and both merge parents. The only new commit merges main 1751c399. The previous full-lifecycle deadline/cancellation fix remains intact. I found one new P2 validation failure and one P3 documentation regression in the conflict resolutions.

P2 — regenerate the Windows skip inventory. The merge retained the three Windows exclusions in packages/runtime-host/src/__tests__/oauth-execution-authority.test.ts:217,262,290, but selected main's inventory without their rows. The missing entries are the published OAuth lease-claim, refresh-finalization and lease-release reconciliation tests. A fresh inventory has 136 excluded declarations and 30 windows-backend-gap entries, not the committed 133 and 27. Both main's new entries and these three PR entries are needed. npm run windows:inventory:write restores exactly those three rows and the two counts; the valid platform exclusions should not be removed to satisfy the check. I reproduced the stale-inventory failure locally, while an isolated export of the actual merged main parent passes. This is also the failure in CI run 38835403130, before dependency installation/build, not a product test failure.

P3 — restore the settled/unsettled distinction in the failure matrix. The merge replaced the two producer-interruption rows with unconditional staging cleanup at docs/architecture/managed-dependency-storage-authority-v1.zh-CN.md:116. That contradicts section 7 and the unchanged runner: without proof that the process tree has exited, staging must be retained and must not reach ordinary Storage rejection cleanup. Restore the separate confirmed-exit and unconfirmed-exit outcomes. This is a documentation regression, not evidence that execution now deletes unsettled staging; a real local supervisor-failure probe still returns unsettled and leaves staging present.

The other conflict resolutions preserve both sides' behavior:

• The node-pty patch retains main's exit-notification/exit-code handling and adds the PR's input-pipe and output-worker cleanup. The patches apply cleanly. Executing the patched agent's JavaScript lifecycle methods with resource stand-ins checks both behaviors; removing either one makes its assertion fail. This is not native Windows validation.
• The transcript-reader tests match main's guarded store-opening/closing logic, which also preserves the PR's cleanup intent when setup succeeds or fails.
• The dependency document retains main's four crash failpoints and the producer section, apart from the matrix regression above. The Windows inventory is the remaining lost-merge-content problem.

The producer modules and Rust supervisor sources are unchanged from 08182f00. The current regressions still enforce cancellation during copying and the final inventory deadline. Loading the actual pre-fix a23a4509 producer makes both selected regressions fail with completed instead of failed. Current plugin-package handling also retains main's optional Runtime/Client entries alongside the shared directory-sync helper. No production consumer of this producer has appeared.

Earlier limitations are not fixed by this merge: unsettled control directories still have no returned retention path or reclamation policy, and specific Windows launcher errors before child creation still cannot produce a settled report. A missing supervisor executable, however, is already classified as a settled pre-spawn failure and its control directory is cleaned; it should not be conflated with those Windows paths. I verified the local process-result and retention paths, but did not execute the Windows-native early-error cases or benchmark full-tree quota scans.

Validation: the clean workspace build, Runtime/Runtime Host type checks, checks for 31 changed TypeScript/JavaScript files, and protocol epoch guard pass. The focused Host/Runtime group has 245 passed, 2 skipped, 1 failed; its Bash-sandbox failure also reproduces with the actual main Host implementation on this runner. Storage authority/crash tests have 20 passed, 4 skipped. Script tests have 4 passed, 9 skipped. Bubblewrap is unavailable here and the kernel rejects user-namespace setup, so I did not rerun native Linux containment/npm integration; Windows-native/Rust and Desktop E2E were not run locally.

There are no Runtime Host protocol changes against merged main 1751c399; epoch 222 is unchanged. Latest fetched main a91a9adc contains the additional release-recovery change and merges cleanly by static check, but I did not execute that synthetic merge tree. GitHub reports this PR as mergeable but blocked: test is red for the P2 above, while Windows packaging, recovery and sandbox protocol checks passed.

[Astro-Han]

[P2] Regenerate the Windows inventory for these three exclusions

These POSIX directory-sync failure tests now legitimately exclude Windows, but the merge kept main's docs/windows-test-inventory.md without the three OAuth rows. The fresh counts are 136 total / 30 windows-backend-gap, versus the committed 133 / 27. This causes the early Windows test skip inventory is stale failure in CI run 38835403130; the merged main parent passes the same check. Please run npm run windows:inventory:write and commit the three reconciliation rows and updated counts, preserving the new entries from main.