[PR #594] [MERGED] fix(artifacts): interrupt ZIP extraction under backpressure #2094

New issue

Closed

opened 2026-10-16 21:14:10 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:14:10 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/594
Author: @rudycelekli
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: campaign/rea-zip-backpressure-abort-20261005

📝 Commits (2)

901a716 fix(artifacts): interrupt ZIP extraction under backpressure
dd89009 docs(artifacts): record ZIP cancellation ordering and close semantics

📊 Changes

2 files changed (+67 additions, -1 deletions)

View changed files

📝 src/artifacts/ZipArtifactReader.ts (+21 -1)
➕ tests/boundary/providers/native/zipBackpressureCancellation.test.ts (+46 -0)

📄 Description

Summary

Cancel ZIP extraction promptly when an unread consumer applies backpressure.

No issue is linked: this is a newly reproduced correctness defect found while reviewing current upstream; the regression and affected production seam are included for review.

Problem and expected behavior

Opening a large stored ZIP entry and aborting after its readable buffer fills leaves the producer waiting for drain. A caller cancellation must close the stream and release the producer.

Change and scope

Connect the caller signal and stream close to the extraction controller and make the drain wait abortable. Preserve the typed cancelled adapter failure.

Contract and boundary impact

Semantic owner and earliest changed stage: ZipArtifactReader.extractStream, producer drain wait.
CLI and MCP/tool-catalog contract: existing operations and schemas remain compatible; Artifact extraction returns a cancelled failure and releases the stream instead of retaining an unread producer.
Provider, bridge, target-format, or platform compatibility: ZIP extraction on supported Node hosts.
Evidence, artifact, provenance, or reconstruction contract: existing source identity and explicit incomplete/unsupported evidence remain preserved.
Process execution, authorization, cleanup, or containment impact: Caller abort and consumer close release the extraction work and remove the abort listener.
Generated metadata (docs/product-catalog.json), package, or installation impact: no metadata or dependency changes required.

Evidence and regression coverage

Tests added or updated: tests/boundary/providers/native/zipBackpressureCancellation.test.ts.
Base reproduction or other evidence: Executed real filesystem and zip.js boundary test: upstream leaves the stream undestroyed; fixed stream is destroyed and emits ArtifactReaderFailure(reason=cancelled). Affected base is ebb4c45.
User-visible CLI/MCP output: Artifact extraction returns a cancelled failure and releases the stream instead of retaining an unread producer.
Remaining proof gaps: No real Hopper/Ghidra session was needed or run.

For evidence-bearing changes:

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

npm run check:pr — passed
npm run check:ci — passed
npm run verify:package — passed
npm pack --dry-run — passed

Focused regression and reproduction commands ran with the outcomes described above.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: No real Hopper/Ghidra session was needed or run.
Package or release metadata impact: no version, dependency, generated catalog or installation changes.
Security, privacy, process, or containment review: Caller abort and consumer close release the extraction work and remove the abort listener. Final diff contains source and regression tests; local fixtures, binaries, caches and logs are excluded.

Review checklist

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.

AI assistance was used to investigate, implement, and test this change. Commits are signed and include a DCO sign-off.

Review clarification

The caller-abort path is reproduced through real ZIP backpressure. The sole production caller, ArtifactExtraction, fully drains the reader; the consumer-close branch is defensive cleanup. The abort callback destroys the output with its typed cancelled failure before aborting the internal controller. Abort listeners dispatch synchronously, and zip.js cannot settle getData inside that dispatch. Successful stream close also aborts the controller after extraction completion, which is currently benign.

_{🔄 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/594 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `campaign/rea-zip-backpressure-abort-20261005` --- ### 📝 Commits (2) - [`901a716`](https://github.com/morluto/rea/commit/901a716860e18b9abefb142d59ee6d36e7939831) fix(artifacts): interrupt ZIP extraction under backpressure - [`dd89009`](https://github.com/morluto/rea/commit/dd8900948a49842a84d692c891739c7229198520) docs(artifacts): record ZIP cancellation ordering and close semantics ### 📊 Changes **2 files changed** (+67 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/ZipArtifactReader.ts` (+21 -1) ➕ `tests/boundary/providers/native/zipBackpressureCancellation.test.ts` (+46 -0) </details> ### 📄 Description ## Summary Cancel ZIP extraction promptly when an unread consumer applies backpressure. No issue is linked: this is a newly reproduced correctness defect found while reviewing current upstream; the regression and affected production seam are included for review. ## Problem and expected behavior Opening a large stored ZIP entry and aborting after its readable buffer fills leaves the producer waiting for drain. A caller cancellation must close the stream and release the producer. ## Change and scope Connect the caller signal and stream close to the extraction controller and make the drain wait abortable. Preserve the typed cancelled adapter failure. ## Contract and boundary impact - Semantic owner and earliest changed stage: ZipArtifactReader.extractStream, producer drain wait. - CLI and MCP/tool-catalog contract: existing operations and schemas remain compatible; Artifact extraction returns a cancelled failure and releases the stream instead of retaining an unread producer. - Provider, bridge, target-format, or platform compatibility: ZIP extraction on supported Node hosts. - Evidence, artifact, provenance, or reconstruction contract: existing source identity and explicit incomplete/unsupported evidence remain preserved. - Process execution, authorization, cleanup, or containment impact: Caller abort and consumer close release the extraction work and remove the abort listener. - Generated metadata (`docs/product-catalog.json`), package, or installation impact: no metadata or dependency changes required. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/providers/native/zipBackpressureCancellation.test.ts`. - Base reproduction or other evidence: Executed real filesystem and zip.js boundary test: upstream leaves the stream undestroyed; fixed stream is destroyed and emits ArtifactReaderFailure(reason=cancelled). Affected base is ebb4c45. - User-visible CLI/MCP output: Artifact extraction returns a cancelled failure and releases the stream instead of retaining an unread producer. - Remaining proof gaps: No real Hopper/Ghidra session was needed or run. For evidence-bearing changes: - [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 - `npm run check:pr` — passed - `npm run check:ci` — passed - `npm run verify:package` — passed - `npm pack --dry-run` — passed Focused regression and reproduction commands ran with the outcomes described above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: No real Hopper/Ghidra session was needed or run. - Package or release metadata impact: no version, dependency, generated catalog or installation changes. - Security, privacy, process, or containment review: Caller abort and consumer close release the extraction work and remove the abort listener. Final diff contains source and regression tests; local fixtures, binaries, caches and logs are excluded. ## Review checklist - [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. AI assistance was used to investigate, implement, and test this change. Commits are signed and include a DCO sign-off. ## Review clarification The caller-abort path is reproduced through real ZIP backpressure. The sole production caller, ArtifactExtraction, fully drains the reader; the consumer-close branch is defensive cleanup. The abort callback destroys the output with its typed cancelled failure before aborting the internal controller. Abort listeners dispatch synchronously, and zip.js cannot settle getData inside that dispatch. Successful stream close also aborts the controller after extraction completion, which is currently benign. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:14:10 +00:00