[PR #4894] fix(update): drain output after progress destination closes #6461

New issue

Open

opened 2026-10-31 01:14:38 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:38 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4894
Author: @evenafterallisaid
Created: 10/17/2026
Status: 🔄 Open

Base: main ← Head: fix/update-output-budget

📝 Commits (5)

dbf1ecb fix(update): bound package-manager output collection
57a667c merge main and defer to the upstream output policy
f003008 fix(update): drain output after progress destination closes
099ae5b merge main into updater progress fix
25ab1ca merge main and keep updater process tests portable

📊 Changes

4 files changed (+177 additions, -6 deletions)

View changed files

📝 README.md (+3 -0)
📝 src/application/UpdateCommandProgress.test.ts (+90 -0)
📝 src/application/UpdateCommandProgress.ts (+18 -2)
📝 tests/process-global/updateOutput.test.ts (+66 -4)

📄 Description

follow-up to #4888.

human-mode updates can hang when a borrowed progress destination closes without an error while its write acknowledgement is still pending. the progress sink stays backpressured, pauses child output, and prevents the updater from reaching physical output closure.

the progress owner now observes destination closure, including a destination already destroyed before attachment. it fails its private sink so both child streams resume draining. error/close listeners remain owned until outstanding acknowledgements settle; late success or rejection cannot erase the original closure failure. closing the destination after completed progress preserves success. the README explains that an installation may finish despite a progress error and recommends checking the installed version before retrying.

executed validation

latest main integration, 099ae5b39: merged main 8f7d12e30, including the upstream Linux descriptor-cleanup repair that resolves the inherited failure from the earlier run. all 124 tests across nine suites passed on Windows 11 Home / Node 24.18.0, including the previously failing Ghidra verifier prerequisite case. npm run check:ci, architecture guards, npm run docs:check (five README tests), and normal hooks passed. the hosted CI run passed on this head, including all five compiled test shards, source/static/docs checks, and the selected native-package and website lanes.

Windows 11 Home, Node 24.18.0: two focused regression cases fail on main 01cdbbdc5, then pass with the patch.
a real Node subprocess writing 1 MiB also reproduces the blocked updater on that baseline; with the patch it drains and exits 0 before the delayed observer acknowledges, while the update returns the progress failure. the writer/late acknowledgement is a controlled seam, not a claim of naturally occurring OS pipe failure.
earlier validation: 115 tests across 8 suites passed, including the built public CLI update-alias acceptance test, recording package-manager workflows, output collection, late acknowledgements, already-closed destinations and completed-progress controls.
npm run check:ci passed all 8 tasks and architecture guards. npm run docs:check passed, including 5 README tests. existing lint warnings remain.

scope and limits

existing updater progress only; the upstream retention policy and package-manager launch/termination authority are preserved. real package installations and non-Windows OS execution were not performed. recording package managers do not establish real registry behavior. elapsed/CPU/peak-memory/token effects were not measured for this correctness repair; the hanging case has no completed baseline comparison.

Author readiness

Human owner: @evenafterallisaid

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/4894 **Author:** [@evenafterallisaid](https://github.com/evenafterallisaid) **Created:** 10/17/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/update-output-budget` --- ### 📝 Commits (5) - [`dbf1ecb`](https://github.com/morluto/rea/commit/dbf1ecb2d2dbfa81deee7d339edf9835e684e360) fix(update): bound package-manager output collection - [`57a667c`](https://github.com/morluto/rea/commit/57a667cfb4c8a609f70d693e13cc5672e773cb9d) merge main and defer to the upstream output policy - [`f003008`](https://github.com/morluto/rea/commit/f003008be7fcac670d7c95d8449e2a935c26b2a8) fix(update): drain output after progress destination closes - [`099ae5b`](https://github.com/morluto/rea/commit/099ae5b39fe13c533968298feeda893156710d7f) merge main into updater progress fix - [`25ab1ca`](https://github.com/morluto/rea/commit/25ab1cadac54b9cc510f334a86fcdb491adfca3f) merge main and keep updater process tests portable ### 📊 Changes **4 files changed** (+177 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+3 -0) 📝 `src/application/UpdateCommandProgress.test.ts` (+90 -0) 📝 `src/application/UpdateCommandProgress.ts` (+18 -2) 📝 `tests/process-global/updateOutput.test.ts` (+66 -4) </details> ### 📄 Description follow-up to #4888. human-mode updates can hang when a borrowed progress destination closes without an error while its write acknowledgement is still pending. the progress sink stays backpressured, pauses child output, and prevents the updater from reaching physical output closure. the progress owner now observes destination closure, including a destination already destroyed before attachment. it fails its private sink so both child streams resume draining. error/close listeners remain owned until outstanding acknowledgements settle; late success or rejection cannot erase the original closure failure. closing the destination after completed progress preserves success. the README explains that an installation may finish despite a progress error and recommends checking the installed version before retrying. ## executed validation latest main integration, `099ae5b39`: merged main `8f7d12e30`, including the upstream Linux descriptor-cleanup repair that resolves the inherited failure from the earlier run. all 124 tests across nine suites passed on Windows 11 Home / Node 24.18.0, including the previously failing Ghidra verifier prerequisite case. `npm run check:ci`, architecture guards, `npm run docs:check` (five README tests), and normal hooks passed. the [hosted CI run](https://github.com/morluto/rea/actions/runs/39106932844) passed on this head, including all five compiled test shards, source/static/docs checks, and the selected native-package and website lanes. - Windows 11 Home, Node 24.18.0: two focused regression cases fail on main `01cdbbdc5`, then pass with the patch. - a real Node subprocess writing 1 MiB also reproduces the blocked updater on that baseline; with the patch it drains and exits 0 before the delayed observer acknowledges, while the update returns the progress failure. the writer/late acknowledgement is a controlled seam, not a claim of naturally occurring OS pipe failure. - earlier validation: 115 tests across 8 suites passed, including the built public CLI update-alias acceptance test, recording package-manager workflows, output collection, late acknowledgements, already-closed destinations and completed-progress controls. - `npm run check:ci` passed all 8 tasks and architecture guards. `npm run docs:check` passed, including 5 README tests. existing lint warnings remain. ## scope and limits existing updater progress only; the upstream retention policy and package-manager launch/termination authority are preserved. real package installations and non-Windows OS execution were not performed. recording package managers do not establish real registry behavior. elapsed/CPU/peak-memory/token effects were not measured for this correctness repair; the hanging case has no completed baseline comparison. ## Author readiness Human owner: @evenafterallisaid - [ ] 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. --- <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-10-31 01:14:38 +00:00

bsakaguchi referenced this issue

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

[PR #6461] [CLOSED] fix(python): keep negative-zero pyc float constants distinct #7356