Skip to content

fix(prod): follow both Windows log files and survive truncation - #559

Open
RonenMars wants to merge 1 commit into
mainfrom
fix/windows-prod-logs-followups
Open

fix(prod): follow both Windows log files and survive truncation#559
RonenMars wants to merge 1 commit into
mainfrom
fix/windows-prod-logs-followups

Conversation

@RonenMars

@RonenMars RonenMars commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Follow-ups to #520 and #523.

The bug prod logs had on Windows

Get-Content -Wait cannot back a two-file follow. Given several paths it blocks inside the first file's read loop and never opens the second — not even to print its seed lines. On the default prod logs (stdout + stderr) that hid stderr entirely, which is the one stream a crashed boot leaves anything in.

It also cannot recover from truncation. -F was chosen on POSIX precisely because prod logs --clear and the boot log-cap both empty these files; a -Wait follower keeps its stored offset, sits past the new EOF, and silently never prints again — alive-looking and permanently useless.

windowsFollowScript() replaces it with an explicit poll loop that resets a file's offset when it shrinks.

$ErrorActionPreference = 'Stop' covers the seed reads, so an unreadable path exits non-zero instead of powershell.exe's default "non-terminating error, exit 0" — which runProdLogs would otherwise report as success with no output. The poll body deliberately swallows its own errors instead: a file vanishing mid-follow is what -F is for and must not kill the follower.

A deploy that aborted on the installs it exists to heal

Repair-LaunchCmd's Get-Command node was unguarded. Under ErrorActionPreference = Stop a missing node aborts the entire deploy with a raw CommandNotFoundException rather than a readable message — and the >> condition above it makes that path fire on every install predating the redirection, which is exactly the population this function exists to repair.

Three assertions that could not fail

Each of these was green while the thing it claimed to lock was broken:

  • deploy-windows-script asserted with a whole-file toContain. Invoke-Setup carries its own identical $logsDir = ... line, so the test passed even with the launcher's copy deleted — which would build the redirect targets from Join-Path $null. Now scoped to Get-LaunchCmdLines.
  • expect(getLogPaths()).toEqual(logPaths()) compared a function to itself: task-scheduler's getLogPaths is return logPaths(). Replaced with assertions on the shape the launcher actually depends on.
  • prod-commands used a bare vi.mock("node:child_process") factory, which blanks every other export. conflict-check.ts imports execFileSync from there and calls it inside try/catch, so an undefined execFileSync threw TypeError, was swallowed, and made detectConflictingAgents() return [] unconditionally — every prod doctor case below was asserting against a detector that could not detect anything. Now spreads importActual.

Docs

The ROADMAP paragraph is a rebase resolution rather than my original text, and the difference matters.

This branch was cut before #527. It claimed CI does not cover the Windows redirection because test:smoke ran an explicit eight-file allowlist containing neither affected test — true when written, false now: #527 (2026-08-11) replaced that allowlist with the whole suite on both platforms, and main already carried a correction saying so. That correction is kept intact here rather than overwritten.

What this PR adds to it: lifecycle/prod-commands.test.ts alongside deploy-windows-script.test.ts, and a note that the PowerShell follow loop in cli/prod.ts is exercised by nothing at all — Get-Content and file-share semantics only exist at runtime on Windows.

Two stale %TEMP%\threadbase.err references now point at ~/.threadbase/logs/stderr.log, which is what prod logs --errors-only reads.

Verification

tsc --noEmit clean; biome check clean across 378 files, re-run after the rebase onto main.

The behaviour this changes is Windows-only and, as above, unreachable by CI — the follow loop needs a real Windows box. A manual verification script covering both-files-followed, truncation recovery, --errors-only, and the Repair-LaunchCmd guard is ready to run and has not been run yet. The three test repairs are verifiable anywhere.

Get-Content -Wait cannot back `prod logs` on Windows: given several paths it blocks inside the first file's read loop and never opens the second, so stderr — the one stream a crashed boot leaves anything in — was never shown.
It also keeps its offset across truncation, so `prod logs --clear` and the boot log-cap left the follower parked past EOF, alive-looking and permanently silent; the replacement poll loop resets the offset when a file shrinks, which is what -F gives POSIX.
Sets $ErrorActionPreference = 'Stop' for the seed reads so an unreadable path exits non-zero instead of powershell.exe reporting success with no output, while the poll body still swallows its own errors because a file vanishing mid-follow is precisely what -F is for.
Guards Repair-LaunchCmd's `Get-Command node` lookup, which under ErrorActionPreference = Stop aborted the entire deploy with a raw CommandNotFoundException on exactly the pre-redirection installs it exists to heal.
Repairs three assertions that could not fail: a whole-file toContain that Invoke-Setup's identical line satisfied, a getLogPaths()/logPaths() comparison of a function against itself, and a bare node:child_process mock that blanked execFileSync so detectConflictingAgents() returned [] unconditionally.
Corrects ROADMAP's claim that CI covers the Windows redirection — test:smoke runs an explicit eight-file list including neither of these tests, and no job executes launch.cmd — and repoints two stale %TEMP%\threadbase.err references at the real log paths.
@RonenMars
RonenMars force-pushed the fix/windows-prod-logs-followups branch from bd4a204 to 2eb19df Compare August 13, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant