[rush reporter] Fix output ownership and idle rendering (stacked follow-up) - #6019
Open
Sean Larkin (TheLarkInn) wants to merge 4 commits into
Open
Conversation
Address current review findings on #5989, #5993, and #5997 without changing reporter defaults or wire contracts. Preserve command-owned bootstrap flags, reserve machine/file stdout, observe current-engine direct output through the existing adapter, register frontend-owned environment controls, and stop lifecycle timers on close. Extend the #5998 demo and include the missing release note for existing #5996 operation forwarding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4747f826-8c83-495d-80df-3d1168662e10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4747f826-8c83-495d-80df-3d1168662e10
Sean Larkin (TheLarkInn)
requested a review
from Ian Clanton-Thuon (iclanton)
as a code owner
September 7, 2026 13:23
Sean Larkin (TheLarkInn)
requested a review
from Ian Clanton-Thuon (iclanton)
September 7, 2026 13:23
Sean Larkin (TheLarkInn)
requested a review
from Pete Gonzalez (octogonz)
as a code owner
September 7, 2026 13:23
Sean Larkin (TheLarkInn)
requested a review
from David Michon (dmichon-msft)
September 7, 2026 13:23
Follow up #6000 and Reporter CI: Node reports S_IFIFO mode for inherited Windows pipes while Stats.isFIFO() returns false. Keep descriptor validation strict, add a mode-predicate regression, and require negative context tests to reach the acknowledgement handshake. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4747f826-8c83-495d-80df-3d1168662e10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4747f826-8c83-495d-80df-3d1168662e10
This was referenced Sep 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Address the current reporter review findings on #5989, #5993, and #5997, the Windows Heft reporter failure from #6000, and related demo/release-note follow-ups for #5998 and #5996.
This is a scoped follow-up against a reporter-only integration baseline, not another PR of the entire implementation to
main. The review diff is exactly 25 files, 549 insertions, and 90 deletions. The original Reporter PRs remain separate and are not replaced or merged by this PR.Combined daemon/client integration coverage and current-main reconciliation are tracked in #6018. This PR remains the reporter-only review slice; neither PR replaces the original component DAG or bypasses its review, CI, or release gates.
Baseline and history
user/selarkin/reporter-review-base-4747at0bf12a8be55498f8484ed98e1e78dd36ee045f03.user/selarkin/reporter-dag-4747atf5db19d95af5054314b89ab6bf555ea7030054e2.1d6c3dcfc0571414c45b2c618f68e19343dc70cb.bd461a0e8fb951ea308b41f5c70cc4b593126abfplus the isolated Windows Heft pipe fix7b0760349faba4f96ded9dc0310b40a95650bff3, also published to owning PR Integrate negotiated Heft child reporting #6000 by normal fast-forward.3b005ee1581124740a76e6df2bdb233fba51be0bin their ancestry.Current component dependencies:
The actual base of #5993 is #5988; its older R2B-only description is not the current branch relationship.
Separate component-stack alignment, September 7, 2026
Native stack 6007 is now aligned with
mainat77abfe24dc1d1df889efc3d8f467ea4dfc7cc04a. The explicitly approved root replay contained only the two unmerged R2A commits aftera6332acbeec5e420dcbfc4a84a92aeab31c0ce2a; already-merged R1B content was not replayed. Officialgh-stackcascading rebase and explicit per-branch lease push published the eight active layers. The generated build-tests RepoState conflict was regenerated from the actual unchanged lockfile/manifests, not hand-edited. R5A also received the existing daemon-forwarding release note, with no daemon runtime edit.The three ordinary dependents were synchronized with normal merge commits and an atomic, non-force push. Their exact feature patches remain unchanged and their review diffs are restored to 31 / 40 / 12 files.
3b005ee15811a60f284a5d925bba19cf627495379c19acf4e3bf1e724f18d57e66b96f00b556aec7b4bbf7909be65d3dcc02a147cc19a575d1970397f741114bf4373cbffdf669d01c84401593b826c2ac012304a0e36545d472ecf0978bce5d1382007f71081480ea4306757c01056c4ee0dba92fe7c43a2e4a8ca860e80e2c1f6b2c1cAll 12 pre-alignment heads, including merged #5986, have create-only backup refs under
user/selarkin/reporter-stack-backup-4747/20260907T194240Z-3621077d/; all 25 earlier backup refs also remain. Original authors, commit messages, feature patches, and the follow-up's original history are preserved.Targeted aligned-root/top/daemon build and test operations passed (2 + 8 + 2 operations), as did the AI/bootstrap/Heft dependent validations (2 + 6 + 8 operations), change-file verification, the frozen bootstrap generator check, and all 14 AI qualification gates. No tests, thresholds, defaults, or legacy paths were weakened.
Head-specific hosted CI is now complete. The September 7, 2026 21:45 UTC snapshot confirmed all 20 tracked runs terminal SUCCESS, with all 120 matrix jobs successful in their latest attempts. This includes every newly published component head, plus the unchanged #5990 and #6019 heads; no result is borrowed from a different commit.
3415821833834158218649,3415821949634158220118,3415821843334158218761,3415821824734158218524,3415821952234158219876,34158219645(attempt 2)34158219187,3415821957134158218386,34158219232(attempt 2)3415883807534158841901341588460533318732234934134077846Two first attempts were not green and remain part of the evidence. R10A's unchanged
Performance.test.ts3-percent wall-time gate passed in the first clone but failed in the second; R5A's unmodifiednode-core-libraryasync process-list test exceeded its existing 5000ms timeout in the second clone after passing in the first. Both affected heads independently passed their other full CI run. One job-targeted rerun request was made for each terminal failed run; GitHub executed second matrix attempts, which both passed completely. The Reporter test also passed a targeted local retest at the exact R10A head. No source, threshold, sample count, test timeout, or performance gate was changed to obtain these results.All component PRs are conflict-free, including #5988. CLA and the native-stack CodeQL checks are successful. The remaining immediate landing gate is independent maintainer approval for the native stack and privacy root, followed by dependency-ordered landing; those PRs still report
REVIEW_REQUIRED, with no unresolved review threads. Existing requests includeiclantonanddmichon-msft; no duplicate review pings were sent. Main still requires one approval and its normal status checks; no bypass was used.This operation did not change this PR's head, base, or 25-file / 549-addition / 90-deletion review diff, and did not modify #6018. No PR was merged or protection bypassed. R9/R11 and the coordinated Rush 6/default/legacy release gates remain outstanding.
Details
--separator while still rejecting invalid owned controls.RUSH_REPORTERandRUSH_LOG_LEVELas frontend-owned environment controls without activating reporters or interpreting those values in the engine.presentationTerminal; add unrefed heartbeat/repaint timers and stop them on close.Stats.isFIFO()on Windows even for valid pipes. Regular files remain rejected, and negative context tests now require an actual acknowledgement handshake.rush listJSON/file output and file-mode purge logging.How it was tested
On the exact head above, Linux with Node.js 22.23.2, September 7, 2026:
All 10 uncached build/test operations passed without warnings. All 14 AI gates passed across 13 corpus cases; aggregate AI output is 45.89% of legacy and 48.76% of plaintext. The expanded direct/Heft/non-phased demo passed.
Native Windows Node.js 26.7.0 also passed four handshake/context cases, structured output/diagnostic ordering, and real regular-file rejection using the actual Heft reporter implementation bundled with its existing dependencies.
The native runtime probe was invoked from WSL Bash as
"/mnt/c/Program Files/nodejs/node.exe" -. It reportedprocess.execPathasC:\Program Files\nodejs\node.exe,process.platformaswin32, and versionv26.7.0. That Windows process spawned its own executable withstdio: ['ignore', 'pipe', 'pipe', 'pipe', 'pipe']. For both real inherited descriptors 3 and 4, the observed values were:fs.constants.S_IFIFO4096(0x1000)fs.constants.S_IFMT61440(0xf000)fs.fstatSync(fd).mode4096(0x1000)mode & S_IFMT4096(0x1000)isFIFO()/isSocket()false/falseA native synchronous pipe write/read returned the four bytes
ack\n, and the child exited 0. These are observed values for that runtime, not an assumption that the documentation's Windows constant-availability statement applies to every Node version.Previous run
34126562266passed all four Linux matrix jobs but failed both Windows jobs inHeftChildReporter; those failures led to the pipe-mode fix above.Both historical replacement full-CI runs completed SUCCESS on the exact heads listed below. The #6000 entry is evidence for the original Windows fix, not a green claim for its newer parent-synchronized head above:
7b0760349faba4f96ded9dc0310b40a95650bff334133602891f5db19d95af5054314b89ab6bf555ea7030054e234134077846The matrix covers Node 20/22/24/26 on Ubuntu and Node 24/26 on Windows. The runs were dispatched explicitly because the automatic PR trigger only targets
main; their conclusions and head SHAs were read directly rather than inferred from a CLA-only PR rollup. Independent approval and the component/release gates remain separate requirements.Validation limitation: the optional repository-wide
startsmoke loads untargeted project configurations and stops at a missingapi-documenterrig under the targeted install. It is not claimed passed. Targeted operation/watch-parity and timer-lifecycle suites passed.Landing and release boundaries