[PR #6266] [CLOSED] fix(ci): avoid utility loading in the Windows ACL observer #7241

New issue

Closed

opened 2026-11-01 21:46:26 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:26 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6266
Author: @gensokyo-dev
Created: 10/22/2026
Status: ❌ Closed

Base: main ← Head: fix/windows-acl-observer-overhead-20261022

📝 Commits (5)

37e6161 fix(ci): avoid utility loading in the Windows ACL observer
d85e0ff fix(ci): select the Windows lane for ACL observer inputs
e7c9080 fix(ci): load the ACL observer before allocating its workspace
b1c7c27 fix(ci): remove utility loading from ACL regression setup
a606f2a Merge remote-tracking branch 'origin/main' into fix/windows-acl-observer-overhead-20261022

📊 Changes

7 files changed (+232 additions, -24 deletions)

View changed files

📝 .github/workflows/ci.yml (+1 -0)
📝 docs/testing.md (+10 -3)
📝 scripts/ci/scopes.mjs (+2 -0)
➕ scripts/lib/windows-private-runtime-acl.ps1 (+33 -0)
📝 scripts/verify-windows-native.mjs (+7 -21)
📝 tests/boundary/filesystem/ciChangeScope.test.ts (+8 -0)
➕ tests/boundary/process/windowsPrivateRuntimeAclObserver.test.ts (+171 -0)

📄 Description

Summary

Remove avoidable PowerShell utility work from the Windows private-runtime ACL observer. Main's scheduled run exceeded the existing 15-second deadline: owner SID finished at954ms, rules at10,485ms, readonly at10,500ms, without completing output. The exact call/OS condition consuming those intervals was not traced.

The shared observer now reads directory security through .NET, obtains SID rules once and emits the same five boolean/count fields with utility autoloading disabled. All five real assertions, diagnostics and the15-second deadline remain. Load its source before allocating a temporary directory, and register both new files with the Windows package scope.

Boundary and verification

Verification-only change; no CLI/MCP, provider authority, Evidence, compiler-selection or installation contract change. Existing owned temporary-directory readonly mutation/cleanup remains. A separate maintainer scope decision is not applicable to this routine verifier fix. The linked main failure supplies the issue report.

Six explicit PowerShell projection scenarios cover owner mismatch, foreign trustee, unprotected DACL and empty/single/two rules on a space/Unicode path. Language-defined ACL fixtures use strict member access and traps for former utility commands. They validate projection semantics, not actual Windows ACLs.
Executed controls: baseline projection fails the utility trap; an injected .Access read fails strict property access. Both single-file planner cases fail before scope registration and pass afterward.
The first Windows attempt failed in the new fixture's utility-based setup at15s, before native verification. That fixture now constructs objects with PowerShell language features, without JSON/module/member cmdlets; its deadline and all scenarios remain. No failed-test retry or timeout increase.
Latest main2edf3b23c is merged into a606f2ae5. Executed locally:320focused tests and full npm run check pass. Node24.18.0/npm11.16.0; four-CPU affinity, serial Turbo, two checker threads. Focused task40.391s wall/56.210s CPU/755.8MiB sampled family RSS; static task23.319s/45.446s/2,024.0MiB. Warm filesystem/build caches;100ms RSS sampling can double-count shared pages or miss short children. Agent tokens and Windows CPU/memory are unmeasured. No whole-CI speedup claim.
Executed on real Windows PowerShell 5.1 in current-head PR CI: all 48 curated files pass (703 tests, 33 named skips), native verifier and package smoke pass, with Node 24/22 native suites passing 47/58 tests. The real ACL script completes its internal phases at 70 ms, excluding PowerShell startup; the 15-second subprocess deadline is unchanged. This is an observed sample, not a comparative Windows benchmark.
Full current-head qualification: portable coverage passes 25,409 tests with 201 skips (1,969 files, 25 skipped); all other platform/provider jobs pass except Windows's existing packedBridgeVerification first Python case timing out at 30 seconds, which correctly makes the required aggregate fail. That test/helper are unchanged by this PR; the independent canonical Windows lane passes all 20 packed-bridge cases in 2.420 s. No claim that the intermittent Python timeout's root cause is established or fixed, and no failed run was retried.
Latest-main integration b6007e0a5 + a606f2ae5 applies cleanly: executed check:fast and 569 focused tests in 16 files, including all newly changed main test files. Source tree 95fca0906d7fb463fb408229882b1d91a3eba556. Integration tests:132.816s wall/268.284s CPU/1,644.6MiB sampled family RSS; cache and sampling caveats above apply. Final main verification is pending.
Greptile reviewed exact a606f2ae5 at 5/5; its selector finding is corrected and resolved. Codex review remains unavailable due to service usage limits; no valid Codex review completed.

Author readiness

Human owner: @gensokyo-dev

Declarations entered under the user's explicit delegation; independent human review was not observed in this session.

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6266 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/windows-acl-observer-overhead-20261022` --- ### 📝 Commits (5) - [`37e6161`](https://github.com/morluto/rea/commit/37e61613d9bec0b79c083f21cac72cfcb1828848) fix(ci): avoid utility loading in the Windows ACL observer - [`d85e0ff`](https://github.com/morluto/rea/commit/d85e0fffe4ebe985093098903659b5c6c81be978) fix(ci): select the Windows lane for ACL observer inputs - [`e7c9080`](https://github.com/morluto/rea/commit/e7c90802e957d5c342603bac2ecb16d5bd03d3b7) fix(ci): load the ACL observer before allocating its workspace - [`b1c7c27`](https://github.com/morluto/rea/commit/b1c7c27d3cc12df495a0964a034947ea4be2bea2) fix(ci): remove utility loading from ACL regression setup - [`a606f2a`](https://github.com/morluto/rea/commit/a606f2ae527bcaa736aa6108531eb0f0a4909e2d) Merge remote-tracking branch 'origin/main' into fix/windows-acl-observer-overhead-20261022 ### 📊 Changes **7 files changed** (+232 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+1 -0) 📝 `docs/testing.md` (+10 -3) 📝 `scripts/ci/scopes.mjs` (+2 -0) ➕ `scripts/lib/windows-private-runtime-acl.ps1` (+33 -0) 📝 `scripts/verify-windows-native.mjs` (+7 -21) 📝 `tests/boundary/filesystem/ciChangeScope.test.ts` (+8 -0) ➕ `tests/boundary/process/windowsPrivateRuntimeAclObserver.test.ts` (+171 -0) </details> ### 📄 Description ## Summary Remove avoidable PowerShell utility work from the Windows private-runtime ACL observer. Main's [scheduled run](https://github.com/morluto/rea/actions/runs/39723321808/job/119648114325) exceeded the existing 15-second deadline: owner SID finished at954ms, rules at10,485ms, readonly at10,500ms, without completing output. The exact call/OS condition consuming those intervals was not traced. The shared observer now reads directory security through .NET, obtains SID rules once and emits the same five boolean/count fields with utility autoloading disabled. All five real assertions, diagnostics and the15-second deadline remain. Load its source before allocating a temporary directory, and register both new files with the Windows package scope. ## Boundary and verification Verification-only change; no CLI/MCP, provider authority, Evidence, compiler-selection or installation contract change. Existing owned temporary-directory readonly mutation/cleanup remains. A separate maintainer scope decision is not applicable to this routine verifier fix. The linked main failure supplies the issue report. - Six explicit PowerShell projection scenarios cover owner mismatch, foreign trustee, unprotected DACL and empty/single/two rules on a space/Unicode path. Language-defined ACL fixtures use strict member access and traps for former utility commands. They validate projection semantics, not actual Windows ACLs. - Executed controls: baseline projection fails the utility trap; an injected `.Access` read fails strict property access. Both single-file planner cases fail before scope registration and pass afterward. - The [first Windows attempt](https://github.com/morluto/rea/actions/runs/39734466798/job/119687441083) failed in the new fixture's utility-based setup at15s, before native verification. That fixture now constructs objects with PowerShell language features, without JSON/module/member cmdlets; its deadline and all scenarios remain. No failed-test retry or timeout increase. - Latest main2edf3b23c is merged into a606f2ae5. Executed locally:320focused tests and full `npm run check` pass. Node24.18.0/npm11.16.0; four-CPU affinity, serial Turbo, two checker threads. Focused task40.391s wall/56.210s CPU/755.8MiB sampled family RSS; static task23.319s/45.446s/2,024.0MiB. Warm filesystem/build caches;100ms RSS sampling can double-count shared pages or miss short children. Agent tokens and Windows CPU/memory are unmeasured. No whole-CI speedup claim. - Executed on real Windows PowerShell 5.1 in [current-head PR CI](https://github.com/morluto/rea/actions/runs/39739776206/job/119702221528): all 48 curated files pass (703 tests, 33 named skips), native verifier and package smoke pass, with Node 24/22 native suites passing 47/58 tests. The real ACL script completes its internal phases at 70 ms, excluding PowerShell startup; the 15-second subprocess deadline is unchanged. This is an observed sample, not a comparative Windows benchmark. - [Full current-head qualification](https://github.com/morluto/rea/actions/runs/39739769950): portable coverage passes 25,409 tests with 201 skips (1,969 files, 25 skipped); all other platform/provider jobs pass except Windows's existing `packedBridgeVerification` first Python case timing out at 30 seconds, which correctly makes the required aggregate fail. That test/helper are unchanged by this PR; the independent canonical Windows lane passes all 20 packed-bridge cases in 2.420 s. No claim that the intermittent Python timeout's root cause is established or fixed, and no failed run was retried. - Latest-main integration b6007e0a5 + a606f2ae5 applies cleanly: executed `check:fast` and 569 focused tests in 16 files, including all newly changed main test files. Source tree 95fca0906d7fb463fb408229882b1d91a3eba556. Integration tests:132.816s wall/268.284s CPU/1,644.6MiB sampled family RSS; cache and sampling caveats above apply. Final main verification is pending. - Greptile reviewed exact a606f2ae5 at 5/5; its selector finding is corrected and resolved. Codex review remains unavailable due to service usage limits; no valid Codex review completed. ## Author readiness Human owner: @gensokyo-dev Declarations entered under the user's explicit delegation; independent human review was not observed in this session. - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:46:26 +00:00