[PR #5672] fix(artifacts): handle ASAR cancellation before output consumption #7026

New issue

Open

opened 2026-11-01 21:44:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5672
Author: @Plastic-time
Created: 10/20/2026
Status: 🔄 Open

Base: main ← Head: fix/5659-publication-stream

📝 Commits (10+)

dc353da fix(artifacts): own source errors during output setup
1dbb53e fix(artifacts): join silent source destruction before returning
595ac1c fix(artifacts): retain source cleanup failures with primary errors
2f1632e fix(artifacts): join source finalization and classify secondary failures
a45b5c4 fix(firmware): separate source cleanup from output rollback
f760f52 fix(artifacts): retain live source cleanup dependencies
0665b89 fix(artifacts): preserve source failure completion and semantics
db0e63c fix(artifacts): retain scope cleanup owners and typed failures
46ab8bf chore: sync publication cleanup fix with main
afe799d Merge main into fix/5659-publication-stream

📊 Changes

2 files changed (+116 additions, -0 deletions)

View changed files

📝 src/artifacts/AsarEntryStream.ts (+3 -0)
➕ tests/boundary/providers/native/artifactProvider.sourceCancellation.test.ts (+113 -0)

📄 Description

Problem and outcome

ASAR extraction cancelled during destination preparation can emit an unhandled stream error before its consumer attaches a listener, terminating Node instead of returning the cancellation.

Attach the observer when the stream is created, as ZIP already does; retain the error for the existing consumer/cleanup path. Final scope: three production lines plus one regression file. This follows the maintainer’s request to isolate the remaining ASAR failure.

Evidence and validation

Hosted validation for head 39f50e61b49afabf5655e9b1ecb7a8d475f380aa: CI required passed (2026-10-27 19:22:23 UTC). Rechecked on 2026-10-30.

Windows / Node 24.22.1, baseline fc7f042c8: real-ASAR extraction exited with an unhandled cancellation error. With the listener, it returned the original cause, rolled back and retried successfully.
Linux / Node 24.22.1 / npm 11.16.0, 2026-10-27: npm run test:focused passed 20 tests in six files on the PR branch with the corrected archive-digest fixture, and 22 tests on tested main 0cd7ef456 plus the patch. Covers preparation-time cancellation, denied rollback, same-owner retry and normal extraction.
Negative control on that main with the corrected fixture and no listener: Vitest exited 1 with two uncaught cancellation errors. Restoring the listener passed the cancellation/ASAR pair (5 tests).
PR-branch npm run check:fast, changed-file formatting and git diff --check passed. Main-plus-patch changed-file typed lint recorded 0 errors and 2 existing warnings.

Test selection and local validation limits

The six selected files were artifactProvider.sourceCancellation, AsarEntryStream, artifactProvider.cleanupRetry, artifactProvider.zipCancellation, artifactProvider.extractionAdmission and artifactProvider.extractionIdentity. The corrected fixture uses the archive’s actual digest.

Full local npm run check is not a pass: the older branch’s unused staticFileUrlPath export also failed on untouched d4463e6. Tested main removes it; main plus this patch passed typecheck, lint, formatting, Knip, metadata, architecture guards and production reachability, but full typed lint ended with SIGKILL.

Local full-suite and CLI/MCP end-to-end runs were not repeated; public-adapter cancellation injection and real firmware engines remain unverified. Hosted CI separately passed all eight shards.

Contract and boundary impact

ASAR owns early error observation. Existing extraction/rollback filesystem effects remain. No new ownership model, dependency, provider, network access, authority or public schema.

Prepared with AI assistance.

Author readiness

Human owner: @Plastic-time

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.

_{🔄 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/5672 **Author:** [@Plastic-time](https://github.com/Plastic-time) **Created:** 10/20/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/5659-publication-stream` --- ### 📝 Commits (10+) - [`dc353da`](https://github.com/morluto/rea/commit/dc353dae0ca69cc5d4ca355a3eb2ab42356099a6) fix(artifacts): own source errors during output setup - [`1dbb53e`](https://github.com/morluto/rea/commit/1dbb53e40255816251486b4d13b5896b6c6f9009) fix(artifacts): join silent source destruction before returning - [`595ac1c`](https://github.com/morluto/rea/commit/595ac1c6da6e0b65eae1d7b43778e19f3217a43c) fix(artifacts): retain source cleanup failures with primary errors - [`2f1632e`](https://github.com/morluto/rea/commit/2f1632eb7bacde36df3b441c022572728eb073f1) fix(artifacts): join source finalization and classify secondary failures - [`a45b5c4`](https://github.com/morluto/rea/commit/a45b5c4a009351634445a4af2a6ba8d0ee12e6c3) fix(firmware): separate source cleanup from output rollback - [`f760f52`](https://github.com/morluto/rea/commit/f760f5258ba4f151180cade235ecba5272c59f88) fix(artifacts): retain live source cleanup dependencies - [`0665b89`](https://github.com/morluto/rea/commit/0665b892b6c87fbff55b1e822bdd3e6ff0b4731e) fix(artifacts): preserve source failure completion and semantics - [`db0e63c`](https://github.com/morluto/rea/commit/db0e63ca2fcc365c044e2f6d328e8ec624dcc816) fix(artifacts): retain scope cleanup owners and typed failures - [`46ab8bf`](https://github.com/morluto/rea/commit/46ab8bffc60859be5267f88b47a7838899d59e3b) chore: sync publication cleanup fix with main - [`afe799d`](https://github.com/morluto/rea/commit/afe799d39925d2a5446b5a2310ffcca22ddd1e61) Merge main into fix/5659-publication-stream ### 📊 Changes **2 files changed** (+116 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/AsarEntryStream.ts` (+3 -0) ➕ `tests/boundary/providers/native/artifactProvider.sourceCancellation.test.ts` (+113 -0) </details> ### 📄 Description ## Problem and outcome ASAR extraction cancelled during destination preparation can emit an unhandled stream `error` before its consumer attaches a listener, terminating Node instead of returning the cancellation. Attach the observer when the stream is created, as ZIP already does; retain the error for the existing consumer/cleanup path. **Final scope: three production lines plus one regression file.** This follows [the maintainer’s request](https://github.com/morluto/rea/pull/5672#issuecomment-6428008963) to isolate the remaining ASAR failure. ## Evidence and validation Hosted validation for head `39f50e61b49afabf5655e9b1ecb7a8d475f380aa`: [CI required passed](https://github.com/morluto/rea/actions/runs/40566202396/job/122415909615) (2026-10-27 19:22:23 UTC). Rechecked on 2026-10-30. - Windows / Node 24.22.1, baseline `fc7f042c8`: real-ASAR extraction exited with an unhandled cancellation error. With the listener, it returned the original cause, rolled back and retried successfully. - Linux / Node 24.22.1 / npm 11.16.0, 2026-10-27: `npm run test:focused` passed **20 tests in six files** on the PR branch with the corrected archive-digest fixture, and **22 tests** on tested main `0cd7ef456` plus the patch. Covers preparation-time cancellation, denied rollback, same-owner retry and normal extraction. - Negative control on that main with the corrected fixture and no listener: Vitest exited **1 with two uncaught cancellation errors**. Restoring the listener passed the cancellation/ASAR pair (**5 tests**). - PR-branch `npm run check:fast`, changed-file formatting and `git diff --check` passed. Main-plus-patch changed-file typed lint recorded 0 errors and 2 existing warnings. <details> <summary>Test selection and local validation limits</summary> The six selected files were `artifactProvider.sourceCancellation`, `AsarEntryStream`, `artifactProvider.cleanupRetry`, `artifactProvider.zipCancellation`, `artifactProvider.extractionAdmission` and `artifactProvider.extractionIdentity`. The corrected fixture uses the archive’s actual digest. Full local `npm run check` is not a pass: the older branch’s unused `staticFileUrlPath` export also failed on untouched `d4463e6`. Tested main removes it; main plus this patch passed typecheck, lint, formatting, Knip, metadata, architecture guards and production reachability, but full typed lint ended with `SIGKILL`. Local full-suite and CLI/MCP end-to-end runs were not repeated; public-adapter cancellation injection and real firmware engines remain unverified. Hosted CI separately passed all eight shards. </details> ## Contract and boundary impact ASAR owns early error observation. Existing extraction/rollback filesystem effects remain. No new ownership model, dependency, provider, network access, authority or public schema. Prepared with AI assistance. ## Author readiness Human owner: @Plastic-time - [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. --- <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-11-01 21:44:49 +00:00

bsakaguchi referenced this issue

2026-11-03 13:38:54 +00:00

[PR #7026] [MERGED] docs(i18n): add complete Romanian README #7586