morluto/rea

Fork 0

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

[GH-ISSUE #4592] [Bug] Skill recovery-backup reads lose failed-close ownership #2382

New issue

Closed

opened 2026-10-17 21:03:57 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-17 21:03:57 +00:00

Owner

Originally created by @morluto on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4592

Before submitting

Searched open and closed issues for related skill/backup/cleanup reports; #4458 covers the underlying reader, and #4581 covers uninstall backup selection. This report concerns the remaining skill consumer.
No credentials or sensitive target data included.

Area

Setup, packaging, or upgrade

Observed behavior

Controlled filesystem-boundary reproduction: when closing a skill recovery-backup read fails, installSkill([]) returns a backup-stage failure, but setup host disposal does not retry that file handle. The native descriptor remains open until the test explicitly closes it.

Reproduction

On the source revision below, add the controlled regression in tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts: create an isolated Linux-layout home with a stale shared SKILL.md and matching .rea.backup; intercept only that backup's real FileHandle.close to reject until released; call systemSetupHost(...).installSkill([]), release the close fault, then call host.close().
Executed: npm run test:focused -- tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts.
Result: 1 failed, 4 passed. The new retention assertion fails: expected 1 to be greater than 1 (close was attempted once, with no host-disposal retry). The test manually reclaims its descriptor in finally.

Expected behavior

Keep the exact failed-close owner in the setup ArtifactResourceScope; disposal must retry it. Preserve the backup-stage failure and avoid modifying the skill while backup verification is incomplete.

Impact and scope

Observed in the controlled real-file test, not a spontaneous OS close failure. Repeated failed backup reads can leave descriptors unowned. Native Windows/Linux close-failure frequency: NOT_ENOUGH_INFO; not exercised.

REA version

f9ccacca2c

Runtime and platform

Node 24.18.0, macOS arm64; injected close rejection on a real file handle, Linux setup layout.

Target and artifact context

Synthetic managed skill and recovery-backup files in a test-owned temporary home.

Interface

Package or setup

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The new test fails at the assertion that host.close adds a close attempt. Source: SetupSkill.ts at the audited revision. preserveCurrentSkillBackup calls selectCurrentRecoveryBackup without the resource scope accepted by installCanonicalSkill. The installer catches the owner-bearing reader failure and wraps it in SetupSkillInstallError; host disposal has no retained backup handle.

Analysis or suspected cause

Source-derived: duplicate select/copy/verify logic in the skill consumer omits ownership propagation. Reuse the shared preservation owner introduced by PR #4580 and pass the installer's resource scope, retaining existing verified backups without recopying them.

Additional context

No public CLI or native close-failure reproduction claimed. Related: #4458, #4581, PR #4580.

Originally created by @morluto on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4592 ## Before submitting - [x] Searched open and closed issues for related skill/backup/cleanup reports; #4458 covers the underlying reader, and #4581 covers uninstall backup selection. This report concerns the remaining skill consumer. - [x] No credentials or sensitive target data included. ## Area Setup, packaging, or upgrade ## Observed behavior Controlled filesystem-boundary reproduction: when closing a skill recovery-backup read fails, `installSkill([])` returns a backup-stage failure, but setup host disposal does not retry that file handle. The native descriptor remains open until the test explicitly closes it. ## Reproduction On the source revision below, add the controlled regression in `tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts`: create an isolated Linux-layout home with a stale shared SKILL.md and matching `.rea.backup`; intercept only that backup's real FileHandle.close to reject until released; call systemSetupHost(...).installSkill([]), release the close fault, then call host.close(). Executed: `npm run test:focused -- tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts`. Result: 1 failed, 4 passed. The new retention assertion fails: `expected 1 to be greater than 1` (close was attempted once, with no host-disposal retry). The test manually reclaims its descriptor in finally. ## Expected behavior Keep the exact failed-close owner in the setup ArtifactResourceScope; disposal must retry it. Preserve the backup-stage failure and avoid modifying the skill while backup verification is incomplete. ## Impact and scope Observed in the controlled real-file test, not a spontaneous OS close failure. Repeated failed backup reads can leave descriptors unowned. Native Windows/Linux close-failure frequency: NOT_ENOUGH_INFO; not exercised. ## REA version f9ccacca2cbf7c7f1c338cddd19160fc0a778074 ## Runtime and platform Node 24.18.0, macOS arm64; injected close rejection on a real file handle, Linux setup layout. ## Target and artifact context Synthetic managed skill and recovery-backup files in a test-owned temporary home. ## Interface Package or setup ## Earliest failure stage Cleanup, cancellation, or lifecycle ## Relevant output or evidence The new test fails at the assertion that host.close adds a close attempt. Source: [SetupSkill.ts at the audited revision](https://github.com/morluto/rea/blob/f9ccacca2cbf7c7f1c338cddd19160fc0a778074/src/application/SetupSkill.ts#L516). `preserveCurrentSkillBackup` calls selectCurrentRecoveryBackup without the resource scope accepted by installCanonicalSkill. The installer catches the owner-bearing reader failure and wraps it in SetupSkillInstallError; host disposal has no retained backup handle. ## Analysis or suspected cause Source-derived: duplicate select/copy/verify logic in the skill consumer omits ownership propagation. Reuse the shared preservation owner introduced by PR #4580 and pass the installer's resource scope, retaining existing verified backups without recopying them. ## Additional context No public CLI or native close-failure reproduction claimed. Related: #4458, #4581, PR #4580.

bsakaguchi added the

triage

bug

labels

2026-10-17 21:03:57 +00:00

bsakaguchi commented

2026-10-17 21:03:58 +00:00

Author

Owner

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026):

