fix(update): make the auto-updater reachable from cron and loud when it fails (v1.55.4) - #191
Merged
Merged
Conversation
…it fails (v1.55.4)
`spawn('npm', …)` resolves through PATH, and the runs that need updating most
are the ones with the emptiest PATH. `one sync schedule` writes a crontab line;
cron hands that job `PATH=/usr/bin:/bin`, where an nvm / Volta / fnm / Homebrew
npm does not appear. The spawn died as "npm: command not found" — and because it
ran under `shell: true`, the SHELL started fine and merely exited 127, so the
`'error'` handler never fired and nothing was logged, counted, or surfaced.
Net effect: the installs that run most often were the ones that could never
update themselves. One scheduled install stayed on v1.48.0 for two weeks,
re-running the same doomed update every 5 minutes, while its owner's interactive
installs tracked latest normally. Because it was frozen below v1.55.3 it also
never got the telemetry dedupe fix, and re-sent its rollup queue on every run —
1.55M duplicate PostHog events, 57% of all CLI rollup volume over 14 days.
- Resolve npm next to `process.execPath` (npm sits beside node in every standard
layout) instead of trusting PATH.
- Prepend node's own directory to the child's PATH: an absolute npm is not
enough on its own, since the npm shim's shebang is `#!/usr/bin/env node` and
fails one layer further down otherwise.
- Drop `shell: true` on POSIX, so a missing binary raises a real ENOENT `'error'`
event rather than a silent 127. Windows keeps it (Node won't run a `.cmd`
without a shell) with the path quoted.
- Check the exit code, not just `'error'`: a non-zero exit now settles the lock
and counts as a failure.
- Account for abandoned installs across runs. The detached child usually
outlives us, so its exit code is unobservable — but the lock it left behind is
not. A stale lock plus a version that did not move means the install never
landed. The version we are running, not the lock, is the truth, so a
successful install that also abandoned its lock is not miscounted.
- Stop failing silently: npm's output goes to `~/.one/auto-update.log`, and
after 3 consecutive failures a warning goes to stderr (never stdout, so it
cannot corrupt `--agent` JSON), repeated at most once a day.
Same npm resolution applied to the interactive `one update`, which had the
identical bug in any minimal-PATH environment.
Tests cover npm resolution, PATH construction (including Windows key casing,
where emitting both `Path` and `PATH` leaves the winner undefined), failure
accounting, notice throttling, and an end-to-end pair that reproduces the
original silent 127 under `PATH=/usr/bin:/bin` and proves the fix exits 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KNxvXMWirX42XQKX2y9daC
The PATH-prepend test hardcoded a POSIX `:` separator, which is a single opaque entry on Windows where the delimiter is `;` — so the assertion failed on all three Windows matrix jobs while the implementation (which uses path.delimiter) was correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KNxvXMWirX42XQKX2y9daC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
spawn('npm', …)resolves throughPATH— and the runs that need updating most are the ones with the emptiestPATH.one sync scheduleinstalls a crontab line. cron hands that jobPATH=/usr/bin:/bin, where an nvm / Volta / fnm / Homebrewnpmdoes not exist. So every auto-update attempt from a scheduled run died withnpm: command not found.It died silently. Because the spawn used
shell: true, the shell started fine and merely exited127— which fires no'error'event, the only thing the old code listened for:Nothing was logged, counted, retried differently, or shown. The installs that run most often were the ones that could never update themselves.
What it cost
A user's scheduled install has been pinned to v1.48.0 since Aug 13, re-running the same doomed update every 5 minutes, while their interactive installs on the same account tracked latest normally.
Because it was frozen below v1.55.3, it never received the telemetry
uuiddedupe fix either — so it re-sent its entire rollup queue on every run:That single frozen install is 57% of all CLI Usage Rollup events we ingested in 14 days. Peak day: 244,007 events from 94 real rollups. Client-side fixes cannot reach a client that cannot update — this is the bug that makes every other fix undeliverable.
The fix
process.execPathinstead of trustingPATH. npm sits beside node in every standard layout (nvm, Volta, fnm, Homebrew, the official installer, the Docker node images), soexecPathis a reliable anchor. Bare-npmPATH lookup stays as the fallback.PATH. An absolute npm is not sufficient alone — the npm shim's shebang is#!/usr/bin/env node, so a cronPATHwithout node fails one layer further down, just as quietly.shell: trueon POSIX, so a missing binary raises a real ENOENT'error'event rather than a silent 127. Windows keeps the shell (Node refuses to run a.cmdwithout one) with the path quoted.'error'. A non-zero exit now settles the lock and counts as a failure.~/.one/auto-update.log, and after 3 consecutive failed attempts a warning goes to stderr — never stdout, so it can't corrupt--agentJSON or a piped command — repeated at most once a day.Same npm resolution applied to the interactive
one update, which had the identical bug in any minimal-PATHenvironment.Tests
23 new tests in
src/commands/update.test.ts, including an end-to-end pair that runs under a realPATH=/usr/bin:/bin:Plus npm resolution, PATH construction (including Windows key casing — emitting both
PathandPATHleaves the winner undefined), failure accounting, and notice throttling.Suite: 501 tests, 495 pass, 6 fail. Those 6 are pre-existing on
main(verified by stashing:mainalone is 478 tests / 6 fail) — a macOS-only/private/varvs/varsymlink assertion inconfig.test.ts. Zero regressions, and CI runs ubuntu/windows where they don't occur.Docs
README.md(auto-update section) andsrc/lib/guide-content.ts(scheduled syncs) both updated to describe cron-safe updating and where to look when an install fails.Follow-ups (not in this PR)
npm i -g @withone/cli@latest— this fix can't reach it, by definition.CLI Usage Rollupfrom pre-1.55.3 clients; it would have saved ~1.55M events this fortnight regardless of who upgrades.🤖 Generated with Claude Code
https://claude.ai/code/session_01KNxvXMWirX42XQKX2y9daC