Emit shadow Rush lifecycle events - #5991
Conversation
|
Independent R3 review against #5976/RFC phase 3: lifecycle/result ordering, phase-aware operation scope, legacy error correlation, telemetry projection, and no-raw-output behavior are correctly isolated while |
53d6d5d to
e81d5a2
Compare
|
Combined deep review of current head
|
e81d5a2 to
8255a72
Compare
|
Rebased the single R3B commit onto #5988 head Fixed Validation: build through Low follow-on: keep the outer telemetry privacy gate until #5990 reaches shared main. Then remove it and forward all envelopes to the hardened subscriber. |
|
Four-review rerun on Required fix: make final command or session completion, or current-iteration outcomes, establish the recovered success result. Add a fail-then-success watch test. Dismissible low follow-ons: reset group |
8255a72 to
2f9ac7d
Compare
|
Round two fixed at Derived exit now follows current operation outcomes and authoritative root Validation passed: build through |
|
Final combined review of One intentional low follow-on remains: remove the outer telemetry privacy gate after #5990 reaches shared ancestry. This head is low-only. |
2f9ac7d to
dd27a9c
Compare
dd27a9c to
52f751f
Compare
52f751f to
58cd2a3
Compare
58cd2a3 to
14ab214
Compare
2a6a582 to
6b2b758
Compare
3541683 to
745843b
Compare
745843b to
3776ee9
Compare
b556aec to
f7909be
Compare
Sean Larkin (TheLarkInn)
left a comment
There was a problem hiding this comment.
R3 lifecycle acceptance review against #5976: early failure and finalization outcomes must agree with authoritative native exit behavior.
| this._sessionStartTimeMs = performance.now(); | ||
| this._sessionLifecycleEmitter.emitSessionStarted({ | ||
| rushVersion: _getRushSessionReporterSourceVersion(this.rushSession)! | ||
| }); |
There was a problem hiding this comment.
[P2] Establish the root lifecycle before constructor failures can finalize the reporter. A rush.json/configuration failure is handled in the constructor, where _emitReporterCompletion(1) latches completion and closes the sink before this root emitter exists. executeAsync then returns through _initializationFailed without reaching this block. An exact-head parser reproduction returns false/exits 1 but captures zero events and leaves _getRushSessionDerivedExitStatus at succeeded/0. Errors during unassociated-plugin initialization similarly have no command emitter, so the optional diagnostic call emits nothing. Initialize or buffer the root failure lifecycle before close, use a session-scoped diagnostic when no command exists, and correlate only a diagnostic that was emitted.
There was a problem hiding this comment.
Fixed on this owning PR in f439cc6. Root session reporting is available before fallible repository initialization; failures before command selection emit a session-scoped diagnostic and consistent failure completion without changing native fatal behavior. Real malformed-config and early-hook cases failed before the fix and now pass. The owning R3B parser/session/telemetry selection passed 59 cases; downstream reporter-terminal initialization was adapted separately during the stack update. Fresh current-head CI is pending.
|
|
||
| // If we make it here, everything went fine, so reset the exit code back to 0 | ||
| process.exitCode = 0; | ||
| this._emitReporterCompletion(0); |
There was a problem hiding this comment.
[P2] Await command-finalization hooks before publishing successful completion. _emitReporterCompletion(0) runs before telemetry.ensureFlushedAsync below. If a public flushTelemetry hook rejects, the base parser returns false and changes process.exitCode to 1, but _reporterCompletionEmitted prevents the outer path from correcting the already-published commandResult/commandCompleted/sessionCompleted. A real delayed rejecting flushTelemetry hook reproduces succeeded:true/exitCode:0 in every final event and derived status, while the actual command exits 1. Defer final success emission until the hook completes and cover the rejection through the failure lifecycle.
There was a problem hiding this comment.
Fixed on this owning PR in f439cc6. Successful completion is no longer emitted from the inner execution callback before public telemetry finalization settles. A rejecting real flushTelemetry hook now produces a failure diagnostic and matching final status while preserving the base parser's native false return and nonzero exit behavior. Tests hold the public hook across an event-loop turn and cover both fulfillment and rejection. Fresh current-head CI is pending.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e
…ilures Create the root reporting context before repository setup, share correlated failure emission, and publish final completion only after telemetry hooks settle without changing native error handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4747f826-8c83-495d-80df-3d1168662e10
f7909be to
f439cc6
Compare
Part of #5976
Stack parent: #5988
Summary
RUSH_OPERATION_FAILEDdiagnostics without routing raw chunksAlreadyReportedErrorfailures so catch boundaries do not emit duplicate structured diagnosticsTelemetrySubscriberprojection into existing Rush telemetry while leaving process exit behavior authoritativeRUSH_COMMAND_FAILEDdiagnostic for representative uncorrelated command failuresValidation
rush installrush build --to @microsoft/rushrush test --only @rushstack/rush-reporter --only @microsoft/rush-lib --only @microsoft/rushrush checkrush change --verifyNo-output guarantee
Legacy terminal rendering and
StreamCollatorremain the only visible stdout/stderr owners. The shadow sink emits no terminal text, operation chunks are not routed to reporters, and focused tests compare legacy graph output with and without the reporter adapter.Non-goals
StreamCollatoror emitting the R5 raw operation/output stream