Skip to content

[rush reporter] Fix output ownership and idle rendering (stacked follow-up) - #6019

Open
Sean Larkin (TheLarkInn) wants to merge 4 commits into
user/selarkin/reporter-review-base-4747from
user/selarkin/reporter-dag-4747
Open

[rush reporter] Fix output ownership and idle rendering (stacked follow-up)#6019
Sean Larkin (TheLarkInn) wants to merge 4 commits into
user/selarkin/reporter-review-base-4747from
user/selarkin/reporter-dag-4747

Conversation

@TheLarkInn

@TheLarkInn Sean Larkin (TheLarkInn) commented Sep 7, 2026

Copy link
Copy Markdown
Member

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

  • Base: user/selarkin/reporter-review-base-4747 at 0bf12a8be55498f8484ed98e1e78dd36ee045f03.
  • Head: user/selarkin/reporter-dag-4747 at f5db19d95af5054314b89ab6bf555ea7030054e2.
  • The base has the same source tree as validated integration checkpoint 1d6c3dcfc0571414c45b2c618f68e19343dc70cb.
  • The head includes validated fix checkpoint bd461a0e8fb951ea308b41f5c70cc4b593126abf plus the isolated Windows Heft pipe fix 7b0760349faba4f96ded9dc0310b40a95650bff3, also published to owning PR Integrate negotiated Heft child reporting #6000 by normal fast-forward.
  • The baseline and head both retain the separately published R2A fixture fix 3b005ee1581124740a76e6df2bdb233fba51be0b in their ancestry.
  • Original authors and all twelve Reporter branch-tip histories are retained. No rebase or force push was used to construct this follow-up.

Current component dependencies:

main -> #5987 -> #5989 -> #5988 -> #5991 -> #5992 -> #5996 -> #5997
main -> #5990
#5988 -> #5993
#5997 -> #6000, #5999, #5998
#5990 is also the privacy prerequisite for #5999.

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 main at 77abfe24dc1d1df889efc3d8f467ea4dfc7cc04a. The explicitly approved root replay contained only the two unmerged R2A commits after a6332acbeec5e420dcbfc4a84a92aeab31c0ce2a; already-merged R1B content was not replayed. Official gh-stack cascading 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.

PR Before this alignment Published head
#5987 3b005ee15811 a60f284a5d92
#5989 5bba19cf6274 95379c19acf4
#5988 e3bf1e724f18 d57e66b96f00
#5991 b556aec7b4bb f7909be65d3d
#5992 cc02a147cc19 a575d1970397
#5996 f741114bf437 3cbffdf669d0
#5997 1c84401593b8 26c2ac012304
#5998 a0e36545d472 ecf0978bce5d
#5993 1382007f7108 1480ea430675
#6000 7c01056c4ee0 dba92fe7c43a
#5999 2e4a8ca860e8 0e2c1f6b2c1c

All 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.

PR Successful exact-head CI runs
#5987 34158218338
#5989 34158218649, 34158219496
#5988 34158220118, 34158218433
#5991 34158218761, 34158218247
#5992 34158218524, 34158219522
#5996 34158219876, 34158219645 (attempt 2)
#5997 34158219187, 34158219571
#5998 34158218386, 34158219232 (attempt 2)
#5993 34158838075
#6000 34158841901
#5999 34158846053
#5990, unchanged head 33187322349
#6019, unchanged head 34134077846

Two first attempts were not green and remain part of the evidence. R10A's unchanged Performance.test.ts 3-percent wall-time gate passed in the first clone but failed in the second; R5A's unmodified node-core-library async 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 include iclanton and dmichon-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

  • Probe bootstrap reporter/log-level controls leniently until frontend ownership is established. Preserve command-owned and value-less flags and the -- separator while still rejecting invalid owned controls.
  • Keep bootstrap output off file-only and command-JSON stdout, retaining authenticated handoff capture and stderr failure fallback.
  • Keep configuration discovery off stdout when an explicit reporter request will fail against an incompatible engine.
  • Reuse the existing output adapter for direct current-engine writes, reserving JSON/AI/file stdout while preserving already-rendered human output and command-specific JSON. Restore observation before reporter close.
  • Register RUSH_REPORTER and RUSH_LOG_LEVEL as frontend-owned environment controls without activating reporters or interpreting those values in the engine.
  • Route watcher presentation through presentationTerminal; add unrefed heartbeat/repaint timers and stop them on close.
  • Recognize inherited Windows reporter pipes from their file-type mode bits. Node disables Stats.isFIFO() on Windows even for valid pipes. Regular files remain rejected, and negative context tests now require an actual acknowledgement handshake.
  • Extend the direct demo to verify non-phased rush list JSON/file output and file-mode purge logging.
  • Add API/release entries, including the previously missing release note for existing Reporter operation forwarding in the daemon multiplexer. No daemon runtime code is changed by this diff.

How it was tested

On the exact head above, Linux with Node.js 22.23.2, September 7, 2026:

node common/scripts/install-run-rush.js retest \
  --only @rushstack/rush-reporter --only @microsoft/rush \
  --only @microsoft/rush-lib --only @rushstack/heft \
  --only @rushstack/rush-daemon --parallelism 4
node libraries/reporter/scripts/generateBootstrapProtocol.js --check
node libraries/reporter/scripts/runAiReporterQualification.js
node common/scripts/install-run-rush.js check
node common/scripts/install-run-rush.js change --verify --no-fetch
RUSH_PARALLELISM=4 node apps/rush/src/test/sandbox/reporter-demo/run.mjs

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 reported process.execPath as C:\Program Files\nodejs\node.exe, process.platform as win32, and version v26.7.0. That Windows process spawned its own executable with stdio: ['ignore', 'pipe', 'pipe', 'pipe', 'pipe']. For both real inherited descriptors 3 and 4, the observed values were:

Runtime observation Value
fs.constants.S_IFIFO 4096 (0x1000)
fs.constants.S_IFMT 61440 (0xf000)
fs.fstatSync(fd).mode 4096 (0x1000)
mode & S_IFMT 4096 (0x1000)
isFIFO() / isSocket() false / false

A 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 34126562266 passed all four Linux matrix jobs but failed both Windows jobs in HeftChildReporter; 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:

PR Head Full-CI run Result
#6000 7b0760349faba4f96ded9dc0310b40a95650bff3 34133602891 All 6 matrix jobs successful; last job completed September 7, 2026 at 15:19:50 UTC
#6019 f5db19d95af5054314b89ab6bf555ea7030054e2 34134077846 All 6 matrix jobs successful; last job completed September 7, 2026 at 15:23:10 UTC

The 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 start smoke loads untargeted project configurations and stops at a missing api-documenter rig under the targeted install. It is not claimed passed. Targeted operation/watch-parity and timer-lifecycle suites passed.

Landing and release boundaries

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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

1 participant