fix(#4460): correct two stale changesets left by an admin-merge race (#4572)

* fix: address orthogonal-review findings on the new work in this PR

Isolated code-review + security-review of everything added to this PR
since its original review (hono override, check-env.cjs rewrite/revert,
new lib file, its test, installer enumeration). Security review: clean,
no findings. Code review found:

- BLOCKER: .changeset/silly-hens-relax.md described a hono override
  this PR no longer actually makes -- PR #4560 landed the identical fix
  on next first, and this branch's own hono commit became a genuine
  no-op the moment it was rebased onto that updated next (git diff
  origin/next -- package.json package-lock.json is empty). Deleted the
  orphaned changeset; next already carries #4560's equivalent one
  (.changeset/zesty-seals-click.md).
- HIGH: .changeset/tame-hens-jump.md's body still described the
  execNpm-routing approach that was tried and reverted -- stale text
  from before that revert, would have shipped a release note for code
  that isn't actually in the diff. Rewritten to describe what actually
  shipped (self-contained spawnSync, 15s timeout, accurate ENOENT vs.
  timeout vs. non-zero-exit diagnosis).
- LOW: no comment explaining why the spawnSync call has no try/catch
  (safe -- its documented contract routes failures through the returned
  result, never a throw -- but worth stating given this file's whole
  purpose is graceful degradation). Added one.
- nit: exitCode 0 + empty stdout fell through to "npm binary not found
  on PATH", misdescribing a real npm binary that simply printed
  nothing. Gave it its own message; updated the corresponding test.

Manually re-verified describeNpmVersionCheckFailure's branches and the
real check:env success path before re-running gsd-test, since this
repo blocks local node --test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix: rest of the orthogonal-review fixes (previous commit only caught the deletion)

Tooling mistake in the previous commit: a git add with the already-staged
deleted changeset mixed into the same pathspec list errored out and
silently skipped staging the other four files, so only the changeset
deletion actually committed. This commit carries the rest of that same
change: tame-hens-jump.md's rewritten body, check-env.cjs's no-try/catch
comment, npm-version-check-diagnosis.cjs's exitCode-0-empty-stdout fix,
and the corresponding test update.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(#4460): fix changeset pr field to point at this PR, not the original

.changeset/tame-hens-jump.md's pr field still said 4552 (the PR its
original text was authored under), but this PR (#4572) is what's
actually landing the corrected body -- changeset-lint's own
DEFECT.CHANGESET-PR-FIELD-DRIFT check caught it: "pr: 4552, expected
pr: 4572".

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-09 16:26:25 -04:00
committed by GitHub
parent 385ed619f1
commit 615b74ff45
5 changed files with 17 additions and 9 deletions

View File

@@ -196,6 +196,9 @@ function main() {
// than inventing a new number. Confirmed via real Windows CI: a 10s
// window was insufficient twice under ~51-file concurrent test load.
const NPM_VERSION_TIMEOUT_MS = 15_000;
// No try/catch needed: spawnSync's documented contract routes ENOENT and a
// timeout-triggered kill through the RETURNED result's `.error` field, not
// a thrown exception -- there is nothing here for a catch to intercept.
const npmVersionSpawn = spawnSync(npmCmd, ['--version'], { encoding: 'utf8', timeout: NPM_VERSION_TIMEOUT_MS, shell: process.platform === 'win32' });
const npmVersionResult = {
exitCode: npmVersionSpawn.status ?? 1,

View File

@@ -46,6 +46,13 @@ function describeNpmVersionCheckFailure(result) {
if (result.exitCode !== 0) {
return `npm --version exited ${result.exitCode} with no usable output`;
}
if (result.exitCode === 0) {
// npm ran and exited cleanly but printed nothing -- distinct from every
// case above (which all involve a failed/absent spawn), so it gets its
// own message rather than falling through to "not found on PATH", which
// would misdescribe a real npm binary that simply produced no output.
return 'npm --version exited 0 but produced no output';
}
return 'npm binary not found on PATH';
}