morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4741] test(adb): bind snapshot cleanup fixtures to runtime ownership #6383

New issue

Open

opened 2026-10-31 01:14:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4741
Author: @siegfly
Created: 10/17/2026
Status: 🔄 Open

Base: main ← Head: codex/windows-adb-snapshot-fixtures

📝 Commits (10+)

2f7030a test(adb): inject snapshot cleanup faults at the owned runtime boundary
dd18084 test(ci): run ADB snapshot ownership regressions on Windows
463dede chore: sync Windows ADB fixtures with latest main
0312b7f chore: sync Windows ADB fixtures with latest main
95e112b chore: sync Windows ADB fixtures with latest main
892a3b0 chore: include latest Electron ownership fix from main
6ef69f0 chore: sync process cleanup and snapshot fixes from main
617ec8a chore: include latest JavaScript alias convergence fix
26bb012 chore: sync main for Windows ADB snapshot fixtures
65b8f9d Merge branch 'main' into codex/windows-adb-snapshot-fixtures

📊 Changes

5 files changed (+55 additions, -24 deletions)

View changed files

📝 .github/workflows/ci.yml (+1 -0)
📝 scripts/ci/scopes.mjs (+1 -0)
📝 tests/boundary/filesystem/ciChangeScope.test.ts (+12 -0)
📝 tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts (+35 -24)
📝 tests/process-global/curatedWindowsLane.test.ts (+6 -0)

📄 Description

Summary

Restore four Windows ADB snapshot-ownership regression tests by injecting cleanup faults through the owned runtime-root boundary. Run this file in the curated Windows CI lane and verify its changed-path routing.

No linked issue: isolated during independent Windows verification of #3075/#3076/#4334. This fix is separate from those PRs.

Problem and expected behavior

Windows private-root cleanup uses its native guarded owner, not fs.rm. Three fault/cancellation tests intercept fs.rm and miss the intended cleanup stage; an unconsumed interception may instead break the ordinary workspace teardown. A fourth test recognizes snapshot files with a slash-only prefix and misses the Windows handle. Tests must reach the intended final cleanup, preserve partial receipts/cancellation causes, retain failed ownership and retry the same resource on close.

On main f85a932d71279415dd95cc30083137d1161f83b8, restoring the unmodified mainline test file reproduces 12 passed / 4 failed on Windows, including the missed faults and handle-order assertion. The corrected file passes all 16 cases, with no added skips or relaxed assertions.

Change and scope

Pass the existing AdbProviderOptions.createRuntimeRoot dependency through the shared fixture. Wrap the real root's close to inject the first failure/cancellation, then delegate successful cleanup to its owner. Remove the superseded global rm interception in those cases.
Recognize immediate snapshot children with dirname(path) === runtimePath.
Add the file to package change selection and windows-curated; regressions assert both actual planner selection and the Windows invocation.

Five test/CI files change; production code is unchanged. The existing private-root dependency owns the interpretation across both hosts. Unrelated ordinary-directory rm failure tests remain intact.

Contract and boundary impact

Semantic owner: the provider-owned runtime-root lease; earliest change is test fixture fault injection.
Contribution scope: ordinary test/CI correction within existing authority; a new provider capability or advance maintainer scope decision is not applicable.
CLI/MCP schemas, Evidence/artifact contracts, target selection, production authority, installation and generated metadata: unchanged.
Test effects: read/write self-owned temporary files and exercise the real host private-directory owner. ADB execution is stubbed; no device is contacted. No binary, generated output, credential, provider database or target material is committed.

Validation

Current head 26bb012a0 includes main 3ea0d9489 / 6.7.0. Verification uses Node 24.18.0 / npm 11.16.0.

Fresh seven-file ADB/provider/ownership/CI-routing selection: Windows 274 pass / 100 existing Unix-fixture skips; Linux 374 pass / zero skips. All 16 ADB snapshot-ownership cases execute and pass on Windows. Fresh local Windows CI selection: 41 files / 532 pass / 31 existing skips. Full check, docs:check (5 README cases) and normal pre-push checks pass. Final-tree Windows package verification passes: 207 tools, packaged bridge, idempotent client setup, uninstall and exact-release update.

Synchronization and CI diagnosis

Merged latest main without conflicts. The final PR diff still contains the original five ADB-test/CI files; no independent process-fixture or Hopper correction is added. The merge-hook Flutter autofix was removed before publishing this head, and the final source tree is identical to the verified tree.

Independent fixes and limitations

