fix(ci): windows-x86 SSH probe — Bun.spawnSync timeout returns null exitCode by mobileskyfi · Pull Request #93 · tikoci/quickchr

mobileskyfi · 2026-07-10T19:44:23Z

Summary

Fixes #92 — a second, distinct windows-x86 SSH batch-login CI failure (not #87, which
is already fixed and confirmed not reproducing).

test/integration/provisioning.test.ts's independent SSH login probe (separate from
verifyBatchLogin() in src/lib/provision.ts, which fix(provision): Windows batch-login verification (ssh -F NUL) #88 already instrumented) only
checked stdout, so a failure collapsed to Received: "" with no diagnosable trace.
Adding stderr/exit-code logging on failure (this PR's first commit) reproduced the
failure 3/3 on a narrow windows-x86 + provisioning.test.ts + stable dispatch:
[ssh probe] exit null: <no output> — Bun.spawnSync's timeout option returning an
instant, empty result (~400ms after account creation, nowhere near the 20s timeout or
any plausible connection/network delay).
verifyBatchLogin() uses async Bun.spawn() + a manual setTimeout-based kill instead
of Bun.spawnSync's timeout option, and has never failed this way. Rewriting the
test's probe to the same shape (second commit) was tested directly and passed 3/3
across a second round of narrow dispatches — the fix holds.

CI already runs the latest stable Bun (1.3.14, unpinned oven-sh/setup-bun, shipped
2026-07-08) — this isn't a stale-runtime gap. Full root-cause writeup, evidence, and an
open question about a second spawnSync(..., timeout) call site
(test/lab/ssh-keys/run-matrix.ts, also exercised on Windows via lab.yml) are in #92.

Test plan

bun run check — clean
integration.yml -f platforms=windows-x86 -f test-filter=provisioning.test.ts -f routeros-targets=stable -f run-examples=false, repeated:
- Before fix (spawnSync): 3/3 fail, all [ssh probe] exit null: <no output> — runs 29116468133, 29116483756, 29116488086
- After fix (spawn): 3/3 pass — runs 29117958656, 29117963187, 29117967629 (that batch's one job failure was the unrelated CI: provisioning.test.ts fixed 360s timeout can be shorter than the download-retry worst case #91, confirmed by its own clean DOWNLOAD_FAILED trace and the SSH test passing later in the same run)

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests
- Improved SSH login test reliability by using asynchronous process handling.
- Added output monitoring and a 20-second timeout to prevent stalled test runs.
- Enhanced diagnostic logging when the expected login confirmation is not detected.

…ilure The second, independent host-OpenSSH batch-login check in "SSH key installed for quickchr managed user" only ever decoded stdout, so a failure collapsed to Received: "" with no trace of why — the same diagnostic blindness #88 fixed in verifyBatchLogin() itself, just not applied to the test's own duplicate probe. Two windows-x86/stable CI runs post-#88 (29097379124, 29106310402) hit this exact assertion with an empty stdout ~500ms after account creation, while the internal verifyBatchLogin() succeeded silently both times — a different, not-yet-diagnosed failure from #87's NUL bug. This is instrumentation only, to capture what ssh actually says on the next occurrence.

… verifyBatchLogin) Bun.spawnSync's timeout option reproduced 3/3 on windows-x86 as an instant, undiagnosable exitCode: null with empty stdout+stderr (~400ms after account creation — nowhere near any real timeout), confirmed by the prior commit's stderr/exit-code instrumentation. verifyBatchLogin() (src/lib/provision.ts) uses async Bun.spawn() with a manual kill timer instead and has never failed that way in the same CI runs. Switching the test's own duplicate probe to the same shape to test that as the root cause directly.

coderabbitai · 2026-07-10T19:44:48Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d415a82b-6f68-489b-beb1-14fa7c753c86

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The provisioning test replaces synchronous SSH verification with asynchronous Bun.spawn handling, including piped output, a 20-second kill timeout, concurrent stream draining, diagnostics, and cleanup before asserting the login marker.

Changes

SSH verification

Layer / File(s)	Summary
Async SSH process control `test/integration/provisioning.test.ts`	Replaces `Bun.spawnSync` with `Bun.spawn`, drains `stdout` and `stderr`, kills the process after 20 seconds, clears the timer, and asserts `managed-key-login-ok` is present.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

tikoci/quickchr#82: Introduced the SSH batch-login verification logic that this change refactors.
tikoci/quickchr#83: Also modified the managed SSH key batch-login verification.
tikoci/quickchr#88: Updated the same verification step’s SSH configuration handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the core fix: replacing the windows-x86 SSH probe's Bun.spawnSync timeout behavior in the provisioning test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch diag/ssh-second-probe-diagnostic

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

This PR addresses a Windows/x86 CI failure in the integration SSH batch-login probe by replacing Bun.spawnSync(..., { timeout }) with an async Bun.spawn() + manual kill timer pattern (matching verifyBatchLogin()), improving reliability and diagnosability on Windows.

Changes:

Reworked the provisioning integration test’s independent SSH probe from spawnSync to async spawn with a manual timeout kill.
Added in-test commentary documenting the Windows-specific failure mode observed with spawnSync timeouts.
Improved failure logging by including exit code and combined output on probe failure.

- Initialize the output accumulator up front instead of a bare `let x: string` only assigned on the happy path. - Match success against combined stdout+stderr, like verifyBatchLogin() does, instead of stdout alone — avoids missing a success message ssh happened to emit on stderr, and keeps the two probes consistent.

mobileskyfi added 2 commits July 10, 2026 11:59

Copilot AI review requested due to automatic review settings July 10, 2026 19:44

Copilot started reviewing on behalf of mobileskyfi July 10, 2026 19:44 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread test/integration/provisioning.test.ts Outdated

mobileskyfi merged commit 9f76c21 into main Jul 10, 2026
9 checks passed

mobileskyfi deleted the diag/ssh-second-probe-diagnostic branch July 10, 2026 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ci): windows-x86 SSH probe — Bun.spawnSync timeout returns null exitCode#93

fix(ci): windows-x86 SSH probe — Bun.spawnSync timeout returns null exitCode#93
mobileskyfi merged 3 commits into
mainfrom
diag/ssh-second-probe-diagnostic

mobileskyfi commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mobileskyfi commented Jul 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mobileskyfi commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading