From d58bb2b0bb55e18279c16c4265405c5c73f73846 Mon Sep 17 00:00:00 2001 From: Time Attakc <89218912+time-attack@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:11:34 -0700 Subject: [PATCH] fix(check-update): resolve the latest version from VERSION, not the empty releases API (#486) (#3520) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo publishes zero GitHub releases, so releases/latest is a permanent 404 and fetchLatestRelease() could never succeed — the entire upgrade notification subsystem was a silent no-op, and refreshUpdateCache() cached a fabricated up_to_date marker on every failure. - Resolve the latest version from raw.githubusercontent.com/.../master/VERSION (same trusted host fetchChangelog already uses). Bounded, shape-gated parse; handles legacy 3-segment and -suffix channel forms. - Discriminate network_error from no_releases in the --json error field and human output. - Never write up_to_date on a failed check: preserve the last-known-good marker (mtime bump keeps the TTL throttle) or write nothing. - Rejected the issue's proposed npm fallback: the gbrain npm package is an unrelated GPU library (#505) and would produce false upgrade prompts. Co-authored-by: Garry Tan Co-authored-by: Claude Opus 5 --- src/commands/check-update.ts | 98 +++++++++---- src/commands/self-upgrade.ts | 3 +- test/check-update-refresh.serial.test.ts | 158 +++++++++++++++++---- test/check-update.test.ts | 7 + test/self-upgrade-checkonly.serial.test.ts | 6 +- 5 files changed, 216 insertions(+), 56 deletions(-) diff --git a/src/commands/check-update.ts b/src/commands/check-update.ts index 628c31772..a93a5ff37 100644 --- a/src/commands/check-update.ts +++ b/src/commands/check-update.ts @@ -8,7 +8,7 @@ import { semverGt, semverLte, } from '../core/semver.ts'; -import { writeUpdateCache, type UpdateMarker } from '../core/self-upgrade.ts'; +import { readUpdateCache, writeUpdateCache, type UpdateMarker } from '../core/self-upgrade.ts'; /** Best-effort cache write — a read-only ~/.gbrain must never make the check throw. */ function safeWriteCache(marker: UpdateMarker): void { @@ -45,26 +45,53 @@ function upgradeCommandForMethod(method: string): string { } } +/** Where the latest version is resolved from. gbrain publishes NO GitHub + * releases (the `releases/latest` API is a permanent 404), so the release + * train's source of truth is the `VERSION` file on master — same trusted host + * `fetchChangelog` already uses. An npm fallback was rejected: the `gbrain` + * package on npm is an unrelated GPU library (#505), so it would produce false + * upgrade prompts pointing at a stranger's package. */ +const VERSION_SOURCE_URL = 'https://raw.githubusercontent.com/garrytan/gbrain/master/VERSION'; +const RELEASE_NOTES_URL = 'https://github.com/garrytan/gbrain/blob/master/CHANGELOG.md'; + +/** Extract a version from the raw VERSION file body: first line, optional `v` + * prefix, optional `-suffix` channel tag (`0.31.1.1-fixwave` compares as its + * numeric base — fail-safe: a suffix-only bump never prompts). Body is bounded + * before parsing so a malformed/huge response can't blow up the check. */ +export function parseVersionFileBody(body: string): string | null { + const firstLine = body.slice(0, 256).trim().split('\n')[0].trim(); + const m = firstLine.match(/^v?(\d+\.\d+\.\d+(?:\.\d+)?)(?:[-+][0-9A-Za-z.-]+)?$/); + return m && isValidVersionString(m[1]) ? m[1] : null; +} + +export type LatestReleaseResult = + | { ok: true; tag: string; published_at: string; url: string } + | { ok: false; reason: 'network_error' | 'no_releases' }; + /** - * Fetch the latest GitHub release. Exported (v0.42) so the self-upgrade refresh - * path and tests can reuse it. 5s timeout (was 10s) — this runs on the detached - * refresh, never the hot path, but a tight bound keeps the refresh cheap. + * Resolve the latest published gbrain version (from VERSION on master — see + * VERSION_SOURCE_URL). Exported (v0.42) so the self-upgrade refresh path and + * tests can reuse it. 5s timeout — this runs on the detached refresh, never the + * hot path. Failures are discriminated: `network_error` (offline/timeout) vs + * `no_releases` (endpoint answered but no usable version). */ -export async function fetchLatestRelease(): Promise<{ tag: string; published_at: string; url: string } | null> { +export async function fetchLatestRelease(): Promise { + let res: Response; try { - const res = await fetch('https://api.github.com/repos/garrytan/gbrain/releases/latest', { + res = await fetch(VERSION_SOURCE_URL, { headers: { 'User-Agent': `gbrain/${VERSION}` }, signal: AbortSignal.timeout(5_000), }); - if (!res.ok) return null; - const data = await res.json() as any; - return { - tag: data.tag_name || '', - published_at: data.published_at || '', - url: data.html_url || '', - }; } catch { - return null; + return { ok: false, reason: 'network_error' }; + } + try { + if (!res.ok) return { ok: false, reason: 'no_releases' }; + const tag = parseVersionFileBody(await res.text()); + if (!tag) return { ok: false, reason: 'no_releases' }; + return { ok: true, tag, published_at: '', url: RELEASE_NOTES_URL }; + } catch { + return { ok: false, reason: 'network_error' }; } } @@ -118,17 +145,33 @@ export function extractChangelogBetween(changelog: string, from: string, to: str } /** - * Fetch the latest release and write the self-upgrade cache (the marker line - * read by the CLI startup hook). Fail-open: on any network failure we cache - * `UP_TO_DATE ` so the TTL prevents hammering GitHub on every - * invocation. Returns the resolved marker for callers that want it. This is the - * function the detached single-flight refresh (`gbrain check-update - * --refresh-cache`) invokes. + * A failed check must NEVER write `up_to_date` — that was #486: the fetch + * failed permanently (dead releases API) and every user was told "you're + * current" forever. Instead, re-write the last-known-good marker (bumping its + * mtime so the cache TTL still throttles retries and a network blip can't + * erase a pending upgrade_available notice). No prior marker → write nothing; + * the next invocation retries. + */ +function preserveCacheOnFailedCheck(): void { + try { + const prior = readUpdateCache(); + if (prior) safeWriteCache(prior.marker); + } catch { + /* best-effort */ + } +} + +/** + * Fetch the latest version and write the self-upgrade cache (the marker line + * read by the CLI startup hook). On fetch failure the last-known-good marker is + * preserved (see preserveCacheOnFailedCheck) — never a fabricated `up_to_date`. + * This is the function the detached single-flight refresh (`gbrain + * check-update --refresh-cache`) invokes. */ export async function refreshUpdateCache(): Promise { const release = await fetchLatestRelease(); - if (!release) { - safeWriteCache({ kind: 'up_to_date', current: VERSION }); + if (!release.ok) { + preserveCacheOnFailedCheck(); return; } const latestVersion = release.tag.replace(/^v/, ''); @@ -166,9 +209,8 @@ export async function runCheckUpdate(args: string[]) { const release = await fetchLatestRelease(); - if (!release) { - // Warm the cache fail-open so the startup hook doesn't re-fetch every call. - safeWriteCache({ kind: 'up_to_date', current: VERSION }); + if (!release.ok) { + preserveCacheOnFailedCheck(); if (json) { console.log(JSON.stringify({ current_version: VERSION, @@ -179,10 +221,12 @@ export async function runCheckUpdate(args: string[]) { release_url: '', changelog_diff: '', published_at: '', - error: 'no_releases', + error: release.reason, }, null, 2)); + } else if (release.reason === 'network_error') { + console.log(`GBrain ${VERSION} — could not check for updates (network unavailable).`); } else { - console.log(`GBrain ${VERSION} — could not check for updates (no releases found or network unavailable).`); + console.log(`GBrain ${VERSION} — could not determine the latest published version.`); } return; } diff --git a/src/commands/self-upgrade.ts b/src/commands/self-upgrade.ts index c0d96bde0..a3f2812fb 100644 --- a/src/commands/self-upgrade.ts +++ b/src/commands/self-upgrade.ts @@ -35,7 +35,8 @@ export async function runSelfUpgrade(args: string[]): Promise { const force = args.includes('--force'); const json = args.includes('--json'); - const release = await fetchLatestRelease(); + const result = await fetchLatestRelease(); + const release = result.ok ? result : null; const latest = release ? release.tag.replace(/^v/, '') : null; const behind = !!latest && isValidVersionString(latest) && isNewerVersion(VERSION, latest); diff --git a/test/check-update-refresh.serial.test.ts b/test/check-update-refresh.serial.test.ts index 300a456cf..7669ec797 100644 --- a/test/check-update-refresh.serial.test.ts +++ b/test/check-update-refresh.serial.test.ts @@ -1,8 +1,10 @@ /** * Serial (stubs globalThis.fetch): exercises the self-upgrade cache REFRESH - * orchestration end-to-end — `refreshUpdateCache()` fetches the latest release - * and writes the correct marker to the shared cache file that the CLI startup - * hook reads. Network is stubbed; the cache write + marker logic are real. + * orchestration end-to-end — `refreshUpdateCache()` resolves the latest version + * (from the VERSION file on master, #486 — the repo has zero GitHub releases, + * so the old `releases/latest` API path could never succeed) and writes the + * correct marker to the shared cache file that the CLI startup hook reads. + * Network is stubbed; the cache write + marker logic are real. * * Quarantined as *.serial.test.ts because it reassigns the process-global * `fetch` (cross-file-unsafe under the parallel runner). @@ -13,10 +15,11 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { VERSION } from '../src/version.ts'; import { parseSemver } from '../src/core/semver.ts'; -import { readUpdateCache } from '../src/core/self-upgrade.ts'; -import { refreshUpdateCache } from '../src/commands/check-update.ts'; +import { readUpdateCache, writeUpdateCache } from '../src/core/self-upgrade.ts'; +import { fetchLatestRelease, parseVersionFileBody, refreshUpdateCache, runCheckUpdate } from '../src/commands/check-update.ts'; const realFetch = globalThis.fetch; +const realLog = console.log; let homeDir: string; let priorHome: string | undefined; @@ -27,14 +30,13 @@ function bump(kind: 'minor' | 'patch' | 'micro'): string { return `${v[0]}.${v[1]}.${v[2]}.${v[3] + 1}`; } -function stubReleaseFetch(tag: string | null, ok = true): void { +/** Stub the VERSION-file fetch. body === null → network throw. */ +function stubVersionFetch(body: string | null, status = 200): void { globalThis.fetch = (async (url: any) => { const u = String(url); - if (u.includes('/releases/latest')) { - if (tag === null) throw new Error('network down'); - return new Response(JSON.stringify({ tag_name: tag, published_at: '2026-01-01T00:00:00Z', html_url: 'https://x' }), { - status: ok ? 200 : 500, - }); + if (u.includes('/gbrain/master/VERSION')) { + if (body === null) throw new Error('network down'); + return new Response(body, { status }); } // Changelog fetch (only happens when update available) — return empty. return new Response('', { status: 200 }); @@ -49,49 +51,155 @@ beforeEach(() => { afterEach(() => { globalThis.fetch = realFetch; + console.log = realLog; if (priorHome === undefined) delete process.env.GBRAIN_HOME; else process.env.GBRAIN_HOME = priorHome; rmSync(homeDir, { recursive: true, force: true }); }); +describe('fetchLatestRelease — resolves from the VERSION file, discriminates failures', () => { + test('bare version body → ok with that tag', async () => { + stubVersionFetch('0.99.1.0\n'); + expect(await fetchLatestRelease()).toMatchObject({ ok: true, tag: '0.99.1.0' }); + }); + + test('network throw → network_error (NOT no_releases — offline users are not told "no releases exist")', async () => { + stubVersionFetch(null); + expect(await fetchLatestRelease()).toEqual({ ok: false, reason: 'network_error' }); + }); + + test('HTTP 404 → no_releases', async () => { + stubVersionFetch('Not Found', 404); + expect(await fetchLatestRelease()).toEqual({ ok: false, reason: 'no_releases' }); + }); + + test('garbage body → no_releases', async () => { + stubVersionFetch('rate limited'); + expect(await fetchLatestRelease()).toEqual({ ok: false, reason: 'no_releases' }); + }); +}); + +describe('parseVersionFileBody — shape gate over the raw fetch body', () => { + test('trailing newline, v prefix, 3-segment legacy, suffix channel', () => { + expect(parseVersionFileBody('0.42.67.0\n')).toBe('0.42.67.0'); + expect(parseVersionFileBody('v0.42.67.0')).toBe('0.42.67.0'); + expect(parseVersionFileBody('0.31.3\n')).toBe('0.31.3'); // legacy 3-segment + expect(parseVersionFileBody('0.31.1.1-fixwave\n')).toBe('0.31.1.1'); // suffix compares as base + }); + + test('malformed / huge / injected bodies → null', () => { + expect(parseVersionFileBody('')).toBeNull(); + expect(parseVersionFileBody('not a version')).toBeNull(); + expect(parseVersionFileBody('$(rm -rf /)')).toBeNull(); + expect(parseVersionFileBody('1.2')).toBeNull(); // 2-segment: not a gbrain version + expect(parseVersionFileBody('9'.repeat(10_000_000))).toBeNull(); // bounded, no blowup + }); +}); + describe('refreshUpdateCache — full refresh orchestration (network stubbed)', () => { - test('minor-bump release → writes upgrade_available marker', async () => { + test('minor-bump VERSION on master → writes upgrade_available marker', async () => { const latest = bump('minor'); - stubReleaseFetch(`v${latest}`); + stubVersionFetch(`${latest}\n`); await refreshUpdateCache(); const entry = readUpdateCache(); expect(entry?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); }); - test('patch release → writes upgrade_available marker', async () => { + test('patch bump → writes upgrade_available marker', async () => { const latest = bump('patch'); - stubReleaseFetch(`v${latest}`); + stubVersionFetch(`${latest}\n`); await refreshUpdateCache(); expect(readUpdateCache()?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); }); - test('micro release → writes upgrade_available marker', async () => { + test('micro bump → writes upgrade_available marker', async () => { const latest = bump('micro'); - stubReleaseFetch(`v${latest}`); + stubVersionFetch(`${latest}\n`); await refreshUpdateCache(); expect(readUpdateCache()?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); }); - test('network failure → writes up_to_date marker (fail-open, TTL prevents hammering)', async () => { - stubReleaseFetch(null); + test('minor bump published as legacy 3-segment → still detected', async () => { + const v = parseSemver(VERSION)!; + const latest = `${v[0]}.${v[1] + 1}.0`; // 3-segment, no micro + stubVersionFetch(`${latest}\n`); + await refreshUpdateCache(); + expect(readUpdateCache()?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); + }); + + test('suffix channel release (X.Y.Z.W-fixwave) → compares as numeric base', async () => { + const latest = bump('micro'); + stubVersionFetch(`${latest}-fixwave\n`); + await refreshUpdateCache(); + expect(readUpdateCache()?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); + }); + + test('same version on master → up_to_date marker', async () => { + stubVersionFetch(`${VERSION}\n`); await refreshUpdateCache(); expect(readUpdateCache()?.marker).toEqual({ kind: 'up_to_date', current: VERSION }); }); - test('non-OK HTTP → fail-open up_to_date', async () => { - stubReleaseFetch(`v${bump('minor')}`, false); + // The #486 bug class: a failed check must never fabricate "you're current". + test('network failure with NO prior cache → writes NOTHING (never a fabricated up_to_date)', async () => { + stubVersionFetch(null); await refreshUpdateCache(); - expect(readUpdateCache()?.marker).toEqual({ kind: 'up_to_date', current: VERSION }); + expect(readUpdateCache()).toBeNull(); }); - test('garbage tag → fail-open up_to_date (forged/invalid version never cached as upgrade)', async () => { - stubReleaseFetch('v$(rm -rf /)'); + test('network failure with prior upgrade_available → pending notice PRESERVED, not erased', async () => { + const latest = bump('minor'); + writeUpdateCache({ kind: 'upgrade_available', current: VERSION, latest }); + stubVersionFetch(null); await refreshUpdateCache(); - expect(readUpdateCache()?.marker).toEqual({ kind: 'up_to_date', current: VERSION }); + expect(readUpdateCache()?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); + }); + + test('non-OK HTTP → no fabricated up_to_date', async () => { + stubVersionFetch('nope', 500); + await refreshUpdateCache(); + expect(readUpdateCache()).toBeNull(); + }); + + test('garbage body → no fabricated marker (forged/invalid version never cached as upgrade)', async () => { + stubVersionFetch('$(rm -rf /)'); + await refreshUpdateCache(); + expect(readUpdateCache()).toBeNull(); + }); +}); + +describe('runCheckUpdate --json — failure discrimination (#486)', () => { + function capture(): string[] { + const lines: string[] = []; + console.log = (...a: unknown[]) => { lines.push(a.join(' ')); }; + return lines; + } + + test('offline → error: network_error (not "no releases exist")', async () => { + stubVersionFetch(null); + const lines = capture(); + await runCheckUpdate(['--json']); + const out = JSON.parse(lines.join('\n')); + expect(out.error).toBe('network_error'); + expect(out.update_available).toBe(false); + expect(readUpdateCache()).toBeNull(); // and no fabricated up_to_date cache + }); + + test('endpoint answers but no usable version → error: no_releases', async () => { + stubVersionFetch('garbage'); + const lines = capture(); + await runCheckUpdate(['--json']); + expect(JSON.parse(lines.join('\n')).error).toBe('no_releases'); + }); + + test('newer VERSION on master → update_available true with latest_version set', async () => { + const latest = bump('minor'); + stubVersionFetch(`${latest}\n`); + const lines = capture(); + await runCheckUpdate(['--json']); + const out = JSON.parse(lines.join('\n')); + expect(out.update_available).toBe(true); + expect(out.latest_version).toBe(latest); + expect(readUpdateCache()?.marker).toEqual({ kind: 'upgrade_available', current: VERSION, latest }); }); }); diff --git a/test/check-update.test.ts b/test/check-update.test.ts index 6a2e2b6b9..20b60992c 100644 --- a/test/check-update.test.ts +++ b/test/check-update.test.ts @@ -35,6 +35,13 @@ describe('isNewerVersion', () => { expect(isNewerVersion('0.42.66.0', '0.42.66.1')).toBe(true); }); + test('orders legacy 3-segment against 4-segment: 0.42.67.0 > 0.42.66.1 > 0.42.66', () => { + expect(isNewerVersion('0.42.66.1', '0.42.67.0')).toBe(true); + expect(isNewerVersion('0.42.66', '0.42.66.1')).toBe(true); + expect(isNewerVersion('0.42.66', '0.42.66.0')).toBe(false); // 3-segment == its .0 micro + expect(isNewerVersion('0.42.67.0', '0.42.66.1')).toBe(false); + }); + test('rejects equal, older, and malformed versions', () => { expect(isNewerVersion('0.42.66.0', '0.42.66.0')).toBe(false); expect(isNewerVersion('0.42.66.1', '0.42.66.0')).toBe(false); diff --git a/test/self-upgrade-checkonly.serial.test.ts b/test/self-upgrade-checkonly.serial.test.ts index 7f4dfd4f5..f443a66a4 100644 --- a/test/self-upgrade-checkonly.serial.test.ts +++ b/test/self-upgrade-checkonly.serial.test.ts @@ -31,9 +31,9 @@ function microBump(): string { function stub(tag: string | null, changelog: string): void { globalThis.fetch = (async (url: any) => { const u = String(url); - if (u.includes('/releases/latest')) { + if (u.includes('/gbrain/master/VERSION')) { if (tag === null) throw new Error('network down'); - return new Response(JSON.stringify({ tag_name: tag, published_at: '2026-01-01', html_url: 'https://x/rel' }), { status: 200 }); + return new Response(tag + '\n', { status: 200 }); } if (u.includes('CHANGELOG.md')) return new Response(changelog, { status: 200 }); return new Response('', { status: 200 }); @@ -65,7 +65,7 @@ describe('self-upgrade --check-only surfaces what you get', () => { const out = JSON.parse(captured.join('\n')); expect(out.update_available).toBe(true); expect(out.latest_version).toBe(latest); - expect(out.release_url).toBe('https://x/rel'); + expect(out.release_url).toBe('https://github.com/garrytan/gbrain/blob/master/CHANGELOG.md'); expect(out.changelog_diff).toContain('Shiny new thing'); });