Skip to content

fix(update): keep pipe-drain proof off host console - #100763

Open
andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:fix/95971-pipe-drain-host-console-v2
Open

andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:fix/95971-pipe-drain-host-console-v2

Conversation

@andrexibiza

Copy link
Copy Markdown
Contributor

What changed and why

-SelfTestPipeDrain intentionally captures an 8 MiB flood to prove the Windows handoff drains child pipes without backpressure. Invoke-HermesStep then replayed that full capture through Write-Host; GitHub Actions throttled its 128 KiB lines, pushed the fixture over its wall-clock budget, and kept the job draining long after the fixture exited.

This change separates durable logging from host-console replay. Production callers keep the existing default. Only the fixture's flood arm suppresses raw console replay; its bytes remain in desktop-update-handoff.log and Invoke-HermesStep(...).Output, while concise progress and the final verdict remain visible.

Why this matters to users

This does not change the updater users run. It makes the real Windows watchdog proof reliable on the same hosted Windows environment that gates Hermes changes, so future pipe-drain regressions fail in CI instead of forcing maintainers to rely on suites that skip the executable fixture.

How to test

  • scripts/run_tests.sh tests/test_desktop_update_windows_pipe_drain.py tests/test_desktop_update_windows_python_handoff.py tests/test_desktop_update_windows_progress.py tests/test_desktop_update_windows_timestamp.py (12 tests total; verified serially with disjoint pytest roots on native Windows)
  • PowerShell 5.1 AST parse
  • Ruff, compileall, Windows-footgun scan, git diff --check, attribution audit

Platforms tested

  • Native Windows 11 / Windows PowerShell 5.1

TDD receipt

  • RED: unchanged production code emitted 8,391,826 bytes to fixture stdout after the fixture's own PASS marker and zero exit-code assertions succeeded.
  • GREEN: fixture stdout is bounded below 64 KiB, pipeflood| is absent from stdout, full flood bytes remain in the durable handoff log, and all four fixture arms pass.

Related work and credit

Fixes #95971

Prior Work / Attribution

Prior credit - the humans whose work this builds on, before anyone else:

Related work searched (multiple methods, all states, before finalizing this PR):

Unlinked related work (stars not bound to this PR by keywords):

Builds on: @jackulau (#90564), @OutThisLife (#90937), @fangliquanflq (#95625), and @teknium1 (#95897/#95971).
Merge-order / dependency: standalone; later same-file work must preserve this default-true parameter and fixture-only opt-out.
Duplicates: none; the live FILE-LIST and behavior searches found no existing PR that separates durable capture from host-console replay.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) platform/windows Native Windows-specific behavior or breakage area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Sep 2, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

  • Sensible change: Invoke-HermesStep gains -EchoCapturedOutput so the self-test flood arm no longer replays its output to the host console, avoiding GitHub Actions throttling, while the hand-off log still retains full output (scripts/desktop-update/windows.ps1:1132).
  • Default stays $true, so production callers are unaffected; only the self-test opts out.
  • New test assertions are consistent: flood output must NOT appear in stdout, pipeflood| lines MUST be in the hand-off log, and emitted stdout stays under 64KB.
  • Minor: Write-HandoffLog positional-arg calls with the -f formatted string rely on -f binding $Tag, $ln as a single array operand due to the parens — correct, but explicit -Message/-EchoToHost would be less fragile.

andrexibiza added a commit to andrexibiza/hermes-agent that referenced this pull request Sep 5, 2026
A delayed browser could miss the 900ms terminal event and spin forever after the updater exited. Retain terminal delivery until the page acknowledges it, bound unavailable-client teardown and failed requests, and preserve a truthful final display.

Fixes NousResearch#103747. Builds on OutThisLife and Teknium detached handoff work in NousResearch#83634 and the NousResearch#75895 quiet-window design. Continues Axl Ibiza Windows update investigation (NousResearch#60233, NousResearch#94107, NousResearch#100763), including source/review contributions carried by merged NousResearch#93353 and NousResearch#85170. Existing NousResearch#102373, NousResearch#103140, NousResearch#95719, NousResearch#97299 and NousResearch#103632 retain their separate scopes.
teknium1 pushed a commit that referenced this pull request Sep 6, 2026
A delayed browser could miss the 900ms terminal event and spin forever after the updater exited. Retain terminal delivery until the page acknowledges it, bound unavailable-client teardown and failed requests, and preserve a truthful final display.

Fixes #103747. Builds on OutThisLife and Teknium detached handoff work in #83634 and the #75895 quiet-window design. Continues Axl Ibiza Windows update investigation (#60233, #94107, #100763), including source/review contributions carried by merged #93353 and #85170. Existing #102373, #103140, #95719, #97299 and #103632 retain their separate scopes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SelfTestPipeDrain fixture hangs on hosted windows-latest runners (passes on native Windows)

3 participants