diff --git a/app/electron/cli.test.ts b/app/electron/cli.test.ts index 50913458..4776fb87 100644 --- a/app/electron/cli.test.ts +++ b/app/electron/cli.test.ts @@ -1,10 +1,10 @@ // @vitest-environment node import { describe, it, expect, beforeEach, afterEach } from 'vitest' -import { mkdtempSync, writeFileSync, rmSync, chmodSync } from 'node:fs' +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { spawnCli, CliError } from './cli' +import { spawnCli, CliError, nodeManagerDirs } from './cli' let dir: string const originalBin = process.env.CODEBURN_BIN @@ -61,3 +61,35 @@ describe('spawnCli', () => { } }) }) + +describe('nodeManagerDirs (nvm resolution)', () => { + const savedNvm = process.env.NVM_DIR + afterEach(() => { + if (savedNvm === undefined) delete process.env.NVM_DIR + else process.env.NVM_DIR = savedNvm + }) + + it('scans nvm version dirs newest-first and takes the first that holds codeburn', () => { + // Two versions; the lexicographically-"newest" (v9.0.0 > v22.0.0 as strings) + // has NO codeburn, while the real newer v22.0.0 does. The old `sort().reverse()[0]` + // would pick v9.0.0's bin and miss the CLI entirely. + const nvm = mkdtempSync(join(tmpdir(), 'codeburn-nvm-')) + try { + const versions = join(nvm, 'versions', 'node') + const v9bin = join(versions, 'v9.0.0', 'bin') + const v22bin = join(versions, 'v22.0.0', 'bin') + mkdirSync(v9bin, { recursive: true }) + mkdirSync(v22bin, { recursive: true }) + const codeburn = join(v22bin, 'codeburn') + writeFileSync(codeburn, '#!/bin/sh\n', { mode: 0o755 }) + chmodSync(codeburn, 0o755) + + process.env.NVM_DIR = nvm + const dirs = nodeManagerDirs() + expect(dirs).toContain(v22bin) + expect(dirs).not.toContain(v9bin) + } finally { + rmSync(nvm, { recursive: true, force: true }) + } + }) +}) diff --git a/app/electron/cli.ts b/app/electron/cli.ts index c1beb012..01e3603c 100644 --- a/app/electron/cli.ts +++ b/app/electron/cli.ts @@ -22,7 +22,7 @@ const DEFAULT_TIMEOUT_MS = 45_000 // Homebrew + common Node version managers, mirroring mac/CodeburnCLI.swift so a // GUI-launched app (minimal PATH) still finds a globally-installed `codeburn`. -function nodeManagerDirs(): string[] { +export function nodeManagerDirs(): string[] { const home = homedir() const dirs = [ '/opt/homebrew/bin', @@ -34,10 +34,17 @@ function nodeManagerDirs(): string[] { const nvmDir = process.env.NVM_DIR || join(home, '.nvm') const nvmVersions = join(nvmDir, 'versions', 'node') try { - // Prefer the newest installed node version's bin dir. - const entries = readdirSync(nvmVersions) - const newest = entries.sort().reverse()[0] - if (newest) dirs.push(join(nvmVersions, newest, 'bin')) + // Scan version dirs newest-first and take the first whose bin actually holds + // `codeburn`. A lexicographic max ("v9" > "v22") is not a real "newest", and + // the top dir may not even contain the CLI — so verify, matching CodeburnCLI.swift. + const entries = readdirSync(nvmVersions).sort().reverse() + for (const entry of entries) { + const bin = join(nvmVersions, entry, 'bin') + if (isExecutableFile(join(bin, 'codeburn'))) { + dirs.push(bin) + break + } + } } catch { // no nvm — ignore } diff --git a/app/electron/main.test.ts b/app/electron/main.test.ts index 87965e15..2fce9dc0 100644 --- a/app/electron/main.test.ts +++ b/app/electron/main.test.ts @@ -20,6 +20,56 @@ function fakeSpawn(result: unknown = { current: { cost: 12.34 } }) { return { spawnCli, calls } } +// Every codeburn:* channel with a representative arg tuple → the exact argv it +// must spawn. cliStatus is the one channel that resolves without spawning. +const CHANNELS = [ + 'codeburn:getOverview', + 'codeburn:getPlans', + 'codeburn:getModels', + 'codeburn:getYield', + 'codeburn:getSpendFlow', + 'codeburn:getDevices', + 'codeburn:getShareStatus', + 'codeburn:getIdentity', + 'codeburn:cliStatus', +] as const + +const ARGV_CASES: Array<{ channel: string; args: unknown[]; argv: string[] }> = [ + { channel: 'codeburn:getOverview', args: ['30days', 'claude'], argv: ['status', '--format', 'menubar-json', '--period', '30days', '--provider', 'claude'] }, + { channel: 'codeburn:getOverview', args: ['30days', 'all'], argv: ['status', '--format', 'menubar-json', '--period', '30days'] }, + { channel: 'codeburn:getPlans', args: ['week'], argv: ['status', '--format', 'json', '--period', 'week'] }, + { channel: 'codeburn:getModels', args: ['week', 'claude', true], argv: ['models', '--format', 'json', '--period', 'week', '--provider', 'claude', '--by-task'] }, + { channel: 'codeburn:getModels', args: ['week', 'all', false], argv: ['models', '--format', 'json', '--period', 'week'] }, + { channel: 'codeburn:getYield', args: ['today'], argv: ['yield', '--format', 'json', '--period', 'today'] }, + { channel: 'codeburn:getSpendFlow', args: ['month', 'openai'], argv: ['spend', '--format', 'flow-json', '--period', 'month', '--provider', 'openai'] }, + { channel: 'codeburn:getDevices', args: ['week'], argv: ['devices', '--format', 'json', '--period', 'week'] }, + { channel: 'codeburn:getShareStatus', args: [], argv: ['share', 'status', '--format', 'json'] }, + { channel: 'codeburn:getIdentity', args: [], argv: ['identity', '--format', 'json'] }, +] + +describe('createBridgeHandlers (channel → argv for all channels)', () => { + it('exposes exactly the nine codeburn:* channels', () => { + const handlers = createBridgeHandlers({ spawnCli: vi.fn(), resolveCodeburnPath: () => null }) + expect(Object.keys(handlers).sort()).toEqual([...CHANNELS].sort()) + }) + + it.each(ARGV_CASES)('$channel with $args spawns the expected argv', async ({ channel, args, argv }) => { + const { spawnCli, calls } = fakeSpawn() + const handlers = createBridgeHandlers({ spawnCli, resolveCodeburnPath: () => '/bin/codeburn' }) + const res = await handlers[channel]!(...args) + expect(calls[0]).toEqual(argv) + expect(res).toMatchObject({ ok: true }) + }) + + it('codeburn:cliStatus resolves from resolveCodeburnPath without spawning', async () => { + const spawnCli = vi.fn() + const handlers = createBridgeHandlers({ spawnCli, resolveCodeburnPath: () => '/opt/homebrew/bin/codeburn' }) + const res = await handlers['codeburn:cliStatus']!() + expect(spawnCli).not.toHaveBeenCalled() + expect(res).toEqual({ ok: true, value: { found: true, path: '/opt/homebrew/bin/codeburn' } }) + }) +}) + describe('createBridgeHandlers (IPC wiring)', () => { it('getOverview spawns menubar-json for the period, omitting --provider for "all"', async () => { const { spawnCli, calls } = fakeSpawn() diff --git a/app/electron/preload.ts b/app/electron/preload.ts index dbac60dd..febe0d62 100644 --- a/app/electron/preload.ts +++ b/app/electron/preload.ts @@ -1,8 +1,9 @@ import { contextBridge, ipcRenderer } from 'electron' -// Envelope mirror of main.ts. Handlers resolve with { ok, value } | { ok, error } -// so the structured error `kind` survives the contextBridge boundary. -type Envelope = { ok: true; value: T } | { ok: false; error: { kind: string; message: string } } +// Handlers resolve with { ok, value } | { ok, error } so the structured error +// `kind` survives the contextBridge boundary. `import type` is erased at build, +// so this shares main.ts's declaration without pulling its runtime in. +import type { Envelope } from './main' async function invoke(channel: string, ...args: unknown[]): Promise { const res = (await ipcRenderer.invoke(channel, ...args)) as Envelope diff --git a/app/renderer/hooks/usePolled.test.ts b/app/renderer/hooks/usePolled.test.ts new file mode 100644 index 00000000..080d4ced --- /dev/null +++ b/app/renderer/hooks/usePolled.test.ts @@ -0,0 +1,39 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi } from 'vitest' +import { renderHook, act } from '@testing-library/react' + +import { usePolled } from './usePolled' + +describe('usePolled', () => { + it('discards a stale in-flight fetch that resolves after a newer one (epoch guard)', async () => { + // A fetcher we resolve by hand, one deferred per call, so we can force a + // SLOW deps-A fetch to resolve AFTER a FAST deps-B fetch. + const resolvers: Array<(v: string) => void> = [] + const fetcher = vi.fn(() => new Promise(resolve => { resolvers.push(resolve) })) + + const { result, rerender } = renderHook( + ({ p }: { p: string }) => usePolled(fetcher, [p]), + { initialProps: { p: 'A' } }, + ) + + // #0 mount fetch (deps A) — resolve it to establish a known baseline. + await act(async () => { resolvers[0]!('A0') }) + expect(fetcher).toHaveBeenCalledTimes(1) + expect(result.current.data).toBe('A0') + + // #1 refresh() while deps are still A → an in-flight SLOW fetch whose cancel + // handle the hook discards. Leave it unresolved for now. + act(() => { result.current.refresh() }) + expect(fetcher).toHaveBeenCalledTimes(2) + + // #2 deps change A→B → a FAST fetch that resolves first with fresh data. + rerender({ p: 'B' }) + expect(fetcher).toHaveBeenCalledTimes(3) + await act(async () => { resolvers[2]!('B-fresh') }) + expect(result.current.data).toBe('B-fresh') + + // #1 (the slow deps-A fetch) now resolves LATE. It must NOT clobber B. + await act(async () => { resolvers[1]!('A-stale') }) + expect(result.current.data).toBe('B-fresh') + }) +}) diff --git a/app/renderer/hooks/usePolled.ts b/app/renderer/hooks/usePolled.ts index a30611a4..b6d32c90 100644 --- a/app/renderer/hooks/usePolled.ts +++ b/app/renderer/hooks/usePolled.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { normalizeCliError } from '../lib/ipc' import type { CliError } from '../lib/types' @@ -20,35 +20,41 @@ export function usePolled(fetcher: () => Promise, deps: unknown[], interva const [data, setData] = useState(null) const [error, setError] = useState(null) const [loading, setLoading] = useState(true) + // Generation counter: every load() (mount, deps change, interval, refresh) + // claims the next epoch; a fetch applies its result only while its epoch is + // still current. This is what keeps a slow fetch from an older deps/period + // from clobbering a newer one that already resolved. + const epochRef = useRef(0) const load = useCallback(() => { - let cancelled = false + const epoch = ++epochRef.current setLoading(true) fetcher() .then(result => { - if (cancelled) return + if (epochRef.current !== epoch) return setData(result) setError(null) }) .catch(err => { - if (!cancelled) setError(normalizeCliError(err)) + if (epochRef.current !== epoch) return + setError(normalizeCliError(err)) }) .finally(() => { - if (!cancelled) setLoading(false) + if (epochRef.current !== epoch) return + setLoading(false) }) - return () => { - cancelled = true - } // deps are intentionally the caller-provided dependency list. // eslint-disable-next-line react-hooks/exhaustive-deps }, deps) useEffect(() => { - const cancel = load() + load() const id = setInterval(() => load(), intervalMs) return () => { - cancel() clearInterval(id) + // Retire this generation so an in-flight fetch can't resolve into state + // after unmount or a deps change. + epochRef.current++ } }, [load, intervalMs]) diff --git a/app/renderer/index.html b/app/renderer/index.html index 91c69f9c..a8949407 100644 --- a/app/renderer/index.html +++ b/app/renderer/index.html @@ -5,7 +5,7 @@ CodeBurn