Skip to content

fix: restore phone-home diagnostics in warm-pool download step#62

Merged
kurok merged 1 commit into
mainfrom
fix/warmpool-download-diagnostics
Jul 5, 2026
Merged

fix: restore phone-home diagnostics in warm-pool download step#62
kurok merged 1 commit into
mainfrom
fix/warmpool-download-diagnostics

Conversation

@kurok

@kurok kurok commented Jul 5, 2026

Copy link
Copy Markdown

Summary

buildReusableUserData() (the reuse: stop / warm-pool cold-boot script) dropped the bootstrap ERR trap before the runner-download step and never re-established phone-home diagnostics inside that step's inner sudo -u runner shell — unlike buildUserData()'s plain-path equivalent, which does both. Functions and traps don't survive the sudo -u boundary, so any curl/sha256sum/tar failure inside the warm-pool RUNNER_DOWNLOAD heredoc was completely silent: no failed:downloading bootstrap tag, and mode: start just ran out the registration-timeout clock with nothing to show for it.

Fixes #61

Fix

  • Re-include PHONE_HOME_HELPERS inside the RUNNER_DOWNLOAD heredoc.
  • Set GH_RUNNER_STEP=downloading and re-arm trap 'gh_runner_phone_home "failed:${GH_RUNNER_STEP}"' ERR inside that heredoc, before the download/verify sequence — mirroring buildUserData()'s RUNNER_BOOTSTRAP heredoc line for line.
  • Also tightened ordering in the plain path's configuring phase (GH_RUNNER_STEP=configuring + trap now armed immediately after PHONE_HOME_HELPERS, ahead of the IMDS/config.sh prep lines) for consistency.

Tests added

New regression tests in tests/warmpool.test.js, scoped to the RUNNER_DOWNLOAD heredoc body specifically (via a helper that extracts just that heredoc's contents) so they can't be satisfied by matches elsewhere in the script — e.g. the outer root shell's preparing trap or the register script's configuring trap:

  • re-includes PHONE_HOME_HELPERS inside the RUNNER_DOWNLOAD heredoc (regression, #61)
  • arms an ERR trap for the downloading phase inside the RUNNER_DOWNLOAD heredoc (regression, #61) — also asserts ordering: GH_RUNNER_STEP=downloading and the trap must be set before the curl -fsSLo download call.

Verification

  • npm test — 17 suites / 226 tests, all passing.
  • npm run lint — clean, no findings.
  • npm run package — rebuilt dist/index.js; git status --porcelain -- dist/ confirmed the working-tree dist/ already matched a fresh build (no drift beyond the intended 8-line diff mirroring the src/aws.js change). Only dist/index.js changed; the code-split dist/*.index.js chunk files are untouched by this fix.

Review findings

A code review pass was run against this change; no findings were reported (confirmedCount: 0, no fixes needed/applied).

buildReusableUserData()'s RUNNER_DOWNLOAD heredoc (the reuse: stop
cold-boot path) dropped the ERR trap before entering the sudo -u runner
shell and never re-included PHONE_HOME_HELPERS or re-armed the trap
inside it, unlike buildUserData()'s plain-path RUNNER_BOOTSTRAP
equivalent. Functions and traps don't survive the sudo -u boundary, so
any curl/sha256sum/tar failure during the warm-pool download step was
silent: no failed:downloading bootstrap tag, just a stall until the
external GitHub registration timeout.

Re-include PHONE_HOME_HELPERS inside the RUNNER_DOWNLOAD heredoc and
re-arm the ERR trap for the downloading phase before the download/
verify sequence, mirroring the plain path line for line. Adds
regression tests scoped to the RUNNER_DOWNLOAD heredoc body so they
can't be satisfied by the outer shell's or register script's traps.

Fixes #61

Signed-off-by: kurok <22548029+kurok@users.noreply.github.com>
@kurok kurok merged commit 032802a into main Jul 5, 2026
5 checks passed
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.

reuse: stop download step has no error trap/diagnostics — silent failures indistinguishable from a generic timeout

1 participant