morluto/rea

Fork 0

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

[PR #6920] [MERGED] fix(setup): preserve cleanup ownership and resource reports #7448

New issue

Closed

opened 2026-11-01 21:47:55 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:55 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6920
Author: @DeryFerd
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/24/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/setup-host-cleanup-resources

📝 Commits (10+)

bb0145e fix(setup): report host-owned cleanup resources alongside the primary's
7fdfcf7 Merge remote-tracking branch 'origin/main' into review/6920-setup-cleanup-20261024
f45eec6 fix(setup): preserve cleanup ownership through host reports
49143ca Merge remote-tracking branch 'origin/main' into review/6920-setup-cleanup-20261024
9ac7aac fix(setup): bind finalization reports to cleanup owners
f682343 Merge remote-tracking branch 'origin/main' into review/6920-setup-cleanup-20261024
1ac4a1d fix(setup): retain live resources in returned cleanup views
6d3024f fix(setup): preserve completed cleanup and exact installer resources
d3c5fc1 fix(setup): retain delegated cleanup across authority changes
2edac26 fix(setup): compose callback and source cleanup authorities

📊 Changes

23 files changed (+1543 additions, -188 deletions)

View changed files

📝 docs/installation.md (+12 -0)
📝 src/application/ClientConfigurationCleanup.ts (+7 -1)
📝 src/application/LinuxHopper.test.ts (+4 -1)
📝 src/application/LinuxHopper.ts (+8 -2)
📝 src/application/MacHopper.test.ts (+29 -1)
📝 src/application/MacHopper.ts (+35 -14)
📝 src/application/RegularFileRead.ts (+5 -0)
📝 src/application/Setup.recovery.test.ts (+248 -3)
📝 src/application/Setup.ts (+32 -24)
➕ src/application/SetupCleanupOwner.test.ts (+439 -0)
➕ src/application/SetupCleanupOwner.ts (+254 -0)
📝 src/application/SetupHost.ts (+110 -116)
📝 src/application/SetupTypes.ts (+10 -2)
📝 src/artifacts/AnalysisCleanupLease.test.ts (+23 -0)
📝 src/artifacts/AnalysisCleanupLease.ts (+11 -17)
📝 src/cli/interactiveSetup.ts (+2 -0)
📝 src/domain/analysisErrorBase.ts (+16 -0)
📝 tests/boundary/cli/cliSetup.test.ts (+38 -0)
📝 tests/boundary/filesystem/macHopperInstallOwnership.test.ts (+47 -2)
📝 tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts (+8 -2)

...and 3 more files

📄 Description

Summary

Setup could retain file handles or installer directories while omitting their paths, and leave errors marked incomplete after a successful retry. Cleanup results now follow their actual owners and expose current resource paths in JSON and interactive output.

Approach

Keep completion ownership separate from the latest failure evidence. Preserve primary errors, current diagnostics and earlier still-pending owners without modifying shared or frozen caller errors.
Retry pending children while preserving completed peers; admit actual late installer outcomes through finalization. Delegated callbacks and independent owners must each confirm release.
Use the same lease-group completion lifecycle for direct and aggregate errors. Direct binding preserves static resource facts and any existing live attribution; default aggregate groups provide live attribution. Recovery before report construction is observed. Normalize returned and thrown close errors consistently; already-confirmed cleanup stays completed.
Carry Linux temporary paths and macOS DMG/staging roots from their producing owners, removing only confirmed releases. Ownerless custom-host reports remain unknown snapshots; live recovery requires explicit ownership.

Validation

Final head 62b2ee3b34e56491c2a0de824734f88e06e7c11e, integrating main d81d6b670:

461 focused tests passed across 29 files; npm run check and npm run docs:check passed. Includes real descriptors/files, partial and mixed retries, late outcomes, finalization cancellation, concurrent direct/group recovery, immutable/shared source errors, latest diagnostics, static/explicit-live direct attribution, ADB publication cleanup and compiled interactive CLI output. New regressions fail before their respective fixes.
Compiled production-boundary comparison against exact main d81d6b670, with identical manifest/backup bytes and a controlled real FileHandle close fault: main omits the path and stays incomplete after retry; the fix reports it, clears current cleanup, restores the primary category and retains history. All held descriptors are asserted closed immediately after host retry, before teardown. Input SHA-256 remains a356c7319e1477688d7bc0f26f1d6042845ea5c678bb1db29991f004e0f2fb68.
Actual compiled CLI setup --yes --client claude_code --skillClient shared --json, isolated HOME and controlled backup-close fault: exit 1, needs_human / skill_install_failed, exact retained path, unchanged inputs. Fixture registration remains local to that HOME; test teardown reclaims the descriptor.

Three paired compiled workflows, fresh processes/warm filesystem, two CPUs and alternating order: elapsed median 6.036 → 6.556 s (ranges 5.810–8.028 → 5.945–6.735 s), CPU median 4.654 → 4.913 s, maximum sampled process-family peak 315.4 → 315.8 MiB (30 ms sampling). All six runs retained and validated. Tokenx estimate for options and complete diagnostic reply 651 → 685, including ownership/current-state provenance. These variable samples do not establish a causal speed change. Actual model tokens, protocol/schema overhead, builds and OS-cold runs are unmeasured/excluded.

Real filesystem cleanup runs; installer outcomes and the primary workflow failure are controlled. Local setup probes did not install Hopper, invoke native macOS installers, launch real client applications or analyze provider targets. CI separately exercises its selected real-artifact lanes. No new installation authority or provider capability is introduced.

Current-head CI on merge preview 10b46b33d (main d87e73f2d plus this head) passed all eight portable shards, 962 source files / 17,336 tests (13 skipped), build/static/docs, Windows/macOS arm64/Linux x64/arm64 package lanes, SQLite compatibility on Node 22/24/26, and the selected real-artifact lanes. Fresh automated review reports no issues; all threads are resolved. The previously failing ADB publication file passes all 17 cases in CI. Supplemental unsplit/domain-only local source runs hit their configured 2 GiB process-family cap and are not counted as passed; complete source coverage is established by CI.

Author readiness

Human owner: @DeryFerd

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 author's existing initial-readiness declaration is retained. Follow-up technical review and validation by gensokyo-dev are AI-assisted; no new independent human review is asserted.

_{🔄 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/6920 **Author:** [@DeryFerd](https://github.com/DeryFerd) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/24/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/setup-host-cleanup-resources` --- ### 📝 Commits (10+) - [`bb0145e`](https://github.com/morluto/rea/commit/bb0145e9c7c04a1cf4113b52ec9904b6a3d7339b) fix(setup): report host-owned cleanup resources alongside the primary's - [`7fdfcf7`](https://github.com/morluto/rea/commit/7fdfcf760559902dd9efed62883bada858d9aac3) Merge remote-tracking branch 'origin/main' into review/6920-setup-cleanup-20261024 - [`f45eec6`](https://github.com/morluto/rea/commit/f45eec6d17f8b01f7d92e20b8328d25d7185c4f8) fix(setup): preserve cleanup ownership through host reports - [`49143ca`](https://github.com/morluto/rea/commit/49143ca9e9d94a34290bf1eb433c4bf71ed3e0c5) Merge remote-tracking branch 'origin/main' into review/6920-setup-cleanup-20261024 - [`9ac7aac`](https://github.com/morluto/rea/commit/9ac7aac3701dc78072f7a863217cb6bd67f441d8) fix(setup): bind finalization reports to cleanup owners - [`f682343`](https://github.com/morluto/rea/commit/f682343b49d4715f773d106581a83f9336f67369) Merge remote-tracking branch 'origin/main' into review/6920-setup-cleanup-20261024 - [`1ac4a1d`](https://github.com/morluto/rea/commit/1ac4a1d78bf61db1743bf06cca2a16bb0384f338) fix(setup): retain live resources in returned cleanup views - [`6d3024f`](https://github.com/morluto/rea/commit/6d3024f1b42a1c0ad3ea9b1ba143d531f09b109d) fix(setup): preserve completed cleanup and exact installer resources - [`d3c5fc1`](https://github.com/morluto/rea/commit/d3c5fc12124537876b49c73281a498c9df7425ac) fix(setup): retain delegated cleanup across authority changes - [`2edac26`](https://github.com/morluto/rea/commit/2edac2634627efd6413c2d2214c05881f70758b8) fix(setup): compose callback and source cleanup authorities ### 📊 Changes **23 files changed** (+1543 additions, -188 deletions) <details> <summary>View changed files</summary> 📝 `docs/installation.md` (+12 -0) 📝 `src/application/ClientConfigurationCleanup.ts` (+7 -1) 📝 `src/application/LinuxHopper.test.ts` (+4 -1) 📝 `src/application/LinuxHopper.ts` (+8 -2) 📝 `src/application/MacHopper.test.ts` (+29 -1) 📝 `src/application/MacHopper.ts` (+35 -14) 📝 `src/application/RegularFileRead.ts` (+5 -0) 📝 `src/application/Setup.recovery.test.ts` (+248 -3) 📝 `src/application/Setup.ts` (+32 -24) ➕ `src/application/SetupCleanupOwner.test.ts` (+439 -0) ➕ `src/application/SetupCleanupOwner.ts` (+254 -0) 📝 `src/application/SetupHost.ts` (+110 -116) 📝 `src/application/SetupTypes.ts` (+10 -2) 📝 `src/artifacts/AnalysisCleanupLease.test.ts` (+23 -0) 📝 `src/artifacts/AnalysisCleanupLease.ts` (+11 -17) 📝 `src/cli/interactiveSetup.ts` (+2 -0) 📝 `src/domain/analysisErrorBase.ts` (+16 -0) 📝 `tests/boundary/cli/cliSetup.test.ts` (+38 -0) 📝 `tests/boundary/filesystem/macHopperInstallOwnership.test.ts` (+47 -2) 📝 `tests/boundary/filesystem/setupClientConfigCleanupOwnership.test.ts` (+8 -2) _...and 3 more files_ </details> ### 📄 Description ## Summary Setup could retain file handles or installer directories while omitting their paths, and leave errors marked incomplete after a successful retry. Cleanup results now follow their actual owners and expose current resource paths in JSON and interactive output. ## Approach - Keep completion ownership separate from the latest failure evidence. Preserve primary errors, current diagnostics and earlier still-pending owners without modifying shared or frozen caller errors. - Retry pending children while preserving completed peers; admit actual late installer outcomes through finalization. Delegated callbacks and independent owners must each confirm release. - Use the same lease-group completion lifecycle for direct and aggregate errors. Direct binding preserves static resource facts and any existing live attribution; default aggregate groups provide live attribution. Recovery before report construction is observed. Normalize returned and thrown close errors consistently; already-confirmed cleanup stays completed. - Carry Linux temporary paths and macOS DMG/staging roots from their producing owners, removing only confirmed releases. Ownerless custom-host reports remain unknown snapshots; live recovery requires explicit ownership. ## Validation Final head `62b2ee3b34e56491c2a0de824734f88e06e7c11e`, integrating main `d81d6b670`: - **461 focused tests passed across 29 files**; **`npm run check`** and **`npm run docs:check`** passed. Includes real descriptors/files, partial and mixed retries, late outcomes, finalization cancellation, concurrent direct/group recovery, immutable/shared source errors, latest diagnostics, static/explicit-live direct attribution, ADB publication cleanup and compiled interactive CLI output. New regressions fail before their respective fixes. - Compiled production-boundary comparison against exact main `d81d6b670`, with identical manifest/backup bytes and a controlled real FileHandle close fault: main omits the path and stays incomplete after retry; the fix reports it, clears current cleanup, restores the primary category and retains history. All held descriptors are asserted closed immediately after host retry, before teardown. Input SHA-256 remains `a356c7319e1477688d7bc0f26f1d6042845ea5c678bb1db29991f004e0f2fb68`. - Actual compiled CLI `setup --yes --client claude_code --skillClient shared --json`, isolated HOME and controlled backup-close fault: exit **1**, `needs_human / skill_install_failed`, exact retained path, unchanged inputs. Fixture registration remains local to that HOME; test teardown reclaims the descriptor. Three paired compiled workflows, fresh processes/warm filesystem, two CPUs and alternating order: elapsed median **6.036 → 6.556 s** (ranges **5.810–8.028 → 5.945–6.735 s**), CPU median **4.654 → 4.913 s**, maximum sampled process-family peak **315.4 → 315.8 MiB** (30 ms sampling). All six runs retained and validated. Tokenx estimate for options and complete diagnostic reply **651 → 685**, including ownership/current-state provenance. These variable samples do not establish a causal speed change. Actual model tokens, protocol/schema overhead, builds and OS-cold runs are unmeasured/excluded. Real filesystem cleanup runs; installer outcomes and the primary workflow failure are controlled. Local setup probes did not install Hopper, invoke native macOS installers, launch real client applications or analyze provider targets. CI separately exercises its selected real-artifact lanes. No new installation authority or provider capability is introduced. Current-head CI on merge preview `10b46b33d` (main `d87e73f2d` plus this head) passed **all eight portable shards**, **962 source files / 17,336 tests** (13 skipped), build/static/docs, Windows/macOS arm64/Linux x64/arm64 package lanes, SQLite compatibility on Node 22/24/26, and the selected real-artifact lanes. Fresh automated review reports no issues; all threads are resolved. The previously failing ADB publication file passes all 17 cases in CI. Supplemental unsplit/domain-only local source runs hit their configured 2 GiB process-family cap and are not counted as passed; complete source coverage is established by CI. ## Author readiness Human owner: @DeryFerd - [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 author's existing initial-readiness declaration is retained. Follow-up technical review and validation by gensokyo-dev are AI-assisted; no new independent human review is asserted. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:55 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 00:29:32 +00:00

[GH-ISSUE #5653] [Tracker] New issue intake by area (October 2026) #3423

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7448

No description provided.

Rows
Columns