This appears fixed on current main (2092f90b7) by eba64e938 (the commit message references this issue).

Executed on main 2092f90b7 (macOS arm64, Node 24): npm run test:focused -- … including tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts (7 tests) — all passed (205/205 across the 13 files checked in this batch).

I did not rerun the original fault injection from the report separately; the tests above are the regressions added with the fix. Leaving closure to a maintainer.

@kaoru0822-kitauji commented on GitHub (Oct 17, 2026): This appears fixed on current main (2092f90b7) by eba64e938 (the commit message references this issue). Executed on main 2092f90b7 (macOS arm64, Node 24): `npm run test:focused -- …` including `tests/boundary/filesystem/setupSkillCleanupOwnership.test.ts` (7 tests) — all passed (205/205 across the 13 files checked in this batch). I did not rerun the original fault injection from the report separately; the tests above are the regressions added with the fix. Leaving closure to a maintainer.

bsakaguchi referenced this issue

2026-10-31 00:29:41 +00:00

[GH-ISSUE #2383] [Tracker] Audit analysis and debugging coverage for the TIOBE top 100 languages (October 2026) #794

bsakaguchi closed this issue

2026-10-31 00:33:17 +00:00

bsakaguchi commented

2026-10-31 00:33:17 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Closing as fixed on main after reconciling this report with the landed owner/consumer changes.

eba64e938: fix(setup): retain skill recovery backup cleanup ownership. Cover failed closes at first backup, recovery backup, and post-copy verification; block new descriptors while cleanup is pending and retain the exact owner through disposal retries. Related: #4592.

Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

@morluto commented on GitHub (Oct 18, 2026):  Closing as fixed on `main` after reconciling this report with the landed owner/consumer changes. - [eba64e938](https://github.com/morluto/rea/commit/eba64e9386a83b9c22a7fcee239eff4558b9afc0): fix(setup): retain skill recovery backup cleanup ownership. Cover failed closes at first backup, recovery backup, and post-copy verification; block new descriptors while cleanup is pending and retain the exact owner through disposal retries. Related: #4592. Evidence scope: this closure uses the landed source, regression coverage and validation already recorded in the linked commits/thread. No new local tests or native-provider reproduction were run during this backlog pass; previously documented platform and real-provider gaps still apply.

bsakaguchi referenced this issue

2026-10-31 01:04:46 +00:00

[PR #2400] [CLOSED] feat(native): retain LLDB source coordinates and verify Pascal call entries #5202

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#2382

No description provided.

Rows
Columns