Skip to content

[rush reporter][R6A] Add bootstrap handoff and compatibility adapters - #5993

Open
Sean Larkin (TheLarkInn) wants to merge 5 commits into
copilot/reporter-r2b-frontend-host-controlsfrom
copilot/reporter-r6a-bootstrap-handoff
Open

[rush reporter][R6A] Add bootstrap handoff and compatibility adapters#5993
Sean Larkin (TheLarkInn) wants to merge 5 commits into
copilot/reporter-r2b-frontend-host-controlsfrom
copilot/reporter-r6a-bootstrap-handoff

Conversation

@TheLarkInn

Copy link
Copy Markdown
Member

Part of #5979.

Stack

graph LR
  M[main] --> R1A[#5985 R1A package wiring]
  R1A --> R1B[#5986 R1B bootstrap generation]
  R1B --> R2A[#5987 R2A experiment config]
  R2A --> R2B[#5989 R2B frontend host controls]
  R2B --> R6A[This PR: R6A bootstrap handoff]
Loading

Base: copilot/reporter-r2b-frontend-host-controls. This PR intentionally contains only the R6A delta relative to #5989 and does not enable auto-merge while the stack ancestors remain open.

Design choice

Use a minimal prelude in the existing generated install-run-rush path rather than add a reporter-specific script variant. This keeps every existing bootstrap entry point and update mechanism intact, uses the frozen zero-dependency encoder/protocol major from #5986, and gates all changed visible behavior behind an explicit reporter request or the repository experiment.

The prelude parses only early reporter/log-level controls plus the comment-aware useRushReporter experiment. It incrementally builds a 1 MiB bounded NDJSON buffer, chunks external output at 64 KiB, preserves required records, evicts only replaceable activity status, appends the RFC truncation marker, and fails if required output cannot be retained. npm stdout/stderr are captured as ordered framed records with a hard capture ceiling and persistent UTF-8 decoding.

The handoff is written under the OS temp directory with owner-only permissions where supported, an exclusive filename, and a nonce duplicated in private path/nonce environment variables. The frontend creates the authoritative host before version selection, replays then deletes the file, clears the private environment, and sweeps abandoned files using the existing retention policy.

Compatibility behavior

  • No explicit or repository opt-in: the existing bootstrap logger, inherited npm output, and legacy Rush rendering are unchanged.
  • Explicit unsupported reporter/log-level requests fail before loading rush-lib; explicit protocol-major incompatibility fails with an update/use-legacy diagnostic.
  • Implicit environment detection does not enable the pre-major path. Repository opt-in safely falls back to legacy for an older frontend or incompatible handoff, replaying bounded raw fallback output instead of losing it.
  • New frontend + old engine: legacy engine output remains visible on stderr and is bridged as ordered externalOutput events without contaminating machine-reporter stdout.
  • Old frontend + new engine: the engine receives no host sink and retains the existing LegacyFallbackSink/legacy rendering behavior.
  • Direct rush invocation has no handoff and converges on the same authoritative frontend host used by bootstrap invocation.

Validation

  • node libraries/reporter/scripts/generateBootstrapProtocol.js --check
  • node common/scripts/install-run-rush.js test --only @rushstack/rush-reporter --only @microsoft/rush-lib --only @microsoft/rush
  • Built webpack install-run npm-capture smoke test using cowsay@1.6.0 (ordered stdout/stderr frames, no overflow)
  • node common/scripts/install-run-rush.js check
  • node common/scripts/install-run-rush.js change --verify --no-fetch
  • git diff --check

Focused coverage includes direct/bootstrap startup, replay/deletion, invalid path/nonce, missing/unreadable handoffs, permissions, abandoned cleanup, truncation and required overflow, ordered external output, UTF-8 boundaries, unsupported explicit requests, implicit fallback, and old/new frontend-engine combinations.

Rollback

Revert this single commit. The optional hooks in install-run disappear with the prelude, and the default no-opt-in bootstrap/legacy path remains the unchanged fallback throughout the rollout.

Non-goals

  • R5 operation rendering or replacing StreamCollator
  • Heft child protocol/problem matcher integration
  • agent auto-selection or the Rush 6 default flip
  • removing legacy terminal APIs, rendering, or fallback behavior

@TheLarkInn

Sean Larkin (TheLarkInn) commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Independent /pr-agentmerge review completed against #5979 and RFC sections 4.3, 5.5, and 8.

Fixed in 2b8d8e5ede:

  • unsupported required bootstrap records now fail explicit reporter requests and trigger ordered legacy fallback for repository opt-in;
  • authenticated handoff files are deleted and private environment variables are cleared when frontend selection or reporter initialization fails before replay;
  • both older and newer structured engine protocol-major mismatches now fail explicit requests or receive a LegacyFallbackSink for implicit fallback, rather than an incompatible live manager sink.

Verified the minimal zero-dependency prelude, 1 MiB bounded buffering, 64 KiB UTF-8-safe external-output chunks, required-overflow failure, truncation marker, exclusive nonce/path handoff with owner-only permissions, ordered npm capture, replay/delete/retention cleanup, direct invocation, no-opt-in legacy behavior, and no R5 operation-rendering, Heft, or Rush 6 default-flip scope. The old-engine stderr bridge remains intentional to preserve machine-reporter stdout purity; default/no-opt-in output bypasses it and remains unchanged.

Validation completed: focused @rushstack/rush-reporter, @microsoft/rush, and @microsoft/rush-lib tests/builds; bootstrap generation check; rush check; rush change --verify --no-fetch; and git diff --check.

Stack remains intentionally linear with child auto-merge disabled. Required retarget order after each predecessor merges: #5985 -> retarget #5986 to main -> retarget #5987 to main -> retarget #5989 to main -> retarget #5993 to main. At each step, verify the slice-only diff, full CI, and reviews before advancing. Current external gate: #5985 is mergeable with green checks and auto-merge enabled, but still requires approval; the #5986 Windows Node 26 rerun is still pending. No review threads exist on #5993.

@TheLarkInn

Copy link
Copy Markdown
Member Author

Combined deep review of current head 2b8d8e5ede.

  1. HIGH bootstrap events classify full argv and cwd as public. Omit them or use non-public classified values.
  2. HIGH the old-engine adapter writes captured stdout back to stderr. Restore each stream to itself.
  3. MEDIUM npm install output is captured but not shown live, so long installs appear frozen. Tee while capturing.
  4. MEDIUM bootstrap reporter parsing repeats the pass-through -- bug. Stop scanning at the separator.
  5. MEDIUM rush.json discovery and version resolution occur outside the friendly error handler, producing raw stack traces. Move them inside the guarded path.

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
@TheLarkInn
Sean Larkin (TheLarkInn) force-pushed the copilot/reporter-r6a-bootstrap-handoff branch from 2b8d8e5 to c1e13cd Compare August 28, 2026 16:40
@TheLarkInn

Copy link
Copy Markdown
Member Author

Fixed all five bootstrap review findings in c1e13cd.

  • Removed argv and cwd from public bootstrap events and sanitized reporter-mode invocation output.
  • Preserved stdout and stderr separation for old engines, including async output, cleanup, machine-readable stdout, and hard-exit delivery.
  • Teed npm output live to the correct streams while retaining ordered handoff records without duplicate visible replay.
  • Stopped reporter control parsing at standalone -- and preserved pass-through arguments.
  • Moved rush.json discovery and Rush version resolution into the friendly guarded error path.

Validation: deterministic bootstrap generation; reporter build and 299 tests; rush-lib build and 774 tests; Rush frontend build and 62 tests; rush check; stacked rush change --verify; slice-only diff. Rebased the three R6 commits directly onto 62d9e7a. Auto-merge remains disabled. Ready for review.

@TheLarkInn

Copy link
Copy Markdown
Member Author

Four-review rerun on c1e13cde2c confirms the original bootstrap fixes. Two MEDIUM issues remain:

  1. Path-bearing bootstrap logger messages, including lockfile and npmrc paths, still become public activity events. Add privacy-aware logging and mark installation paths local-sensitive.
  2. Capture parsing inside finally can replace the real npm install error or fail a successful install. Report capture damage separately and never let diagnostic plumbing determine install outcome.

Auto-merge remains disabled.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e
@TheLarkInn

Copy link
Copy Markdown
Member Author

Round-two findings fixed in 04072bd.

  • Bootstrap logging now carries privacy classification. Lockfile and npmrc source, target, and deletion paths are local-sensitive; sentinel absolute-path tests confirm no path reaches public events while ordinary status remains public.
  • Npm capture corruption and partial records are discarded with one local-sensitive stderr warning. Finalization cannot replace the real npm error or fail a successful install.

Validation: deterministic generation; reporter build and 299 tests; rush-lib build and 779 tests; Rush frontend build and 62 tests; rush check; stacked change verification. PR is CLEAN/MERGEABLE and auto-merge remains disabled. Ready for review.

@TheLarkInn

Copy link
Copy Markdown
Member Author

Final four-review gate on 04072bd4f6 confirms all prior bootstrap findings are fixed. One MEDIUM gap remains: createLegacyBootstrap does not implement the new warning logger channel, so capture-damage warnings can be silently dropped on the legacy fallback path.

Required fix: route legacy warnings to stderr in quiet and non-quiet configurations, with coverage. Auto-merge remains disabled.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e
@TheLarkInn

Copy link
Copy Markdown
Member Author

Final narrow finding fixed in be34125.

Legacy capture finalization is currently unreachable because legacy bootstrap exposes no external-output handler and install-run creates a capture only when that handler exists. For future safety, both quiet and non-quiet legacy loggers now route warnings to stderr. Coverage simulates a partial capture in both modes and proves one warning, unchanged outcome, no stdout, no handoff, and cleanup.

Validation: deterministic generation; reporter build and 299 tests; rush-lib build and 781 tests; Rush frontend build and 62 tests; rush check; stacked change verification. PR is CLEAN/MERGEABLE and auto-merge remains disabled. Ready for review.

@TheLarkInn

Copy link
Copy Markdown
Member Author

Final combined review of be34125a37 found no critical, high, medium, or useful low issues. All bootstrap privacy, stream, liveness, capture, warning, handoff, cleanup, and compatibility findings are fixed with regression coverage. This head is review-clean.

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