diff --git a/src/engines.mjs b/src/engines.mjs index 59ea517..0167eb0 100644 --- a/src/engines.mjs +++ b/src/engines.mjs @@ -33,7 +33,12 @@ export const ENGINES = { agentArgs: ["--auto"], agentsView: ["agent", "list"], install: { cmd: "sh", args: ["-c", "curl -fsSL https://getprivacycode.com/install | sh"] }, - upgrade: { cmd: "privacycode", args: ["upgrade"] }, + // Deliberately no native updater. `privacycode upgrade` is opencode's, and + // it works out how to update itself by recognising where it was installed — + // it knows opencode's own locations, not this fork's ~/.privacycode/bin. It + // reports `Using method: unknown` and aborts with "Unknown installation + // method", every time, so it can never upgrade an install we made. Falling + // through to the installer above is what actually moves the version. }, claude: { desc: "Claude Code — Anthropic's agentic CLI", diff --git a/src/upgrade.mjs b/src/upgrade.mjs index 2858475..faf6848 100644 --- a/src/upgrade.mjs +++ b/src/upgrade.mjs @@ -80,6 +80,10 @@ export function planUpgrade(targets = []) { label: key, kind: "engine", spec: installed ? upgradeSpec(ENGINES[key]) : ENGINES[key].install, + // Where to turn when a native updater refuses. Only set when the updater + // is something other than the installer, so a fallback can never repeat + // the command that just failed. + fallback: installed && upgradeSpec(ENGINES[key]) !== ENGINES[key].install ? ENGINES[key].install : null, installed, }); }; @@ -93,6 +97,7 @@ export function planUpgrade(targets = []) { label: key, kind: "tool", spec: installed ? toolUpgradeSpec(TOOLS[key]) : TOOLS[key].install, + fallback: installed && toolUpgradeSpec(TOOLS[key]) !== TOOLS[key].install ? TOOLS[key].install : null, installed, }); }; @@ -133,16 +138,22 @@ export async function runUpgrade(targets = [], io = {}) { return []; } + const exec = io.runCmd || runCmd; + const results = []; - const run = async (name, spec, note) => { + const attempt = async (name, spec, note) => { log(`\n⬆ upgrading ${name}${note ? ` ${note}` : ""} — ${spec.cmd} ${spec.args.join(" ")}`); rule(); - const r = await runCmd(spec.cmd, spec.args); + const r = await exec(spec.cmd, spec.args); rule(); const ok = ranOk(r); log(ok ? `✓ ${name} up to date` : `✗ ${name} upgrade failed (${exitReason(r)})`); - results.push({ name, ok, code: r.code, signal: r.signal ?? null }); - return ok; + return { name, ok, code: r.code, signal: r.signal ?? null }; + }; + const run = async (name, spec, note) => { + const result = await attempt(name, spec, note); + results.push(result); + return result.ok; }; if (self) { @@ -158,7 +169,20 @@ export async function runUpgrade(targets = [], io = {}) { } } } - for (const it of items) await run(it.label, it.spec, it.installed ? "" : "(installing — not present)"); + for (const it of items) { + const result = await attempt(it.label, it.spec, it.installed ? "" : "(installing — not present)"); + // A native updater that can't tell how the binary got there fails the same + // way on every run — an opencode fork living under its own directory, a + // binary someone moved, a machine where the installer left no marker. The + // installer is idempotent and fetches the latest, so reach for it rather + // than leaving the target stranded on an old version. + if (!result.ok && it.fallback) { + log(`· ${it.label}'s own updater could not do it — falling back to its installer`); + results.push(await attempt(it.label, it.fallback, "(installer)")); + continue; + } + results.push(result); + } const failed = results.filter((r) => !r.ok); log(`\n${failed.length ? "✗" : "✓"} upgraded ${results.length - failed.length}/${results.length}${failed.length ? ` — failed: ${failed.map((r) => r.name).join(", ")}` : "."} 🤘`); diff --git a/test/upgrade-install-missing.test.mjs b/test/upgrade-install-missing.test.mjs index 69b6cf0..f83f7c1 100644 --- a/test/upgrade-install-missing.test.mjs +++ b/test/upgrade-install-missing.test.mjs @@ -78,7 +78,11 @@ test("a missing engine reached through an alias is planned with its installer", test("no missing target is ever planned to run the binary it is missing", () => { // The sweep that matters: for EVERY entry with a native updater, the planned // command for a not-installed target must not be the absent binary itself. - assert.ok(withNativeUpdater.length >= 5, "expected several native updaters"); + // Cover both code paths rather than counting to a number that moves whenever + // an entry gains or loses an updater — privacycode dropped its own once it + // turned out it could never update an install of ours. + assert.ok(withNativeUpdater.some(([, , kind]) => kind === "engine"), "no engine has a native updater to sweep"); + assert.ok(withNativeUpdater.some(([, , kind]) => kind === "tool"), "no tool has a native updater to sweep"); for (const [key, entry, kind] of withNativeUpdater) { const item = specOf(key); assert.equal(item.installed, false, `${kind} ${key} should not be installed`); diff --git a/test/upgrade.test.mjs b/test/upgrade.test.mjs index 1458726..483f3f1 100644 --- a/test/upgrade.test.mjs +++ b/test/upgrade.test.mjs @@ -4,7 +4,24 @@ import { tmpdir } from "node:os"; import path from "node:path"; import test from "node:test"; -import { planUpgrade, selfSpec } from "../src/upgrade.mjs"; +import { ENGINES, upgradeSpec } from "../src/engines.mjs"; +import { planUpgrade, runUpgrade, selfSpec } from "../src/upgrade.mjs"; + +// Same trick as withFakeTools, for the cases that turn on a target being +// installed: only an installed target is asked to update itself, so the +// fallback path can't be reached without one on PATH. +async function withFakeBins(names, fn) { + const dir = mkdtempSync(path.join(tmpdir(), "moshcode-upgrade-bins-")); + for (const name of names) { + const file = path.join(dir, name); + writeFileSync(file, "#!/bin/sh\nexit 0\n"); + chmodSync(file, 0o755); + } + const before = process.env.PATH; + process.env.PATH = dir; + try { return await fn(); } + finally { process.env.PATH = before; } +} function withFakeTools(fn) { const dir = mkdtempSync(path.join(tmpdir(), "moshcode-upgrade-")); @@ -63,6 +80,71 @@ test("default upgrade includes self and every installed tool", () => { }); }); +test("privacycode upgrades by re-running its installer, not its own updater", () => { + // `privacycode upgrade` is opencode's updater, and it decides how to update + // by recognising where the binary was installed. It knows opencode's + // locations, not this fork's ~/.privacycode/bin, so against an install of + // ours it reports `Using method: unknown` and aborts every time. Re-running + // the installer is what actually moves the version. + assert.equal(ENGINES.privacycode.upgrade, undefined, "privacycode must not carry a native updater"); + assert.deepEqual(upgradeSpec(ENGINES.privacycode), ENGINES.privacycode.install); + + // The same call on plain opencode keeps its updater: that one does know + // where opencode puts itself. + assert.deepEqual(upgradeSpec(ENGINES.opencode), { cmd: "opencode", args: ["upgrade"] }); +}); + +test("an explicit privacycode upgrade plans the installer", () => { + const plan = planUpgrade(["privacycode"]); + + assert.equal(plan.self, false); + assert.deepEqual(plan.items.map(({ key, kind, spec }) => ({ key, kind, spec })), [ + { + key: "privacycode", + kind: "engine", + spec: { cmd: "sh", args: ["-c", "curl -fsSL https://getprivacycode.com/install | sh"] }, + }, + ]); +}); + +test("a refused native updater falls back to the installer", async () => { + // opencode's updater picks its method from where the binary was installed, + // and reports `Using method: unknown` and gives up when it doesn't + // recognise the location — the same on every run, so the target would stay + // stranded on an old version. The installer is idempotent; use it. + const calls = []; + const runCmd = async (cmd, args) => { + calls.push(`${cmd} ${args.join(" ")}`); + return cmd === "opencode" ? { ok: true, code: 1, signal: null } : { ok: true, code: 0, signal: null }; + }; + const logs = []; + const results = await withFakeBins(["opencode"], () => runUpgrade(["opencode"], { + runCmd, log: (s) => logs.push(s), rule: () => {}, + })); + + assert.deepEqual(calls, [ + "opencode upgrade", + "bash -c curl -fsSL https://opencode.ai/install | bash", + ], "the installer runs only after its own updater refuses"); + assert.deepEqual(results.map((r) => r.ok), [true], "the fallback result is what counts, not the refusal"); + assert.ok(logs.some((l) => l.includes("falling back to its installer")), "the fallback must be visible, not silent"); +}); + +test("a target with no separate updater is not retried", async () => { + // claude upgrades with `npm install -g`, which is already the installer. + // Retrying it would just run the identical command a second time. + const calls = []; + const runCmd = async (cmd, args) => { + calls.push(`${cmd} ${args.join(" ")}`); + return { ok: true, code: 1, signal: null }; + }; + const results = await withFakeBins(["claude"], () => + runUpgrade(["claude"], { runCmd, log: () => {}, rule: () => {} })); + + assert.deepEqual(calls, ["npm install -g @anthropic-ai/claude-code"]); + assert.deepEqual(results.map((r) => r.ok), [false]); +}); + test("unknown upgrade targets remain visible to the caller", () => { const plan = planUpgrade(["not-a-tool"]); assert.deepEqual(plan.unknown, ["not-a-tool"]);