diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index debe9a9..b3fa14f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -125,8 +125,13 @@ jobs: if: ${{ vars.WINGET_PACKAGE_ID != '' }} runs-on: windows-latest steps: + - uses: actions/checkout@v4 + - id: version + shell: bash + run: echo "version=$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT - uses: vedantmgoyal9/winget-releaser@v2 with: identifier: ${{ vars.WINGET_PACKAGE_ID }} installers-regex: '\.exe$' token: ${{ secrets.WINGET_TOKEN }} + release-tag: v${{ steps.version.outputs.version }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b4f09..2b4894c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ the [releases page](https://github.com/JeremySNR/cutawan/releases). This project uses [semantic versioning](https://semver.org/), loosely: while still pre-1.0, minor bumps carry new features and patch bumps carry fixes. +## [Unreleased] + +### Fixed + +- If Cutawan couldn't restart itself after updating a source checkout, trying again now finishes the update instead of reporting "already up to date". +- On Windows, a Python or other command launcher (such as one from conda) is no longer mistaken for a broken Codex install. +- AI connections that don't support strict JSON output (some local and OpenAI-compatible models) are now told exactly what shape to reply in, and a malformed reply is retried instead of failing. +- A size-limited export that still ends up over the limit now says so plainly. + ## [0.12.0] - 2026-09-23 ### Added diff --git a/src/main/pipeline/openai.ts b/src/main/pipeline/openai.ts index eff8a6b..281d6cd 100644 --- a/src/main/pipeline/openai.ts +++ b/src/main/pipeline/openai.ts @@ -289,6 +289,22 @@ function looksLikeUnsupportedFormat(err: unknown): boolean { ) } +function looksLikeInvalidJson(err: unknown): boolean { + return err instanceof OpenAIError && err.status === undefined && err.message === 'Analysis returned invalid JSON' +} + +function schemaInstruction(schemaName: string, schema: Record): string { + return `Return only a JSON object matching the ${schemaName} schema:\n${JSON.stringify(schema, null, 2)}\nNo markdown, no commentary.` +} + +function messagesWithSchemaInstruction( + messages: ChatMessage[], + schemaName: string, + schema: Record +): ChatMessage[] { + return [...messages, { role: 'user', content: schemaInstruction(schemaName, schema) }] +} + /** * Compatible endpoints (Ollama, LM Studio, some Groq/OpenRouter models) often * reject OpenAI's strict json_schema. Try that first, then json_object, then @@ -318,7 +334,7 @@ async function completeChatContent( label: 'json_object', body: { model, - messages, + messages: messagesWithSchemaInstruction(messages, schemaName, schema), response_format: { type: 'json_object' } } }, @@ -326,20 +342,15 @@ async function completeChatContent( label: 'plain', body: { model, - messages: [ - ...messages, - { - role: 'user', - content: - 'Return only a JSON object matching the requested schema. No markdown, no commentary.' - } - ] + messages: messagesWithSchemaInstruction(messages, schemaName, schema) } } ] let lastError: unknown - for (const format of formats) { + for (let i = 0; i < formats.length; i++) { + const format = formats[i] + const isLast = i === formats.length - 1 try { const res = await fetch(`${chatApiBase()}/chat/completions`, { method: 'POST', @@ -356,11 +367,18 @@ async function completeChatContent( } const content = body.choices?.[0]?.message?.content if (!content) throw new OpenAIError('Analysis returned an empty response') - return extractJsonText(content) + const text = extractJsonText(content) + try { + JSON.parse(text) + } catch { + throw new OpenAIError('Analysis returned invalid JSON') + } + return text } catch (err) { lastError = err if (signal?.aborted) throw err - if (!looksLikeUnsupportedFormat(err)) throw err + const canTryNext = looksLikeUnsupportedFormat(err) || looksLikeInvalidJson(err) + if (!canTryNext || isLast) throw err } } throw lastError diff --git a/src/main/subscription.ts b/src/main/subscription.ts index 8d68c46..47a79a8 100644 --- a/src/main/subscription.ts +++ b/src/main/subscription.ts @@ -3,7 +3,7 @@ import { spawn } from 'node:child_process' import { createHash, randomUUID } from 'node:crypto' import { accessSync, constants, existsSync } from 'node:fs' import { mkdir, mkdtemp, open, readFile, rename, rm, writeFile } from 'node:fs/promises' -import { delimiter, dirname, join, resolve } from 'node:path' +import { basename, delimiter, dirname, join, resolve } from 'node:path' import { homedir } from 'node:os' import { analysisRequests as requests } from './pipeline/mediaJobs' import { DEFAULT_SUBSCRIPTION, type SubscriptionSettings } from '@shared/subscription' @@ -52,29 +52,47 @@ export function resolveCodexExecutable(executable: string, searchPath = process. }) ?? executable } -function commandFor(executable: string): { command: string; prefix: string[]; node: boolean } { - // npm installs a .cmd shim on Windows; run its JS entry directly, never via a shell. +function commandFor(executable: string): { command: string; prefix: string[]; node: boolean; shell: boolean } { + // npm installs a Codex .cmd shim on Windows; run its JS entry directly, never via a shell. if (process.platform === 'win32') { const candidates = executable.includes('/') || executable.includes('\\') ? [executable] : (process.env.PATH ?? '').split(delimiter).flatMap(dir => [join(dir, executable + '.exe'), join(dir, executable + '.cmd')]) const found = candidates.find(path => existsSync(path)) if (found?.endsWith('.cmd')) { - const entry = join(dirname(found), 'node_modules', '@openai', 'codex', 'bin', 'codex.js') - if (!existsSync(entry)) throw new Error('Select the Codex executable in Settings; this command shim is not a Codex installation.') - return { command: process.execPath, prefix: [entry], node: true } + if (basename(found).toLowerCase() === 'codex.cmd') { + const entry = join(dirname(found), 'node_modules', '@openai', 'codex', 'bin', 'codex.js') + if (!existsSync(entry)) throw new Error('Select the Codex executable in Settings; this command shim is not a Codex installation.') + return { command: process.execPath, prefix: [entry], node: true, shell: false } + } + // Other .cmd wrappers (e.g. a conda python.cmd) are not Codex; Node needs a shell to launch them. + return { command: found, prefix: [], node: false, shell: true } } - if (found) return { command: found, prefix: [], node: false } + if (found) return { command: found, prefix: [], node: false, shell: false } } - return { command: resolveCodexExecutable(executable), prefix: [], node: false } + return { command: resolveCodexExecutable(executable), prefix: [], node: false, shell: false } +} + +/** + * Quote arguments for cmd.exe (which Node runs as `cmd /d /s /c ""`). + * Inside double quotes cmd treats spaces and & | < > ^ literally; embedded + * quotes are doubled. `%VAR%` can still expand, so arguments must not rely on + * a literal percent sign. + */ +export function cmdLine(parts: string[]): string { + return parts.map((part) => `"${part.replace(/"/g, '""')}"`).join(' ') } async function run(executable: string, args: string[], input: string, signal: AbortSignal): Promise { signal.throwIfAborted() - const { command, prefix, node } = commandFor(executable) + const { command, prefix, node, shell } = commandFor(executable) return new Promise((accept, reject) => { const env = subscriptionEnvironment(process.env) if (node) env.ELECTRON_RUN_AS_NODE = '1' - const child = spawn(command, [...prefix, ...args], { windowsHide: true, shell: false, env, detached: process.platform !== 'win32' }) + // A .cmd launcher needs cmd.exe, which splits on spaces and interprets + // & | < > ^: pass it one fully quoted command line instead of argv. + const child = shell + ? spawn(cmdLine([command, ...prefix, ...args]), [], { windowsHide: true, shell: true, env }) + : spawn(command, [...prefix, ...args], { windowsHide: true, shell: false, env, detached: process.platform !== 'win32' }) const stop = (): void => { // npm/Python launchers can have a native child. Killing just the launcher // would leave inference running (and consuming allowance) after Cancel. diff --git a/src/main/updates.ts b/src/main/updates.ts index 63b5e75..e8b0cd7 100644 --- a/src/main/updates.ts +++ b/src/main/updates.ts @@ -368,6 +368,10 @@ function runStep( const npmCmd = 'npm' let sourceUpdateRunning = false +// Set once a source update is pulled, installed and rebuilt, so only the +// process handover is left. It survives a failed relaunch: pulling again would +// be a no-op and abort, leaving no way to finish the installed update. +let relaunchPending = false /** * True when `git pull --ff-only` fetched nothing. Release discovery @@ -449,6 +453,20 @@ function relaunchAfterSourceUpdate(root: string): Promise { }) } +/** Last step of a source update: hand the rebuilt checkout over to a new process. */ +async function finishSourceUpdate( + root: string, + onProgress: (p: ImportProgress) => void +): Promise { + relaunchPending = true + onProgress({ progress: 1, message: 'Restarting…' }) + // Let the "Restarting…" frame land before swapping. Awaiting the handover + // keeps a failed relaunch from resolving as success and stranding the + // renderer on that frame with no way to retry. + await new Promise((r) => setTimeout(r, 800)) + await relaunchAfterSourceUpdate(root) +} + /** * One-click update for source checkouts: fast-forward the repo, reinstall * dependencies, rebuild, then relaunch. Only manifest/build-cache churn is @@ -468,6 +486,13 @@ export async function updateFromSource(onProgress: (p: ImportProgress) => void): if (sourceUpdateRunning) throw new Error('An update is already running.') sourceUpdateRunning = true try { + // Retry after a failed handover: the update is already built, so go + // straight back to restarting instead of re-running a now no-op pull. + if (relaunchPending) { + await finishSourceUpdate(root, onProgress) + return + } + onProgress({ progress: -1, message: 'Checking the local checkout…' }) const dirty = (await runStep('git', ['status', '--porcelain'], root)) .split('\n') @@ -505,12 +530,7 @@ export async function updateFromSource(onProgress: (p: ImportProgress) => void): onProgress({ progress: -1, message: 'Rebuilding the app…' }) await runStep(npmCmd, ['run', 'build'], root) - onProgress({ progress: 1, message: 'Restarting…' }) - // Let the "Restarting…" frame land before swapping. Awaiting the handover - // keeps a failed relaunch from resolving as success and stranding the - // renderer on that frame with no way to retry. - await new Promise((r) => setTimeout(r, 800)) - await relaunchAfterSourceUpdate(root) + await finishSourceUpdate(root, onProgress) } finally { sourceUpdateRunning = false } diff --git a/src/renderer/src/components/EditorScreen.tsx b/src/renderer/src/components/EditorScreen.tsx index b404398..0d565bb 100644 --- a/src/renderer/src/components/EditorScreen.tsx +++ b/src/renderer/src/components/EditorScreen.tsx @@ -131,6 +131,11 @@ export default function EditorScreen(): React.JSX.Element { } const entry = exports[clip.id] + const capExceeded = + entry?.status === 'done' && + entry.sizeTargetBytes != null && + entry.bytes != null && + entry.bytes > entry.sizeTargetBytes const cropDisabled = clip.edit.aspect === 'original' const hasShotLayout = validLayoutShots(clip.visualLayout, clip.edit.start, clip.edit.end) const automaticLayout = automaticLayoutShots(clip).length > 0 @@ -582,18 +587,25 @@ export default function EditorScreen(): React.JSX.Element { onCancel={() => void cancelExport(clip.id)} /> - {entry?.status === 'done' && entry.downscaled && ( + {entry?.status === 'done' && entry.downscaled && !capExceeded && (

Scaled the frame down so the file would fit {entry.sizeTargetBytes ? ` under ${formatBytes(entry.sizeTargetBytes)}` : ''}.

)} - {entry?.status === 'done' && entry.overBudget && ( + {entry?.status === 'done' && entry.overBudget && !capExceeded && (

This edit is long for the size cap — the picture may look soft. A shorter trim would hold up better.

)} + {capExceeded && ( +

+ This export is over your size limit + {entry.sizeTargetBytes ? ` (${formatBytes(entry.sizeTargetBytes)})` : ''}. Shorten the + clip or raise the cap. +

+ )} {entry?.status === 'error' && entry.error && (

{entry.error}

)} diff --git a/src/shared/types.ts b/src/shared/types.ts index aee02d8..54aff2f 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -357,8 +357,8 @@ export interface ExportResult { downscaled?: boolean /** * True when even the minimum scale could not reach a healthy bits-per-pixel - * at this duration — the file should still fit, but the picture will look - * worse than a shorter clip would. + * at this duration — the picture will look soft. Does not guarantee the + * file stays under the cap; compare `bytes` to `sizeTargetBytes`. */ overBudget?: boolean } diff --git a/src/shared/uploadBudget.ts b/src/shared/uploadBudget.ts index a45f7de..c652357 100644 --- a/src/shared/uploadBudget.ts +++ b/src/shared/uploadBudget.ts @@ -71,7 +71,9 @@ export interface UploadEncodePlan { estimatedBytes: number /** * True when even `MIN_SCALE` cannot reach `TARGET_BITS_PER_PIXEL`. The - * render still goes ahead, but the caller may want to warn. + * render still goes ahead, but the caller may want to warn about soft + * picture quality. When the video bitrate floor dominates, the finished + * file can still exceed `capBytes`. */ overBudget: boolean } diff --git a/tests/openai.test.ts b/tests/openai.test.ts index ae51cfa..ffb64e8 100644 --- a/tests/openai.test.ts +++ b/tests/openai.test.ts @@ -135,6 +135,25 @@ describe('chatJSON', () => { expect(second.response_format?.type).toBe('json_object') }) + it('tells json_object fallbacks the schema and retries after invalid JSON', async () => { + delete process.env.OPENAI_BASE_URL + configureOpenAiEndpoints({ chatBase: 'http://127.0.0.1:11434/v1' }) + const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + const body = JSON.parse(String(init?.body)) as { response_format?: { type?: string } } + // The strict attempt comes back as prose; the next format returns JSON. + const content = body.response_format?.type === 'json_schema' ? 'Sure! Here you go: ok' : '{"ok":true}' + return new Response(JSON.stringify({ choices: [{ message: { content } }] }), { status: 200 }) + }) + vi.stubGlobal('fetch', fetchMock) + const result = await chatJSON<{ ok: boolean }>('sk-test', 'local-model', [{ role: 'user', content: 'hi' }], 'test', schema) + expect(result).toEqual({ ok: true }) + const second = JSON.parse(String(fetchMock.mock.calls[1]?.[1]?.body)) as { + response_format?: { type?: string }; messages: Array<{ content: string }> + } + expect(second.response_format?.type).toBe('json_object') + expect(second.messages.at(-1)?.content).toContain('"required"') + }) + it('does not fall back on a real 401', async () => { delete process.env.OPENAI_BASE_URL const fetchMock = vi.fn(async () => new Response(JSON.stringify({ error: { message: 'bad key' } }), { status: 401 })) diff --git a/tests/sourceUpdateRetry.test.ts b/tests/sourceUpdateRetry.test.ts new file mode 100644 index 0000000..944504f --- /dev/null +++ b/tests/sourceUpdateRetry.test.ts @@ -0,0 +1,64 @@ +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { EventEmitter } from 'node:events' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +const mocks = vi.hoisted(() => ({ + root: '', + app: { isPackaged: false, getVersion: () => '1.0.0', getAppPath: (): string => '', relaunch: vi.fn(), exit: vi.fn() }, + spawn: vi.fn(), + relaunchFails: true +})) +vi.mock('electron', () => ({ app: mocks.app, shell: {} })) +vi.mock('electron-updater', () => ({ autoUpdater: { on: vi.fn(), removeListener: vi.fn() } })) +vi.mock('node:child_process', () => ({ spawn: mocks.spawn })) + +/** A fake child: git/npm steps succeed; the relaunch spawn fails the first time. */ +function fakeProcess(command: string, args: string[]): EventEmitter { + const child = Object.assign(new EventEmitter(), { + stdout: new EventEmitter(), stderr: new EventEmitter(), unref: vi.fn() + }) + setTimeout(() => { + if (command === process.execPath) { + if (mocks.relaunchFails) child.emit('error', new Error('spawn EPERM')) + else child.emit('spawn') + return + } + if (command === 'git' && args[0] === 'pull') child.stdout.emit('data', Buffer.from('Updating abc123..def456\nFast-forward\n')) + child.emit('close', 0) + }, 0) + return child +} + +beforeEach(() => { + vi.resetModules() + vi.useFakeTimers({ toFake: ['setTimeout'], shouldAdvanceTime: true, advanceTimeDelta: 200 }) + mocks.root = mkdtempSync(join(tmpdir(), 'cutawan-source-update-')) + mkdirSync(join(mocks.root, '.git')) + writeFileSync(join(mocks.root, 'package.json'), JSON.stringify({ name: 'cutawan' })) + mocks.app.getAppPath = () => mocks.root + mocks.spawn.mockImplementation(fakeProcess) + mocks.relaunchFails = true + // A dev-server session relaunches through spawn, so the failure is observable. + vi.stubEnv('ELECTRON_RENDERER_URL', 'http://localhost:5173') +}) + +afterEach(() => { + vi.useRealTimers() + vi.unstubAllEnvs() + rmSync(mocks.root, { recursive: true, force: true }) +}) + +it('lets a retry finish an update whose restart failed, without pulling again', async () => { + const updates = await import('../src/main/updates') + await expect(updates.updateFromSource(() => {})).rejects.toThrow(/could not restart itself/) + const pulls = (): number => mocks.spawn.mock.calls.filter(([cmd, args]) => cmd === 'git' && args[0] === 'pull').length + expect(pulls()).toBe(1) + + // Retrying used to pull again, find nothing new and report "already up to date". + mocks.relaunchFails = false + await updates.updateFromSource(() => {}) + expect(pulls()).toBe(1) + expect(mocks.app.exit).toHaveBeenCalledWith(0) +}) diff --git a/tests/subscription.test.ts b/tests/subscription.test.ts index 6cccd02..8525300 100644 --- a/tests/subscription.test.ts +++ b/tests/subscription.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { EventEmitter } from 'node:events' import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' @@ -171,3 +171,11 @@ it('cancels a duplicate waiter without cancelling the original request', async ( expect(await third).toEqual({ title: 'A complete story' }) expect(mock.spawn.mock.calls.filter(call => call[1][0] === 'exec')).toHaveLength(1) }) + +describe('cmd.exe launcher quoting', () => { + it('keeps paths with spaces and shell characters in one argument each', async () => { + const { cmdLine } = await import('../src/main/subscription') + expect(cmdLine(['C:\\Users\\Jane Doe\\miniconda3\\condabin\\python.cmd', 'C:\\Program Files\\Cutawan\\transcribe.py', 'a&b', 'say "hi"'])) + .toBe('"C:\\Users\\Jane Doe\\miniconda3\\condabin\\python.cmd" "C:\\Program Files\\Cutawan\\transcribe.py" "a&b" "say ""hi"""') + }) +})