[PR #4443] [MERGED] fix(filesystem): observe cancellation after file finalization #6248

Closed
opened 2026-10-31 01:13:00 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4443
Author: @N0zoM1z0
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/regular-file-finalization-4379


📝 Commits (5)

  • fce5078 fix(filesystem): observe cancellation after file finalization
  • ce876b9 test(kuna): distinguish cancellation across installation phases
  • b8f2a4a fix(filesystem): retain completed reads when cleanup fails
  • 3d20ad1 fix(filesystem): retain completed values under cancellation
  • 025fd7d fix(filesystem): bind cancellation markers to their file owner

📊 Changes

7 files changed (+543 additions, -178 deletions)

View changed files

📝 src/adb/AdbProvider.ts (+31 -20)
📝 src/application/RegularFileRead.ts (+16 -7)
📝 src/filesystem/OwnedRegularFile.ts (+35 -0)
➕ tests/boundary/filesystem/regularFileCancellation.test.ts (+233 -0)
📝 tests/boundary/filesystem/stableFileHashOwnership.test.ts (+49 -1)
📝 tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts (+125 -100)
📝 tests/boundary/providers/kuna/kunaInstallationLifetime.test.ts (+54 -50)

📄 Description

Summary

Fixes #4379. Regular-file reads observe cancellation after metadata, descriptor closure and close retries. Completed file facts remain available under cancellation, including digests of already acquired ADB files. Part of #4227.

Problem and expected behavior

An unchanged file could be returned successfully after cancellation during its final real stat or close. Checking cancellation must also retain physical observations already completed: reporting a known acquired-file digest as unknown loses evidence. An incomplete close must retain its original outcome and exact cleanup owner.

Change and scope

The descriptor owner emits RegularFileCancelled only after confirmed close, carrying the completed read outcome and caller reason separately. Existing read errors stay primary; failed close keeps RegularFileCleanupFailure and its original physical outcome. Ordinary withRegularFile readers unwrap only cancellation tied to their admitted FileHandle object, preserve exact raw abort reasons and nested callback failures, and check again after resource-scope retries; the redundant forwarding wrapper is removed. Caller-provided reason objects stay opaque.

ADB uses one digest-failure mapper for file, package and bugreport acquisition. It retains completed hashes from either cancellation or failed cleanup, without claiming cleanup uncertainty after confirmed close. The former cleanup-only wrapper/normalizer is replaced. No extra file I/O, cache or normal-path data copy is introduced.

Contract and boundary impact

  • Semantic owner: shared descriptor finalization, application-owned retries, then ADB's existing partial-observation mapping.
  • Contribution scope: existing file-read/acquisition lifecycle fix; new-capability maintainer decision not applicable.
  • CLI/MCP/catalog: shared implementation; no input, schema or success-response change. Cancellation can retain known digest fields in the existing partial-result shape.
  • Compatibility: Linux x64 executed; other local hosts and physical ADB devices unavailable.
  • Evidence: source bytes/record IDs/artifact metadata preserved; completed observations, caller cancellation and cleanup proof remain distinct.
  • Authority: existing read-only file access and caller-selected ADB acquisition/output paths; no new device, execution or cleanup authority.
  • Generated/package/installation impact: none.

Existing affected operations have these effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Change process memory, registers, files, or configuration.
  • Contact network services.

The latter effects belong to existing ADB helper/service interaction and host output files. Deterministic ADB tests use a simulated command transport and real local files; no physical-device execution is claimed.

Evidence and regression coverage

  • Original reporter probe reproduced on fresh Linux main 781d5751a: final real descriptor stat settles after abort, but bytes are returned; descriptor closes. Five finalization regressions fail on base while both physical-result/primary-failure retention controls pass.

  • Compiled probes on a genuine public Evidence export: final metadata/close cancellation preserves the exact caller reason; denied close retains payload and owner for successful retry. Completed stable-file hash remains available under cancellation after successful close. Typed Evidence consumer keeps the original reason in its cause chain.

  • ADB single-file/split-package tests preserve completed hashes through both successful close cancellation and failed cleanup, retain original abort causes, and distinguish cleared from pending cleanup. The successful-close digest regression failed on reviewed head b8f2a4ae2 before this follow-up (1 failed,14 passed).

  • Nested regression: both same-path cases (shared and independent signals) fail on reviewed head 3d20ad181 before the owner-identity fix (2 failed, 8 passed). Final compiled probes preserve the inner owner, path and completed value, and close both descriptors. Matching uses actual FileHandle identity, not path or signal equality.

  • Kuna lifetime coverage separately proves early cancellation stops before shebang admission and later cancellation keeps cleanup waiting through shebang reading.

  • Public compiled CLI/actual stdio MCP: import → filtered summaries → malformed-input recovery → complete export → close/reimport passes twice per revision, preserving all 3 record IDs and artifact metadata.

  • Gaps: exact cancellation timing is production-boundary fault injection around real filesystem calls, not a public SDK cancellation reproduction. ADB transport is simulated; no new native-engine or physical-device claim. Separate stream-abort ownership report #4381 remains outside this fix.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Final head 025fd7df471d345c71299ebe185724311b442c5a (source checks immediately before the unchanged commit):

  • npm run test:focused -- 50 related filesystem, JSON, snapshot, ADB, managed/JADX/Kuna, publisher and setup files — 570 passed.
  • npm run check:fast and npm run docs:check — passed; docs include 5 README consistency cases.
  • Exact-head compiled real-file/stable-hash cancellation, denied-close/retry and nested-owner probes — passed; original public export SHA-256 8d1e5d911b4c0b49ec33f2e4884ad7a9ae7212517237c8c46de01814e02100e8 unchanged.
  • Exact-head public CLI/MCP task — passed twice; 9 MCP calls/task. Baseline 1f5ca5ade has the identical tree to starting main 781d5751a.

Completed public-task medians, two fresh-process runs/revision: base/fix wall 4.78/4.88s, CPU 5.95/5.97s, sampled family RSS 0.640/0.645GiB. All 9 MCP request+full-reply o200k proxy totals: base 2,668–2,673, fix 2,678–2,683. Schemas/control/CLI output/model billing and physical-device overhead are unmeasured. Filesystem/module caches uncontrolled; repeated import reuses its connection. No speedup or context-reduction claim. Serial jobs use 2 CPU cores and a 3 GiB Node heap. RSS sampling may double-count shared pages or miss short children; CPU includes only reaped descendants. Raw attempts and performance-owners.json: /tmp/rea-4379-file-finalization-20261016/.

Compatibility, safety, and release

No breaking change, migration, dependency, installation or generated-file change. Descriptor ownership checks stay intact; failed cleanup is never inferred complete. Only completed observations are retained, without extra copies. No ambient environment dump was persisted. Local real-provider scope is filesystem/Evidence workflows; GUI providers and physical ADB hardware were not used for this focused fix.

Author readiness

Human owner: @N0zoM1z0

Entered under the human owner's explicit working-session instruction to complete the declarations and request review. The assistant performed implementation and validation review; this does not claim a separate manual human review.

  • 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.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 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/4443 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/regular-file-finalization-4379` --- ### 📝 Commits (5) - [`fce5078`](https://github.com/morluto/rea/commit/fce5078509456c8f1dc050cd16e331a339d23fef) fix(filesystem): observe cancellation after file finalization - [`ce876b9`](https://github.com/morluto/rea/commit/ce876b92a1fe9ceb298eb13bbc4b2c97f9619ddf) test(kuna): distinguish cancellation across installation phases - [`b8f2a4a`](https://github.com/morluto/rea/commit/b8f2a4ae25c0859d96e57f56dc73197969aa3431) fix(filesystem): retain completed reads when cleanup fails - [`3d20ad1`](https://github.com/morluto/rea/commit/3d20ad181c5d51f26b02ad3f0b6eca8cb2f9a4c4) fix(filesystem): retain completed values under cancellation - [`025fd7d`](https://github.com/morluto/rea/commit/025fd7df471d345c71299ebe185724311b442c5a) fix(filesystem): bind cancellation markers to their file owner ### 📊 Changes **7 files changed** (+543 additions, -178 deletions) <details> <summary>View changed files</summary> 📝 `src/adb/AdbProvider.ts` (+31 -20) 📝 `src/application/RegularFileRead.ts` (+16 -7) 📝 `src/filesystem/OwnedRegularFile.ts` (+35 -0) ➕ `tests/boundary/filesystem/regularFileCancellation.test.ts` (+233 -0) 📝 `tests/boundary/filesystem/stableFileHashOwnership.test.ts` (+49 -1) 📝 `tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts` (+125 -100) 📝 `tests/boundary/providers/kuna/kunaInstallationLifetime.test.ts` (+54 -50) </details> ### 📄 Description ## Summary Fixes #4379. Regular-file reads observe cancellation after metadata, descriptor closure and close retries. Completed file facts remain available under cancellation, including digests of already acquired ADB files. Part of #4227. ## Problem and expected behavior An unchanged file could be returned successfully after cancellation during its final real stat or close. Checking cancellation must also retain physical observations already completed: reporting a known acquired-file digest as unknown loses evidence. An incomplete close must retain its original outcome and exact cleanup owner. ## Change and scope The descriptor owner emits `RegularFileCancelled` only after confirmed close, carrying the completed read outcome and caller reason separately. Existing read errors stay primary; failed close keeps `RegularFileCleanupFailure` and its original physical outcome. Ordinary `withRegularFile` readers unwrap only cancellation tied to their admitted FileHandle object, preserve exact raw abort reasons and nested callback failures, and check again after resource-scope retries; the redundant forwarding wrapper is removed. Caller-provided reason objects stay opaque. ADB uses one digest-failure mapper for file, package and bugreport acquisition. It retains completed hashes from either cancellation or failed cleanup, without claiming cleanup uncertainty after confirmed close. The former cleanup-only wrapper/normalizer is replaced. No extra file I/O, cache or normal-path data copy is introduced. ## Contract and boundary impact - Semantic owner: shared descriptor finalization, application-owned retries, then ADB's existing partial-observation mapping. - Contribution scope: existing file-read/acquisition lifecycle fix; new-capability maintainer decision not applicable. - CLI/MCP/catalog: shared implementation; no input, schema or success-response change. Cancellation can retain known digest fields in the existing partial-result shape. - Compatibility: Linux x64 executed; other local hosts and physical ADB devices unavailable. - Evidence: source bytes/record IDs/artifact metadata preserved; completed observations, caller cancellation and cleanup proof remain distinct. - Authority: existing read-only file access and caller-selected ADB acquisition/output paths; no new device, execution or cleanup authority. - Generated/package/installation impact: none. Existing affected operations have these effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Change process memory, registers, files, or configuration. - [x] Contact network services. The latter effects belong to existing ADB helper/service interaction and host output files. Deterministic ADB tests use a simulated command transport and real local files; no physical-device execution is claimed. ## Evidence and regression coverage - Original reporter probe reproduced on fresh Linux main `781d5751a`: final real descriptor stat settles after abort, but bytes are returned; descriptor closes. Five finalization regressions fail on base while both physical-result/primary-failure retention controls pass. - Compiled probes on a genuine public Evidence export: final metadata/close cancellation preserves the exact caller reason; denied close retains payload and owner for successful retry. Completed stable-file hash remains available under cancellation after successful close. Typed Evidence consumer keeps the original reason in its cause chain. - ADB single-file/split-package tests preserve completed hashes through both successful close cancellation and failed cleanup, retain original abort causes, and distinguish cleared from pending cleanup. The successful-close digest regression failed on reviewed head `b8f2a4ae2` before this follow-up (1 failed,14 passed). - Nested regression: both same-path cases (shared and independent signals) fail on reviewed head `3d20ad181` before the owner-identity fix (2 failed, 8 passed). Final compiled probes preserve the inner owner, path and completed value, and close both descriptors. Matching uses actual FileHandle identity, not path or signal equality. - Kuna lifetime coverage separately proves early cancellation stops before shebang admission and later cancellation keeps cleanup waiting through shebang reading. - Public compiled CLI/actual stdio MCP: import → filtered summaries → malformed-input recovery → complete export → close/reimport passes twice per revision, preserving all 3 record IDs and artifact metadata. - Gaps: exact cancellation timing is production-boundary fault injection around real filesystem calls, not a public SDK cancellation reproduction. ADB transport is simulated; no new native-engine or physical-device claim. Separate stream-abort ownership report #4381 remains outside this fix. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Final head `025fd7df471d345c71299ebe185724311b442c5a` (source checks immediately before the unchanged commit): - `npm run test:focused --` 50 related filesystem, JSON, snapshot, ADB, managed/JADX/Kuna, publisher and setup files — **570 passed**. - `npm run check:fast` and `npm run docs:check` — passed; docs include 5 README consistency cases. - Exact-head compiled real-file/stable-hash cancellation, denied-close/retry and nested-owner probes — passed; original public export SHA-256 `8d1e5d911b4c0b49ec33f2e4884ad7a9ae7212517237c8c46de01814e02100e8` unchanged. - Exact-head public CLI/MCP task — passed twice; 9 MCP calls/task. Baseline `1f5ca5ade` has the identical tree to starting main `781d5751a`. Completed public-task medians, two fresh-process runs/revision: base/fix wall 4.78/4.88s, CPU 5.95/5.97s, sampled family RSS 0.640/0.645GiB. All 9 MCP request+full-reply o200k proxy totals: base 2,668–2,673, fix 2,678–2,683. Schemas/control/CLI output/model billing and physical-device overhead are unmeasured. Filesystem/module caches uncontrolled; repeated import reuses its connection. No speedup or context-reduction claim. Serial jobs use 2 CPU cores and a 3 GiB Node heap. RSS sampling may double-count shared pages or miss short children; CPU includes only reaped descendants. Raw attempts and `performance-owners.json`: `/tmp/rea-4379-file-finalization-20261016/`. ## Compatibility, safety, and release No breaking change, migration, dependency, installation or generated-file change. Descriptor ownership checks stay intact; failed cleanup is never inferred complete. Only completed observations are retained, without extra copies. No ambient environment dump was persisted. Local real-provider scope is filesystem/Evidence workflows; GUI providers and physical ADB hardware were not used for this focused fix. ## Author readiness Human owner: @N0zoM1z0 Entered under the human owner's explicit working-session instruction to complete the declarations and request review. The assistant performed implementation and validation review; this does not claim a separate manual human review. - [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. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:13:00 +00:00
Sign in to join this conversation.
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#6248
No description provided.