Skip to content

fix(ops): judge the canary install by the SHA on disk, not npm's exit code - #10699

Merged
diegosouzapw merged 1 commit into
release/v3.8.50from
fix/10429-canary-install-exit-code
Aug 19, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.50from
fix/10429-canary-install-exit-code

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Refs #10429.

Problem

npm install -g <tarball> on the .17 gateway writes the whole package and then fails renaming
the old tree into its staging directory:

npm error code ENOTEMPTY
npm error syscall rename
npm error path /usr/lib/node_modules/omniroute
npm error dest /usr/lib/node_modules/.omniroute-h797OOZa

Exit status 217 — but dist/BUILD_SHA, the package version and every dependency are the new
ones. The canary read the non-zero exit as "install failed", aborted before the restart, and
execFileSync threw away npm's stderr, so the log said only Command failed: ssh … npm install.

This stopped the deploy half-done twice on 2026-08-18, each time leaving the host with the
new files on disk under the old running process — the exact split state a deploy script exists
to prevent. Both times it took a manual SSH round-trip to discover the install had actually
succeeded.

Why not just tolerate ENOTEMPTY

Because the exit code is untrustworthy in both directions. The 2026-08-14 outage (#10427) went
the other way: npm install -g exited 0 while shipping a package built from the wrong
branch, and everything downstream believed it.

So neither "non-zero = failed" nor "zero = fine" holds. classifyInstallOutcome() decides on the
BUILD_SHA read back from the installed package:

exit SHA on disk verdict
217 matches installed-with-cleanup-failure → continue, warn
0 matches installed
217 stale failed
0 wrong artifact failed
any unreadable failed (fails closed, same rule as the provenance gate)

Staging directory

npm reuses the same staging name, so the orphan blocks the next install with the identical
error — that is why it recurred. orphanStagingDirFromStderr() extracts the exact path and the
run warns with it.

It is deliberately not removed automatically: that is an rm -rf under /usr/lib, and a
deploy script should not decide that on its own.

Validation

tests/unit/canary-install-outcome-10429.test.ts — 7 cases, written first and confirmed failing
(does not provide an export named 'classifyInstallOutcome'), including the real 2026-08-18
stderr verbatim and the inverse trap (zero exit + wrong artifact must still fail).

Sibling suites green: deploy-canary-10429, build-sha-provenance-10427 (25 tests total).
typecheck:core clean. CLI exercised with --dry-run, which also caught a wiring bug before it
shipped: plan does not expose buildSha, so the expected SHA had to come from readBuildSha()
— left as-is it would have failed every install.

… code

`npm install -g <tarball>` on the .17 gateway writes the whole package and then fails
renaming the old tree into its staging directory (ENOTEMPTY, exit 217). The canary read
that non-zero exit as "install failed", aborted before the restart, and discarded npm's
stderr through execFileSync throwing — so on 2026-08-18 the deploy stopped half-done
twice, each time leaving new files on disk under an old running process, with no clue in
the log.

The exit code is not trustworthy in either direction: the 2026-08-14 outage installed a
package built from the wrong branch and exited 0. classifyInstallOutcome() therefore
decides on the BUILD_SHA read back from the installed package, and fails closed when it
is absent or does not match — a zero exit with the wrong artifact is still a failure.

npm reuses the same staging directory name, so the orphan blocks the next install with
the same error; orphanStagingDirFromStderr() surfaces the exact path. It is not removed
automatically — that is an rm -rf under /usr/lib, not something a deploy script should
decide on its own.

Refs #10429
@jonlwheat2-gif

Copy link
Copy Markdown
Contributor

Review: logic is sound — one wiring gap found and fixed

I applied the PR to the current release/v3.8.50 tip (d7368e243) and validated it. The decision matrix in classifyInstallOutcome() covers all four exit/SHA quadrants plus fail-closed, and the 7 new unit tests pin the real 2026-08-18 stderr verbatim. One genuine gap in the shell wiring, fixed below.

The gap — scripts/ops/deploy-canary.mjs, install section

The classifier's fail-closed path was unreachable from the shell: run(verify) was called as a bare argument to classifyInstallOutcome, so when the remote SHA read itself threw (unreachable host, or dist/BUILD_SHA missing on the host), execFileSync died before classification ran — and the captured npm stderr, the exact output this fix exists to surface, was dropped again.

Before (PR head, lines ~172–179):

const installResult = runCapturing(install);
const outcome = classifyInstallOutcome({
  exitCode: installResult.exitCode,
  stderr: installResult.stderr,
  installedSha: run(verify),
  expectedSha: localBuildSha,
});

After (lines 172–184):

const installResult = runCapturing(install);
let installedSha = null;
try {
  installedSha = run(verify);
} catch {
  // verify throws when the host is unreachable or the SHA file is unreadable; let
  // classifyInstallOutcome fail closed on the null instead of dying in execFileSync
  // and dropping npm's captured stderr again (the failure this script exists for).
}
const outcome = classifyInstallOutcome({
  exitCode: installResult.exitCode,
  stderr: installResult.stderr,
  installedSha,
  expectedSha: localBuildSha,
});

Plus, line 199 — the post-restart attestation read was const installedSha = ..., which now collides with the outer let; it becomes a plain reassignment:

-  const installedSha = run(verify);
+  installedSha = run(verify);

This makes the "unreadable SHA → failed: no BUILD_SHA could be read…" branch actually reachable, with npm's stderr printed right before the failure — consistent with the classifier's design (installedSha is already typed string | null | undefined).

Validation (Windows, Node 24, worktree off the current release tip)

  • 7 new tests pass; sibling suites deploy-canary-10429 + build-sha-provenance-10427 + npm-publish-artifact-provenance pass (25 total in the batch)
  • node --check scripts/ops/deploy-canary.mjs clean; ESLint clean
  • typecheck:core: no errors in touched files

On the branch

fix/10429-canary-install-exit-code (on jonlwheat2-gif/OmniRoute, base d7368e243):

  • 0c648b74f — the PR change (3 files, 225+/4−)
  • dde829780 — the wiring fix above (1 file, 10+/2−)

@diegosouzapw

Copy link
Copy Markdown
Owner Author

The 7 red checks are inherited base-red, not this PR — proven, not assumed.

Both this PR and # fail on the exact same 7 jobs despite having no file in common. Reproduced on the pure base tip d7368e243d in a disposable worktree with no commit from any branch of mine: tests 26 / pass 10 / fail 16.

The failures are in areas this diff does not touch: Antigravity cloudcode envelope + public models, GLM provider import (4 tests), free-model catalog qwen-web ids, i18n key drift (#6695), /api/dead, open-sse/executors/index.ts TS2345, geminiWeb.ts file size, and a Qwen Code JSON config parse.

Tracked under #9985.

@diegosouzapw
diegosouzapw merged commit 86fc1aa into release/v3.8.50 Aug 19, 2026
11 checks passed
@diegosouzapw
diegosouzapw deleted the fix/10429-canary-install-exit-code branch August 19, 2026 00:51
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 20, 2026
… code (diegosouzapw#10699)

`npm install -g <tarball>` on the .17 gateway writes the whole package and then fails
renaming the old tree into its staging directory (ENOTEMPTY, exit 217). The canary read
that non-zero exit as "install failed", aborted before the restart, and discarded npm's
stderr through execFileSync throwing — so on 2026-08-18 the deploy stopped half-done
twice, each time leaving new files on disk under an old running process, with no clue in
the log.

The exit code is not trustworthy in either direction: the 2026-08-14 outage installed a
package built from the wrong branch and exited 0. classifyInstallOutcome() therefore
decides on the BUILD_SHA read back from the installed package, and fails closed when it
is absent or does not match — a zero exit with the wrong artifact is still a failure.

npm reuses the same staging directory name, so the orphan blocks the next install with
the same error; orphanStagingDirFromStderr() surfaces the exact path. It is not removed
automatically — that is an rm -rf under /usr/lib, not something a deploy script should
decide on its own.

Refs diegosouzapw#10429

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
giauphan pushed a commit to giauphan/OmniRoute that referenced this pull request Aug 20, 2026
… code (diegosouzapw#10699)

`npm install -g <tarball>` on the .17 gateway writes the whole package and then fails
renaming the old tree into its staging directory (ENOTEMPTY, exit 217). The canary read
that non-zero exit as "install failed", aborted before the restart, and discarded npm's
stderr through execFileSync throwing — so on 2026-08-18 the deploy stopped half-done
twice, each time leaving new files on disk under an old running process, with no clue in
the log.

The exit code is not trustworthy in either direction: the 2026-08-14 outage installed a
package built from the wrong branch and exited 0. classifyInstallOutcome() therefore
decides on the BUILD_SHA read back from the installed package, and fails closed when it
is absent or does not match — a zero exit with the wrong artifact is still a failure.

npm reuses the same staging directory name, so the orphan blocks the next install with
the same error; orphanStagingDirFromStderr() surfaces the exact path. It is not removed
automatically — that is an rm -rf under /usr/lib, not something a deploy script should
decide on its own.

Refs diegosouzapw#10429

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
… code (diegosouzapw#10699)

`npm install -g <tarball>` on the .17 gateway writes the whole package and then fails
renaming the old tree into its staging directory (ENOTEMPTY, exit 217). The canary read
that non-zero exit as "install failed", aborted before the restart, and discarded npm's
stderr through execFileSync throwing — so on 2026-08-18 the deploy stopped half-done
twice, each time leaving new files on disk under an old running process, with no clue in
the log.

The exit code is not trustworthy in either direction: the 2026-08-14 outage installed a
package built from the wrong branch and exited 0. classifyInstallOutcome() therefore
decides on the BUILD_SHA read back from the installed package, and fails closed when it
is absent or does not match — a zero exit with the wrong artifact is still a failure.

npm reuses the same staging directory name, so the orphan blocks the next install with
the same error; orphanStagingDirFromStderr() surfaces the exact path. It is not removed
automatically — that is an rm -rf under /usr/lib, not something a deploy script should
decide on its own.

Refs diegosouzapw#10429

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
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.

3 participants