fix(upgrade): install a missing target instead of running its updater - #112
Merged
ralyodio merged 1 commit intoJul 31, 2026
Merged
Conversation
`moshcode upgrade doppler` on a box without doppler printed "(installing — not present)" and then ran `doppler update` — the very binary that is missing — so it failed with ENOENT instead of installing. planUpgrade computed an `installed` flag and then ignored it, calling upgradeSpec/toolUpgradeSpec unconditionally. Those helpers prefer an entry's native updater, which is only valid for something already on disk (engines.mjs: "The command that upgrades an already-installed engine in place"). Affected every entry that has a native updater: tools doppler and tailscale, engines opencode, privacycode and aider. Entries without one fall back to `install` on their own, which is why this stayed hidden. Fix: use the target's installer when it is not present, keep the native updater when it is.
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
moshcode upgrade <target>cannot install a target that has a native updater. It announces that it is installing, then runs the binary that is missing:Why
planUpgradecomputes aninstalledflag and then ignores it one line later:toolUpgradeSpec/upgradeSpecreturnentry.upgrade || entry.install, so any entry with a native updater plans that updater even when the target is absent. That contract is documented as installed-only, insrc/engines.mjs:Affected: tools
doppler(doppler update) andtailscale(tailscale update); enginesopencode,privacycode(<bin> upgrade) andaider(aider --upgrade). Every other entry has noupgradefield, so the|| entry.installfallback masks it.This contradicts what the code says it does in three places:
src/upgrade.mjsdoc comment:["claude"|"ugig", …] → named targets (install if not present yet)src/upgrade.mjs:// Explicit names/engine aliases upgrade even when not currently installed.(installing — not present)It also contradicts the existing test, which is named
"explicit tool upgrades use official installers even when not installed"— but only exercisesugigandcoinpay, the two tools with noupgradefield, so it passes while the claim is false.Repro
On unmodified
main(1c05460), withdopplernot on PATH, the real CLI output above. At the planning layer:After the fix
moshcode upgrade dopplerinstalls it for real (Installed Doppler CLI v3.76.1), andtailscale, which is installed here, still planstailscale update.The fix
Choose the spec from the flag that was already being computed: installer when absent, native updater when present. 11 insertions / 4 deletions, 5 of the additions comment.
Tests
New
test/upgrade-install-missing.test.mjs, 11 tests. 5 are the bug, 6 are controls that pass both before and after.The bug tests cover a missing tool, a missing engine, a missing engine reached through an alias (
pc→ privacycode), plus a sweep over every entry that has a native updater — derived fromENGINES/TOOLSrather than hardcoded, so a newly added updater is covered automatically instead of silently skipped.The controls deliberately assert the other direction so the fix cannot buy installability by throwing away the native updaters: an installed tool and an installed engine still plan
doppler update/opencode upgrade; entries with no native updater are unchanged either way;tools/engines/allstill only include installed targets; unknown targets are still collected rather than planned.Fail-before via
git checkout -- src/upgrade.mjs: 5 fail / 6 pass unpatched, 11/11 patched. Full suite 391 → 402, 0 failures.Deliberately not included
test/upgrade.test.mjs's existing test keeps its misleading name and itsugig/coinpaytargets. Renaming or widening it is yours to call; the new file covers the gap without touching it.src/cli.mjs--dry-runreturns{ ok: true, dryRun: true }with nocode, contradicting its own JSDoc (Returns { ok, code } — always). Separate defect, happy to send it if useful.