Main #5224 and the later native-preparation regression update cover most of the earlier independent fixes. Two Windows test gaps remain outside these PRs: a slash-only expected snapshot path and an MCP test expecting native Windows PTY capture to open a snapshot despite the documented unsupported-capability gate. A separate minimal two-file validation copy passes 15 tests on Windows and 15 on Linux. The published independent branch remains at 5453f3cbb; no independent-fix PR is opened, and none of its commits is merged/cherry-picked into these PRs. Its older full diff must not be replayed over newer upstream assertions, particularly the Darwin/Linux preparation distinction.

Current-head CI and automated reviews are pending after synchronization. No real ADB device is contacted, and these fixture/package checks do not claim device qualification.

Hopper remains local-only and unpublished.

New-head CI follow-up

The new run is not green: Apple artifact E2E fails while cleaning the Java availability probe (Java exits 0; process identity cannot be revalidated), and both Linux recorded-crash container lanes fail the historical-PID trace oracle (Ubuntu log). The latter flags an attempted /proc/<sentinel>/stat read in a Node trace. These are different from the old fallback-cancellation assertions.

The affected verification scripts and production cleanup/recorded-crash sources are identical to main 3ea0d9489; this PR's diff remains limited to its original tests/CI scope. This is a source comparison, not clean-main runtime reproduction on macOS or those containers. The recorded-crash oracle currently classifies allowed ownership observations only in a trace containing the exact ps execution; Node-side native ownership reads are a possible explanation, not a proven root cause. No trace allowance, ownership check or test deadline was weakened. Local Windows passes do not qualify macOS or the recorded-crash lane.

Hosted Windows native-controls and NativeAOT builds pass; the Windows capability/package lane is still in progress at the follow-up check.

Completed CI and main controls (2026-10-19)

Hosted Windows capability/package finishes successfully (job 118086477747), together with native controls and NativeAOT. Overall CI is complete and red in Linux shard 2, Apple E2E, both recorded-crash containers and the Darwin arm64 package baseline. Greptile succeeds and the latest readiness workflow succeeds. The Codex summary still refers to old commit fdc593caf; it is not a current-head review result. No unresolved review threads remain.

The Darwin package failure reports cleanup_incomplete during the packaged inspect command. It is not a Windows packaging failure. No process-runtime correction is added to this ADB test/CI PR.

Upstream controls now provide runtime evidence: main 3ea0d9489 CI and main 255d906e1 CI also fail both recorded-crash containers with the same historical-PID /proc//stat assertion. Main 255d906e1 Apple E2E also reports cleanup-stage process-identity-unavailable. Main has Linux cleanup failures too; this identifies a shared failure family, not the exact cause of each PR assertion.

A local main-source owned-command diagnostic, with an exact-path read observer and no production edits, records the sentinel stat read from readProcessRelationSnapshot via createOwnedCleanupPlan even for a normal Node command with no crash input. The sentinel stays alive. This supports an oracle attribution problem: the current verifier allows those reads only in the ps executable trace, while native ownership reads now occur in Node. Full local strace/recorded-crash reproduction is unavailable (strace is not installed), and neither the whitelist nor the ownership checks have been weakened.

These main issues remain separate. No independent repair PR is created; the existing independent branch remains at 5453f3cbb and Hopper remains local-only/unpublished. These heads include main 3ea0d9489; main has since advanced to 255d906e1 with base64/byte-pattern fixes, which do not change the inspected process, JADX or trace-verifier files. No new main sync is claimed.

Author readiness

Human owner: @siegfly

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.

The human owner explicitly confirmed review of the final diff, verification and limitations, responsibility for the complete PR, and initial readiness on 2026-10-17.

_{🔄 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/4741 **Author:** [@siegfly](https://github.com/siegfly) **Created:** 10/17/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/windows-adb-snapshot-fixtures` --- ### 📝 Commits (10+) - [`2f7030a`](https://github.com/morluto/rea/commit/2f7030a12823997e006061633de3a91bb2faf556) test(adb): inject snapshot cleanup faults at the owned runtime boundary - [`dd18084`](https://github.com/morluto/rea/commit/dd18084fd1015dc777ea91fac7e68a09340854a0) test(ci): run ADB snapshot ownership regressions on Windows - [`463dede`](https://github.com/morluto/rea/commit/463dedeca1e3b052095f2e0dff805aff71759dfa) chore: sync Windows ADB fixtures with latest main - [`0312b7f`](https://github.com/morluto/rea/commit/0312b7f89909ea38e28f2cfa4b1e7be7076d3358) chore: sync Windows ADB fixtures with latest main - [`95e112b`](https://github.com/morluto/rea/commit/95e112bceea4baacf424a01db822ccd03f5cc756) chore: sync Windows ADB fixtures with latest main - [`892a3b0`](https://github.com/morluto/rea/commit/892a3b0eefae04930d91c0d4bc4054fca39d5f8c) chore: include latest Electron ownership fix from main - [`6ef69f0`](https://github.com/morluto/rea/commit/6ef69f0120bea5c83ab553e829742badde093ccf) chore: sync process cleanup and snapshot fixes from main - [`617ec8a`](https://github.com/morluto/rea/commit/617ec8afe2735617b6a8f42715b2421c48ecd65d) chore: include latest JavaScript alias convergence fix - [`26bb012`](https://github.com/morluto/rea/commit/26bb012a00e135e382c3335922b69000f3913449) chore: sync main for Windows ADB snapshot fixtures - [`65b8f9d`](https://github.com/morluto/rea/commit/65b8f9dfac74e4b83acd7e282180d0dbff76f909) Merge branch 'main' into codex/windows-adb-snapshot-fixtures ### 📊 Changes **5 files changed** (+55 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+1 -0) 📝 `scripts/ci/scopes.mjs` (+1 -0) 📝 `tests/boundary/filesystem/ciChangeScope.test.ts` (+12 -0) 📝 `tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts` (+35 -24) 📝 `tests/process-global/curatedWindowsLane.test.ts` (+6 -0) </details> ### 📄 Description ## Summary Restore four Windows ADB snapshot-ownership regression tests by injecting cleanup faults through the owned runtime-root boundary. Run this file in the curated Windows CI lane and verify its changed-path routing. No linked issue: isolated during independent Windows verification of #3075/#3076/#4334. This fix is separate from those PRs. ## Problem and expected behavior Windows private-root cleanup uses its native guarded owner, not fs.rm. Three fault/cancellation tests intercept fs.rm and miss the intended cleanup stage; an unconsumed interception may instead break the ordinary workspace teardown. A fourth test recognizes snapshot files with a slash-only prefix and misses the Windows handle. Tests must reach the intended final cleanup, preserve partial receipts/cancellation causes, retain failed ownership and retry the same resource on close. On main `f85a932d71279415dd95cc30083137d1161f83b8`, restoring the unmodified mainline test file reproduces **12 passed / 4 failed on Windows**, including the missed faults and handle-order assertion. The corrected file passes all 16 cases, with no added skips or relaxed assertions. ## Change and scope - Pass the existing AdbProviderOptions.createRuntimeRoot dependency through the shared fixture. Wrap the real root's close to inject the first failure/cancellation, then delegate successful cleanup to its owner. Remove the superseded global rm interception in those cases. - Recognize immediate snapshot children with dirname(path) === runtimePath. - Add the file to package change selection and windows-curated; regressions assert both actual planner selection and the Windows invocation. Five test/CI files change; production code is unchanged. The existing private-root dependency owns the interpretation across both hosts. Unrelated ordinary-directory rm failure tests remain intact. ## Contract and boundary impact - Semantic owner: the provider-owned runtime-root lease; earliest change is test fixture fault injection. - Contribution scope: ordinary test/CI correction within existing authority; a new provider capability or advance maintainer scope decision is not applicable. - CLI/MCP schemas, Evidence/artifact contracts, target selection, production authority, installation and generated metadata: unchanged. - Test effects: read/write self-owned temporary files and exercise the real host private-directory owner. ADB execution is stubbed; no device is contacted. No binary, generated output, credential, provider database or target material is committed. ## Validation Current head **26bb012a0** includes main **3ea0d9489 / 6.7.0**. Verification uses Node 24.18.0 / npm 11.16.0. Fresh seven-file ADB/provider/ownership/CI-routing selection: **Windows 274 pass / 100 existing Unix-fixture skips; Linux 374 pass / zero skips**. All 16 ADB snapshot-ownership cases execute and pass on Windows. Fresh local Windows CI selection: **41 files / 532 pass / 31 existing skips**. Full check, docs:check (5 README cases) and normal pre-push checks pass. Final-tree Windows package verification passes: 207 tools, packaged bridge, idempotent client setup, uninstall and exact-release update. ## Synchronization and CI diagnosis Merged latest main without conflicts. The final PR diff still contains the original five ADB-test/CI files; no independent process-fixture or Hopper correction is added. The merge-hook Flutter autofix was removed before publishing this head, and the final source tree is identical to the verified tree. ## Independent fixes and limitations Main #5224 and the later native-preparation regression update cover most of the earlier independent fixes. Two Windows test gaps remain outside these PRs: a slash-only expected snapshot path and an MCP test expecting native Windows PTY capture to open a snapshot despite the documented unsupported-capability gate. A separate minimal two-file validation copy passes 15 tests on Windows and 15 on Linux. The published independent branch remains at 5453f3cbb; no independent-fix PR is opened, and none of its commits is merged/cherry-picked into these PRs. Its older full diff must not be replayed over newer upstream assertions, particularly the Darwin/Linux preparation distinction. Current-head CI and automated reviews are pending after synchronization. No real ADB device is contacted, and these fixture/package checks do not claim device qualification. Hopper remains local-only and unpublished. ## New-head CI follow-up The new run is not green: [Apple artifact E2E](https://github.com/morluto/rea/actions/runs/39259394111/job/118084177932) fails while cleaning the Java availability probe (Java exits 0; process identity cannot be revalidated), and both Linux recorded-crash container lanes fail the historical-PID trace oracle ([Ubuntu log](https://github.com/morluto/rea/actions/runs/39259394111/job/118084178826)). The latter flags an attempted `/proc/<sentinel>/stat` read in a Node trace. These are different from the old fallback-cancellation assertions. The affected verification scripts and production cleanup/recorded-crash sources are identical to main 3ea0d9489; this PR's diff remains limited to its original tests/CI scope. This is a source comparison, **not clean-main runtime reproduction on macOS or those containers**. The recorded-crash oracle currently classifies allowed ownership observations only in a trace containing the exact ps execution; Node-side native ownership reads are a possible explanation, not a proven root cause. No trace allowance, ownership check or test deadline was weakened. Local Windows passes do not qualify macOS or the recorded-crash lane. Hosted Windows native-controls and NativeAOT builds pass; the Windows capability/package lane is still in progress at the follow-up check.  ### Completed CI and main controls (2026-10-19) Hosted Windows capability/package finishes successfully (job 118086477747), together with native controls and NativeAOT. Overall CI is complete and red in Linux shard 2, Apple E2E, both recorded-crash containers and the Darwin arm64 package baseline. Greptile succeeds and the latest readiness workflow succeeds. The Codex summary still refers to old commit fdc593caf; it is not a current-head review result. No unresolved review threads remain. The Darwin package failure reports cleanup_incomplete during the packaged inspect command. It is not a Windows packaging failure. No process-runtime correction is added to this ADB test/CI PR. Upstream controls now provide runtime evidence: [main 3ea0d9489 CI](https://github.com/morluto/rea/actions/runs/39261789271) and [main 255d906e1 CI](https://github.com/morluto/rea/actions/runs/39266528335) also fail both recorded-crash containers with the same historical-PID /proc/<pid>/stat assertion. Main 255d906e1 Apple E2E also reports cleanup-stage process-identity-unavailable. Main has Linux cleanup failures too; this identifies a shared failure family, not the exact cause of each PR assertion. A local main-source owned-command diagnostic, with an exact-path read observer and no production edits, records the sentinel stat read from readProcessRelationSnapshot via createOwnedCleanupPlan even for a normal Node command with no crash input. The sentinel stays alive. This supports an oracle attribution problem: the current verifier allows those reads only in the ps executable trace, while native ownership reads now occur in Node. Full local strace/recorded-crash reproduction is unavailable (strace is not installed), and neither the whitelist nor the ownership checks have been weakened. These main issues remain separate. No independent repair PR is created; the existing independent branch remains at 5453f3cbb and Hopper remains local-only/unpublished. These heads include main 3ea0d9489; main has since advanced to 255d906e1 with base64/byte-pattern fixes, which do not change the inspected process, JADX or trace-verifier files. No new main sync is claimed.  ## Author readiness Human owner: @siegfly - [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. The human owner explicitly confirmed review of the final diff, verification and limitations, responsibility for the complete PR, and initial readiness on 2026-10-17. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:14:03 +00:00

bsakaguchi referenced this issue

2026-11-01 21:10:53 +00:00

[GH-ISSUE #3582] [Tracker] Ready to close or merge: issues and pull requests (October 2026) #1330

bsakaguchi referenced this issue

2026-11-01 21:10:54 +00:00

[GH-ISSUE #3795] [Tracker] Open documentation issues and pull requests (October 2026) #1468

bsakaguchi referenced this issue

2026-11-01 21:11:07 +00:00