From 578c40c856deb66a46a5f63b96d44b7e4d30e6f0 Mon Sep 17 00:00:00 2001 From: Sun-sunshine06 Date: Sun, 3 May 2026 21:43:26 +0800 Subject: [PATCH 1/4] fix: address issue 254 follow-ups Signed-off-by: Sun-sunshine06 --- .gitignore | 1 + apps/desktop/scripts/after-pack-prune.cjs | 11 ++- .../desktop/scripts/after-pack-prune.test.mjs | 4 +- apps/desktop/src/main/connection-ipc.test.ts | 38 ++++++- apps/desktop/src/main/connection-ipc.ts | 55 ++++++++--- apps/desktop/src/main/ipc/generate.ts | 2 +- apps/desktop/src/main/snapshots-db.test.ts | 3 +- apps/desktop/src/main/snapshots-ipc.ts | 11 +++ .../desktop/src/main/workspace-reader.test.ts | 2 +- .../src/main/workspace-watcher.test.ts | 26 +++-- .../renderer/src/components/PreviewPane.tsx | 2 +- .../src/components/settings/StorageTab.tsx | 3 + .../src/preview/workspace-source.test.ts | 17 ++++ .../renderer/src/preview/workspace-source.ts | 9 +- packages/core/src/agent.test.ts | 98 ++++++++++++++----- packages/core/src/agent.ts | 32 +++--- packages/core/src/design-skills/index.test.ts | 18 ++-- packages/core/src/design-skills/index.ts | 4 +- .../core/src/prompts/sections/output-rules.md | 2 + packages/core/src/resource-state.ts | 9 +- packages/core/src/tools/done.ts | 2 +- packages/core/src/tools/skill.test.ts | 15 ++- packages/exporters/src/pdf.test.ts | 83 +++++++++------- packages/i18n/src/locales/en.json | 2 + packages/i18n/src/locales/pt-BR.json | 2 + packages/i18n/src/locales/zh-CN.json | 2 + packages/providers/src/index.ts | 1 + packages/providers/src/retry.test.ts | 13 ++- packages/providers/src/retry.ts | 7 ++ packages/runtime/src/index.test.ts | 9 ++ packages/runtime/src/index.ts | 41 +++++++- website/package.json | 1 + website/scripts/clean-vitepress-vue-link.mjs | 18 ++++ 33 files changed, 424 insertions(+), 119 deletions(-) create mode 100644 website/scripts/clean-vitepress-vue-link.mjs diff --git a/.gitignore b/.gitignore index 01fb5f93..b51d4c0b 100644 --- a/.gitignore +++ b/.gitignore @@ -57,6 +57,7 @@ pnpm-debug.log* # VitePress website/.vitepress/dist/ website/.vitepress/cache/ +website/.vitepress/.temp/ # Playwright MCP screenshots .playwright-mcp/ diff --git a/apps/desktop/scripts/after-pack-prune.cjs b/apps/desktop/scripts/after-pack-prune.cjs index d8cbb503..983e26ea 100644 --- a/apps/desktop/scripts/after-pack-prune.cjs +++ b/apps/desktop/scripts/after-pack-prune.cjs @@ -14,7 +14,16 @@ function archName(arch) { } function rm(target) { - fs.rmSync(target, { recursive: true, force: true, maxRetries: 3 }); + if (!fs.existsSync(target)) return; + const stat = fs.lstatSync(target); + if (!stat.isDirectory()) { + fs.unlinkSync(target); + return; + } + for (const entry of fs.readdirSync(target)) { + rm(path.join(target, entry)); + } + fs.rmdirSync(target); } function existingDirs(paths) { diff --git a/apps/desktop/scripts/after-pack-prune.test.mjs b/apps/desktop/scripts/after-pack-prune.test.mjs index bb48f058..9eeab4fc 100644 --- a/apps/desktop/scripts/after-pack-prune.test.mjs +++ b/apps/desktop/scripts/after-pack-prune.test.mjs @@ -37,7 +37,7 @@ describe('after-pack-prune', () => { await afterPackPrune({ appOutDir: root, electronPlatformName: 'darwin', - arch: 3, + arch: 'arm64', packager: { appInfo: { productFilename: 'Open CoDesign' } }, }); @@ -64,7 +64,7 @@ describe('after-pack-prune', () => { afterPackPrune({ appOutDir: root, electronPlatformName: 'darwin', - arch: 3, + arch: 'arm64', packager: { appInfo: { productFilename: 'Open CoDesign' } }, }), ).resolves.toBeUndefined(); diff --git a/apps/desktop/src/main/connection-ipc.test.ts b/apps/desktop/src/main/connection-ipc.test.ts index 36f81d0e..6f82ca2f 100644 --- a/apps/desktop/src/main/connection-ipc.test.ts +++ b/apps/desktop/src/main/connection-ipc.test.ts @@ -1032,21 +1032,53 @@ describe('runProviderTest degrade-probe (issue #179)', () => { } }); - it('anthropic: /models 404 does NOT degrade (standard endpoint must stay authoritative)', async () => { + it('anthropic: /models 404 + /v1/messages 404 preserves original 404', async () => { const { calls, restore } = installFakeFetch(() => ({ status: 404 })); try { const res = await runProviderTest({ provider: 'anthropic-like', wire: 'anthropic', apiKey: 'sk-ant-test', - baseUrl: 'https://api.anthropic.com', + baseUrl: 'https://proxy.example.com/anthropic', }); expect(res.ok).toBe(false); if (!res.ok) expect(res.code).toBe('404'); if (!res.ok) expect(res.compatibility).toBe('incompatible'); // Only /v1/models should have been probed — no /v1/messages degrade. - expect(calls).toHaveLength(1); + expect(calls).toHaveLength(2); expect(calls[0]?.url).toMatch(/\/v1\/models$/); + expect(calls[1]?.url).toMatch(/\/v1\/messages$/); + } finally { + restore(); + } + }); + + it('anthropic: /models 404 + /v1/messages 400 degrades because Messages endpoint is alive', async () => { + const { calls, restore } = installFakeFetch((url) => { + if (url.endsWith('/v1/models')) return { status: 404 }; + if (url.endsWith('/v1/messages')) return { status: 400, body: { error: 'model missing' } }; + return { status: 500 }; + }); + try { + const res = await runProviderTest({ + provider: 'anthropic-like', + wire: 'anthropic', + apiKey: 'sk-ant-test', + baseUrl: 'https://proxy.example.com/anthropic', + }); + expect(res.ok).toBe(true); + if (res.ok) { + expect(res.probeMethod).toBe('anthropic_messages_degraded'); + expect(res.compatibility).toBe('degraded'); + } + expect(calls).toHaveLength(2); + expect(calls[0]?.url).toMatch(/\/v1\/models$/); + expect(calls[1]?.url).toMatch(/\/v1\/messages$/); + expect(calls[1]?.method).toBe('POST'); + const body = JSON.parse(calls[1]?.body ?? '{}'); + expect(body.max_tokens).toBe(1); + expect(body.stream).toBe(false); + expect(Array.isArray(body.messages)).toBe(true); } finally { restore(); } diff --git a/apps/desktop/src/main/connection-ipc.ts b/apps/desktop/src/main/connection-ipc.ts index 3b635421..6b59aa56 100644 --- a/apps/desktop/src/main/connection-ipc.ts +++ b/apps/desktop/src/main/connection-ipc.ts @@ -69,7 +69,11 @@ export interface ConnectionTestResult { * endpoint so a gateway that only implements /chat/completions can't * false-positive for a user whose provider is on the Responses API. */ - probeMethod?: 'models' | 'chat_completion_degraded' | 'responses_degraded'; + probeMethod?: + | 'models' + | 'chat_completion_degraded' + | 'responses_degraded' + | 'anthropic_messages_degraded'; compatibility?: 'compatible' | 'degraded'; reasonCategory?: DiagnosticCategory; } @@ -552,7 +556,12 @@ export async function runProviderTest( // chat request before declaring the endpoint dead. We intentionally do // not degrade anthropic — its /v1/models is standard, and skipping it // would mask real path-shape mistakes. - if (res.status === 404 && (creds.wire === 'openai-chat' || creds.wire === 'openai-responses')) { + if ( + res.status === 404 && + (creds.wire === 'openai-chat' || + creds.wire === 'openai-responses' || + creds.wire === 'anthropic') + ) { const degraded = await tryDegradeProbe(creds.wire, normalizedBaseUrl, headers); if (degraded !== null) return degraded; // Inference endpoint also 404'd (or the network dropped) — fall through @@ -572,7 +581,7 @@ export async function runProviderTest( } async function tryDegradeProbe( - wire: 'openai-chat' | 'openai-responses', + wire: 'openai-chat' | 'openai-responses' | 'anthropic', normalizedBaseUrl: string, headers: Record, ): Promise { @@ -580,7 +589,12 @@ async function tryDegradeProbe( if (probe.kind === 'pass') { return { ok: true, - probeMethod: wire === 'openai-responses' ? 'responses_degraded' : 'chat_completion_degraded', + probeMethod: + wire === 'openai-responses' + ? 'responses_degraded' + : wire === 'anthropic' + ? 'anthropic_messages_degraded' + : 'chat_completion_degraded', compatibility: 'degraded', reasonCategory: 'model-discovery-degraded', }; @@ -616,28 +630,37 @@ type ProbeResult = * shape with a 4xx we still know the route exists. */ async function probeInferenceEndpoint( - wire: 'openai-chat' | 'openai-responses', + wire: 'openai-chat' | 'openai-responses' | 'anthropic', normalizedBaseUrl: string, headers: Record, ): Promise { const url = - wire === 'openai-responses' - ? `${normalizedBaseUrl}/responses` - : `${normalizedBaseUrl}/chat/completions`; + wire === 'anthropic' + ? `${normalizedBaseUrl}/v1/messages` + : wire === 'openai-responses' + ? `${normalizedBaseUrl}/responses` + : `${normalizedBaseUrl}/chat/completions`; const body = - wire === 'openai-responses' + wire === 'anthropic' ? JSON.stringify({ - model: 'probe', - input: [{ role: 'user', content: [{ type: 'input_text', text: 'ping' }] }], - max_output_tokens: 1, - stream: false, - }) - : JSON.stringify({ model: 'probe', messages: [{ role: 'user', content: 'ping' }], max_tokens: 1, stream: false, - }); + }) + : wire === 'openai-responses' + ? JSON.stringify({ + model: 'probe', + input: [{ role: 'user', content: [{ type: 'input_text', text: 'ping' }] }], + max_output_tokens: 1, + stream: false, + }) + : JSON.stringify({ + model: 'probe', + messages: [{ role: 'user', content: 'ping' }], + max_tokens: 1, + stream: false, + }); let res: Response; try { res = await fetchWithTimeout(url, { diff --git a/apps/desktop/src/main/ipc/generate.ts b/apps/desktop/src/main/ipc/generate.ts index 4237f158..1a3b2a6c 100644 --- a/apps/desktop/src/main/ipc/generate.ts +++ b/apps/desktop/src/main/ipc/generate.ts @@ -334,7 +334,7 @@ export function registerGenerateIpc({ db, getMainWindow }: RegisterGenerateIpcDe logIpc.info('agent.tool_end', { generationId: id, tool: event.toolName, - isError: event.isError, + isError: event.toolName === 'set_todos' ? false : event.isError, }); } else if (event.type === 'turn_end') { logIpc.info('agent.turn_end', { diff --git a/apps/desktop/src/main/snapshots-db.test.ts b/apps/desktop/src/main/snapshots-db.test.ts index b9b30416..84016d0d 100644 --- a/apps/desktop/src/main/snapshots-db.test.ts +++ b/apps/desktop/src/main/snapshots-db.test.ts @@ -14,6 +14,7 @@ import { recordDiagnosticEvent, updateDesignWorkspace, } from './snapshots-db'; +import { normalizeWorkspacePath } from './workspace-path'; describe('json design store', () => { it('persists designs and snapshots without a native database binding', async () => { @@ -33,7 +34,7 @@ describe('json design store', () => { }); const reopened = initSnapshotsDb(storePath); - expect(getDesign(reopened, design.id)?.workspacePath).toBe(root); + expect(getDesign(reopened, design.id)?.workspacePath).toBe(normalizeWorkspacePath(root)); expect(listDesigns(reopened).map((row) => row.id)).toEqual([design.id]); expect(listSnapshots(reopened, design.id).map((row) => row.id)).toEqual([snapshot.id]); await expect(readFile(storePath, 'utf8')).resolves.toContain('Workspace-first design'); diff --git a/apps/desktop/src/main/snapshots-ipc.ts b/apps/desktop/src/main/snapshots-ipc.ts index 8c4318a6..22096320 100644 --- a/apps/desktop/src/main/snapshots-ipc.ts +++ b/apps/desktop/src/main/snapshots-ipc.ts @@ -763,6 +763,7 @@ export function registerWorkspaceIpc(db: Database, getWin: () => BrowserWindow | if (design === null) { throw new CodesignError('Design not found', 'IPC_NOT_FOUND'); } + if (design.workspacePath === null) return []; const workspacePath = requireBoundWorkspacePath(design, 'Design is not bound to a workspace'); try { return await listWorkspaceFilesAt(workspacePath); @@ -793,6 +794,16 @@ export function registerWorkspaceIpc(db: Database, getWin: () => BrowserWindow | if (design === null) { throw new CodesignError('Design not found', 'IPC_NOT_FOUND'); } + if (design.workspacePath === null) { + const requestedPath = r['path'] as string; + return { + path: requestedPath, + kind: classifyWorkspaceFileKind(requestedPath), + size: 0, + updatedAt: new Date(0).toISOString(), + content: '', + }; + } const workspacePath = requireBoundWorkspacePath(design, 'Design is not bound to a workspace'); try { return await readWorkspaceFileAt(workspacePath, r['path'] as string); diff --git a/apps/desktop/src/main/workspace-reader.test.ts b/apps/desktop/src/main/workspace-reader.test.ts index 8403f8ad..13a418b7 100644 --- a/apps/desktop/src/main/workspace-reader.test.ts +++ b/apps/desktop/src/main/workspace-reader.test.ts @@ -64,7 +64,7 @@ describe('readWorkspaceFilesAt', () => { ); const result = await readWorkspaceFilesAt(root); expect(result.length).toBe(200); - }); + }, 20_000); it('caps total bytes at 2 MB', async () => { // 20 × 150KB = 3 MB total. We expect the reader to stop before pulling diff --git a/apps/desktop/src/main/workspace-watcher.test.ts b/apps/desktop/src/main/workspace-watcher.test.ts index 7c7df167..889cca3d 100644 --- a/apps/desktop/src/main/workspace-watcher.test.ts +++ b/apps/desktop/src/main/workspace-watcher.test.ts @@ -1,3 +1,5 @@ +import { tmpdir } from 'node:os'; +import path from 'node:path'; import { describe, expect, it, vi } from 'vitest'; const handlers = new Map unknown>(); @@ -80,6 +82,10 @@ function captureError(fn: () => unknown): unknown { } } +function tempWorkspace(name: string): string { + return path.join(tmpdir(), name).replaceAll('\\', '/'); +} + describe('files-watcher subscribe / unsubscribe', () => { it('rejects when no design row found', () => { reset(); @@ -122,7 +128,10 @@ describe('files-watcher subscribe / unsubscribe', () => { watchMock.mockImplementation(() => { throw new Error('watch denied'); }); - getDesignMock.mockReturnValue({ id: 'd1', workspacePath: '/tmp/ws' }); + getDesignMock.mockReturnValue({ + id: 'd1', + workspacePath: tempWorkspace('codesign-watch-denied'), + }); registerFilesWatcherIpc({} as never, () => null); const sub = getHandler('codesign:files:v1:subscribe'); @@ -137,7 +146,10 @@ describe('files-watcher subscribe / unsubscribe', () => { vi.useFakeTimers(); const closeSpy = vi.fn(); watchMock.mockImplementation(() => ({ on: vi.fn(), close: closeSpy }) as never); - getDesignMock.mockReturnValue({ id: 'd1', workspacePath: '/tmp/ws' }); + getDesignMock.mockReturnValue({ + id: 'd1', + workspacePath: tempWorkspace('codesign-watch-refcount'), + }); registerFilesWatcherIpc({} as never, () => null); const sub = getHandler('codesign:files:v1:subscribe'); const unsub = getHandler('codesign:files:v1:unsubscribe'); @@ -165,9 +177,11 @@ describe('files-watcher subscribe / unsubscribe', () => { watchMock .mockImplementationOnce(() => ({ on: vi.fn(), close: closeFirst }) as never) .mockImplementationOnce(() => ({ on: vi.fn(), close: closeSecond }) as never); + const firstWorkspace = tempWorkspace('codesign-watch-one'); + const secondWorkspace = tempWorkspace('codesign-watch-two'); getDesignMock - .mockReturnValueOnce({ id: 'd1', workspacePath: '/tmp/ws-one/' }) - .mockReturnValueOnce({ id: 'd1', workspacePath: '/tmp/ws-two' }); + .mockReturnValueOnce({ id: 'd1', workspacePath: `${firstWorkspace}/` }) + .mockReturnValueOnce({ id: 'd1', workspacePath: secondWorkspace }); registerFilesWatcherIpc({} as never, () => null); const sub = getHandler('codesign:files:v1:subscribe'); @@ -178,13 +192,13 @@ describe('files-watcher subscribe / unsubscribe', () => { expect(closeSecond).not.toHaveBeenCalled(); expect(watchMock).toHaveBeenNthCalledWith( 1, - '/tmp/ws-one', + firstWorkspace, { recursive: true }, expect.any(Function), ); expect(watchMock).toHaveBeenNthCalledWith( 2, - '/tmp/ws-two', + secondWorkspace, { recursive: true }, expect.any(Function), ); diff --git a/apps/desktop/src/renderer/src/components/PreviewPane.tsx b/apps/desktop/src/renderer/src/components/PreviewPane.tsx index b991d308..2f0eb2c2 100644 --- a/apps/desktop/src/renderer/src/components/PreviewPane.tsx +++ b/apps/desktop/src/renderer/src/components/PreviewPane.tsx @@ -164,7 +164,7 @@ function PreviewSlot({ ); } else { body = ( -
+
{showCommentUi && active ? (
{commentHintLabel}
) : null} diff --git a/apps/desktop/src/renderer/src/components/settings/StorageTab.tsx b/apps/desktop/src/renderer/src/components/settings/StorageTab.tsx index 6d2fe37e..a97bba94 100644 --- a/apps/desktop/src/renderer/src/components/settings/StorageTab.tsx +++ b/apps/desktop/src/renderer/src/components/settings/StorageTab.tsx @@ -123,6 +123,9 @@ export function StorageTab() {

{t('settings.storage.restartHint')}

+

+ {t('settings.storage.workspaceHint')} +

{paths === null ? (
diff --git a/apps/desktop/src/renderer/src/preview/workspace-source.test.ts b/apps/desktop/src/renderer/src/preview/workspace-source.test.ts index 201c3fe7..8ff1a5a6 100644 --- a/apps/desktop/src/renderer/src/preview/workspace-source.test.ts +++ b/apps/desktop/src/renderer/src/preview/workspace-source.test.ts @@ -84,4 +84,21 @@ describe('workspace preview source resolution', () => { }), ).rejects.toThrow(/Cannot resolve referenced preview source/); }); + + it('falls back to original source when referenced workspace read returns empty content', async () => { + const source = ''; + const read = vi.fn(async (_designId, path) => ({ + path, + content: '', + })); + + await expect( + resolveWorkspacePreviewSource({ + designId: 'd1', + source, + read, + requireReferencedSource: true, + }), + ).resolves.toEqual({ path: 'index.html', content: source }); + }); }); diff --git a/apps/desktop/src/renderer/src/preview/workspace-source.ts b/apps/desktop/src/renderer/src/preview/workspace-source.ts index bbec8c65..c8823f38 100644 --- a/apps/desktop/src/renderer/src/preview/workspace-source.ts +++ b/apps/desktop/src/renderer/src/preview/workspace-source.ts @@ -65,6 +65,13 @@ export async function resolveWorkspacePreviewSource(input: { } return { content: input.source, path }; } - const referenced = await input.read(input.designId, referencedPath); + let referenced: WorkspacePreviewReadResult; + try { + referenced = await input.read(input.designId, referencedPath); + } catch (err) { + if (input.requireReferencedSource) throw err; + return { content: input.source, path }; + } + if (referenced.content.trim().length === 0) return { content: input.source, path }; return { content: referenced.content, path: referenced.path }; } diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 8e8bc529..7461834a 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -644,29 +644,29 @@ describe('generateViaAgent()', () => { ).rejects.toMatchObject({ code: ERROR_CODES.GENERATION_INCOMPLETE }); }); - it('throws GENERATION_INCOMPLETE when done reported errors', async () => { + it('keeps a valid artifact with a warning when done reported errors', async () => { scriptedAgent = { assistantText: RESPONSE_WITH_ARTIFACT }; - await expect( - generateViaAgent( - { - prompt: 'design a meditation app', - history: [], - model: MODEL, - apiKey: 'sk-test', - initialResourceState: resourceState({ + const result = await generateViaAgent( + { + prompt: 'design a meditation app', + history: [], + model: MODEL, + apiKey: 'sk-test', + initialResourceState: resourceState({ + mutationSeq: 1, + lastDone: { + status: 'has_errors', + path: 'index.html', mutationSeq: 1, - lastDone: { - status: 'has_errors', - path: 'index.html', - mutationSeq: 1, - errorCount: 1, - checkedAt: '2026-04-28T00:00:00.000Z', - }, - }), - }, - { fs: makeStubFs({ 'index.html': SAMPLE_HTML }) }, - ), - ).rejects.toMatchObject({ code: ERROR_CODES.GENERATION_INCOMPLETE }); + errorCount: 1, + checkedAt: '2026-04-28T00:00:00.000Z', + }, + }), + }, + { fs: makeStubFs({ 'index.html': SAMPLE_HTML }) }, + ); + expect(result.artifacts).toHaveLength(1); + expect(result.warnings).toEqual([expect.stringContaining('done() reported unresolved errors')]); }); it('allows a done ok state that covers the latest mutation', async () => { @@ -1297,6 +1297,52 @@ describe('generateViaAgent() — transport-level retry', () => { expect(onRetry).toHaveBeenCalledTimes(1); }); + it('retries a provider-side aborted transport error when the user signal is still live', async () => { + scriptedAgent = { + assistantText: RESPONSE_WITH_ARTIFACT, + stopReason: 'aborted', + errorMessage: 'Request was aborted', + overrideScriptForCallIndex: 1, + overrideScript: { + assistantText: RESPONSE_WITH_ARTIFACT, + stopReason: 'stop', + }, + }; + const onRetry = vi.fn(); + const result = await generateViaAgent( + { + prompt: 'design a meditation app', + history: [], + model: MODEL, + apiKey: 'sk-test', + }, + { onRetry, fs: makeStubFs({ 'index.html': SAMPLE_HTML }) }, + ); + expect(result.artifacts).toHaveLength(1); + expect(agentCalls.length).toBe(2); + expect(onRetry).toHaveBeenCalledTimes(1); + }); + + it('does not retry aborted transport errors after the caller signal is aborted', async () => { + scriptedAgent = { + assistantText: RESPONSE_WITH_ARTIFACT, + stopReason: 'aborted', + errorMessage: 'Request was aborted', + }; + const ctrl = new AbortController(); + ctrl.abort(); + await expect( + generateViaAgent({ + prompt: 'design a meditation app', + history: [], + model: MODEL, + apiKey: 'sk-test', + signal: ctrl.signal, + }), + ).rejects.toBeTruthy(); + expect(agentCalls.length).toBe(1); + }); + it('does not retry non-transport errors like 400', async () => { scriptedAgent = { assistantText: '', @@ -1504,7 +1550,9 @@ describe('loadFrameTemplates — device frame starter assets', () => { }); it('rejects symlinked frame template files', async () => { - const { mkdirSync, rmSync, symlinkSync, writeFileSync } = await import('node:fs'); + const { existsSync, mkdirSync, rmSync, symlinkSync, unlinkSync, writeFileSync } = await import( + 'node:fs' + ); const { tmpdir } = await import('node:os'); const path = await import('node:path'); const { FRAME_FILES, loadFrameTemplates } = await import('./frames/index.js'); @@ -1519,9 +1567,11 @@ describe('loadFrameTemplates — device frame starter assets', () => { const first = FRAME_FILES[0]; if (first === undefined) throw new Error('expected at least one frame file'); writeFileSync(path.join(outside, 'secret.jsx'), 'secret', 'utf8'); - rmSync(path.join(dir, first)); + const linkPath = path.join(dir, first); + rmSync(linkPath, { force: true }); + if (existsSync(linkPath)) unlinkSync(linkPath); try { - symlinkSync(path.join(outside, 'secret.jsx'), path.join(dir, first)); + symlinkSync(path.join(outside, 'secret.jsx'), linkPath, 'file'); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'EPERM') return; throw err; diff --git a/packages/core/src/agent.ts b/packages/core/src/agent.ts index 78700b62..c8f6194e 100644 --- a/packages/core/src/agent.ts +++ b/packages/core/src/agent.ts @@ -33,6 +33,7 @@ import { classifyError, claudeCodeIdentityHeaders, inferReasoning, + isProviderAbortedTransportError, isTransportLevelError, looksLikeClaudeOAuthToken, normalizeGeminiModelId, @@ -811,9 +812,15 @@ export async function generateViaAgent( while (transportRetryCount < MAX_TRANSPORT_RETRIES) { const checkMsg = findFinalAssistantMessage(agent.state.messages); if (!checkMsg || checkMsg.stopReason === 'stop') break; - if (checkMsg.stopReason !== 'error') break; - if (!isTransportLevelError(checkMsg.errorMessage)) break; if (input.signal?.aborted) break; + const retryableTransportFailure = + checkMsg.stopReason === 'error' + ? isTransportLevelError(checkMsg.errorMessage) + : checkMsg.stopReason === 'aborted' && + isProviderAbortedTransportError( + checkMsg.errorMessage ?? messageForIncompleteStop(checkMsg.stopReason), + ); + if (!retryableTransportFailure) break; transportRetryCount++; log.warn('[generate] step=transport_retry', { @@ -914,13 +921,15 @@ export async function generateViaAgent( collected.artifacts.push(createHtmlArtifact(file.content, 0)); } } - if (deps.tools === undefined && deps.fs !== undefined) { - assertFinalizationGate({ - state: resourceState, - fs: deps.fs, - enforce: resourceState.mutationSeq > 0, - }); - } + const finalizationWarnings = + deps.tools === undefined && deps.fs !== undefined + ? assertFinalizationGate({ + state: resourceState, + fs: deps.fs, + enforce: resourceState.mutationSeq > 0, + allowUnresolvedDoneWithArtifact: collected.artifacts.length > 0, + }) + : []; log.info('[generate] step=parse_response.ok', { ...ctx, ms: Date.now() - parseStart, @@ -938,8 +947,9 @@ export async function generateViaAgent( costUsd: usage?.cost?.total ?? 0, resourceState, }; - return resourceResult.warnings.length > 0 - ? { ...output, warnings: [...(output.warnings ?? []), ...resourceResult.warnings] } + const warnings = [...finalizationWarnings, ...resourceResult.warnings]; + return warnings.length > 0 + ? { ...output, warnings: [...(output.warnings ?? []), ...warnings] } : output; } diff --git a/packages/core/src/design-skills/index.test.ts b/packages/core/src/design-skills/index.test.ts index 785f1e3d..0758b8af 100644 --- a/packages/core/src/design-skills/index.test.ts +++ b/packages/core/src/design-skills/index.test.ts @@ -1,14 +1,15 @@ -import { mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { randomUUID } from 'node:crypto'; +import { existsSync, mkdirSync, rmSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { DESIGN_SKILL_FILES, loadDesignSkills } from './index.js'; -describe('loadDesignSkills', () => { +describe.sequential('loadDesignSkills', () => { let dir: string; beforeEach(() => { - dir = path.join(tmpdir(), `codesign-design-skills-${process.pid}-${Date.now()}`); + dir = path.join(tmpdir(), `codesign-design-skills-${process.pid}-${randomUUID()}`); mkdirSync(dir, { recursive: true }); }); @@ -44,13 +45,18 @@ describe('loadDesignSkills', () => { writeAll('placeholder'); const first = DESIGN_SKILL_FILES[0]; if (first === undefined) throw new Error('expected at least one design skill file'); - const outside = path.join(tmpdir(), `codesign-design-skills-out-${process.pid}-${Date.now()}`); + const outside = path.join( + tmpdir(), + `codesign-design-skills-out-${process.pid}-${randomUUID()}`, + ); mkdirSync(outside, { recursive: true }); writeFileSync(path.join(outside, 'secret.jsx'), 'secret', 'utf8'); - rmSync(path.join(dir, first)); + const linkPath = path.join(dir, first); + rmSync(linkPath, { force: true }); + if (existsSync(linkPath)) unlinkSync(linkPath); try { try { - symlinkSync(path.join(outside, 'secret.jsx'), path.join(dir, first)); + symlinkSync(path.join(outside, 'secret.jsx'), linkPath, 'file'); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'EPERM') return; throw err; diff --git a/packages/core/src/design-skills/index.ts b/packages/core/src/design-skills/index.ts index ebb481a7..8ab42686 100644 --- a/packages/core/src/design-skills/index.ts +++ b/packages/core/src/design-skills/index.ts @@ -45,13 +45,15 @@ async function assertTemplatePathIsNotSymlink(filePath: string): Promise { * canonical order defined by `DESIGN_SKILL_FILES`. */ export async function loadDesignSkills(dir: string): Promise> { + let entries: string[]; try { - await readdir(dir); + entries = await readdir(dir); await assertTemplatePathIsNotSymlink(dir); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; throw err; } + if (entries.length === 0) return []; return Promise.all( DESIGN_SKILL_FILES.map(async (name): Promise<[string, string]> => { const filePath = path.join(dir, name); diff --git a/packages/core/src/prompts/sections/output-rules.md b/packages/core/src/prompts/sections/output-rules.md index cc54087a..0c6389a3 100644 --- a/packages/core/src/prompts/sections/output-rules.md +++ b/packages/core/src/prompts/sections/output-rules.md @@ -21,3 +21,5 @@ - Use CSS custom properties or a token object for load-bearing visual values. - Content must be domain-specific: no lorem ipsum, "John Doe", "Acme Corp", placeholder numbers, or stale dates. - Responsive behavior is required for user-facing surfaces unless the artifact is an intentionally fixed-format slide or frame. +- Keep text readable across the preview's mobile, tablet, and desktop viewports. Prefer `rem`, `%`, viewport-aware layout, and `clamp()` for important type; avoid tiny fixed `px` labels that become unreadable after resizing. +- Prevent accidental horizontal clipping. Use `box-sizing: border-box`, responsive widths, `max-width: 100%`, and deliberate `overflow-x` behavior for wide slide/report surfaces. diff --git a/packages/core/src/resource-state.ts b/packages/core/src/resource-state.ts index 87485d24..896cb001 100644 --- a/packages/core/src/resource-state.ts +++ b/packages/core/src/resource-state.ts @@ -49,6 +49,7 @@ export interface FinalizationGateInput { state: ResourceStateV1; fs: TextEditorFsCallbacks; enforce: boolean; + allowUnresolvedDoneWithArtifact?: boolean | undefined; } function hasRealChartMarkup(source: string): boolean { @@ -84,8 +85,8 @@ function validationFailures(state: ResourceStateV1, source: string): string[] { return failures; } -export function assertFinalizationGate(input: FinalizationGateInput): void { - if (!input.enforce) return; +export function assertFinalizationGate(input: FinalizationGateInput): string[] { + if (!input.enforce) return []; const file = input.fs.view('index.html'); if (file === null || file.content.trim().length === 0) { throw new CodesignError( @@ -101,6 +102,9 @@ export function assertFinalizationGate(input: FinalizationGateInput): void { ); } if (done.status !== 'ok') { + if (input.allowUnresolvedDoneWithArtifact) { + return ['done() reported unresolved errors; keeping the generated artifact available.']; + } throw new CodesignError( 'Generation incomplete: done() reported unresolved errors.', ERROR_CODES.GENERATION_INCOMPLETE, @@ -119,4 +123,5 @@ export function assertFinalizationGate(input: FinalizationGateInput): void { ERROR_CODES.GENERATION_INCOMPLETE, ); } + return []; } diff --git a/packages/core/src/tools/done.ts b/packages/core/src/tools/done.ts index 888d34c0..d62d1455 100644 --- a/packages/core/src/tools/done.ts +++ b/packages/core/src/tools/done.ts @@ -306,7 +306,7 @@ export function makeDoneTool( 'syntax checks AND loads the file in an isolated runtime to capture ' + 'console errors / load failures, then replies with ' + '`{ status: "ok" | "has_errors", errors: [...] }`. If errors come back, ' + - 'fix them with str_replace_based_edit_tool and call `done` again. ' + + 'you MUST fix them with str_replace_based_edit_tool and call `done` again. ' + 'Stop calling once status is "ok" or after 5 rounds.', parameters: DoneParams, async execute(_id, params): Promise> { diff --git a/packages/core/src/tools/skill.test.ts b/packages/core/src/tools/skill.test.ts index 08a60cc4..309d62cc 100644 --- a/packages/core/src/tools/skill.test.ts +++ b/packages/core/src/tools/skill.test.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import { mkdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -5,12 +6,12 @@ import { CodesignError } from '@open-codesign/shared'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { invokeSkill, listSkillManifest, makeSkillTool } from './skill'; -describe('skill tool', () => { +describe.sequential('skill tool', () => { let skillsRoot: string; let brandRefsRoot: string; beforeEach(() => { - const base = path.join(tmpdir(), `codesign-skill-${process.pid}-${Date.now()}`); + const base = path.join(tmpdir(), `codesign-skill-${process.pid}-${randomUUID()}`); skillsRoot = path.join(base, 'skills'); brandRefsRoot = path.join(base, 'brand-refs'); mkdirSync(skillsRoot, { recursive: true }); @@ -101,7 +102,13 @@ describe('skill tool', () => { }); it('throws when a registered skill file cannot be read', async () => { - rmSync(path.join(brandRefsRoot, 'demo', 'DESIGN.md')); + writeFileSync( + path.join(brandRefsRoot, 'manifest.json'), + JSON.stringify({ + brands: [{ slug: 'demo', name: 'Demo', category: 'Brand', path: 'demo/MISSING.md' }], + }), + 'utf8', + ); await expect( invokeSkill({ name: 'brand:demo', roots: { skillsRoot, brandRefsRoot } }), ).rejects.toSatisfy( @@ -110,7 +117,7 @@ describe('skill tool', () => { }); it('rejects brand refs that traverse symlinked template segments', async () => { - const outside = path.join(tmpdir(), `codesign-skill-outside-${process.pid}-${Date.now()}`); + const outside = path.join(tmpdir(), `codesign-skill-outside-${process.pid}-${randomUUID()}`); mkdirSync(outside, { recursive: true }); writeFileSync(path.join(outside, 'DESIGN.md'), '# Outside Brand\n', 'utf8'); try { diff --git a/packages/exporters/src/pdf.test.ts b/packages/exporters/src/pdf.test.ts index ea686312..2763df89 100644 --- a/packages/exporters/src/pdf.test.ts +++ b/packages/exporters/src/pdf.test.ts @@ -4,6 +4,7 @@ import { join } from 'node:path'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; const fakePdfBytes = Buffer.from('%PDF-1.4 fake'); +const CHROME_TEST_TIMEOUT_MS = process.platform === 'win32' ? 30_000 : 10_000; const launchMock = vi.fn(); const newPageMock = vi.fn(); @@ -46,42 +47,54 @@ afterAll(() => { }); describe('exportPdf', () => { - it('writes a PDF via puppeteer-core against the discovered Chrome', async () => { - const { exportPdf } = await import('./pdf'); - const dest = join(tempDir, 'out.pdf'); - const result = await exportPdf('

hi

', dest); + it( + 'writes a PDF via puppeteer-core against the discovered Chrome', + async () => { + const { exportPdf } = await import('./pdf'); + const dest = join(tempDir, 'out.pdf'); + const result = await exportPdf('

hi

', dest); - expect(launchMock).toHaveBeenCalledWith( - expect.objectContaining({ - executablePath: expect.stringContaining('Chrome'), - headless: true, - }), - ); - expect(setContentMock).toHaveBeenCalledWith( - '

hi

', - expect.objectContaining({ waitUntil: 'networkidle0' }), - ); - expect(pdfMock).toHaveBeenCalled(); - expect(closeMock).toHaveBeenCalled(); - expect(result.path).toBe(dest); - expect(result.bytes).toBe(fakePdfBytes.length); - }); + expect(launchMock).toHaveBeenCalledWith( + expect.objectContaining({ + executablePath: expect.stringContaining('Chrome'), + headless: true, + }), + ); + expect(setContentMock).toHaveBeenCalledWith( + '

hi

', + expect.objectContaining({ waitUntil: 'networkidle0' }), + ); + expect(pdfMock).toHaveBeenCalled(); + expect(closeMock).toHaveBeenCalled(); + expect(result.path).toBe(dest); + expect(result.bytes).toBe(fakePdfBytes.length); + }, + CHROME_TEST_TIMEOUT_MS, + ); - it('respects a chromePath override (no discovery call needed)', async () => { - launchMock.mockClear(); - const { exportPdf } = await import('./pdf'); - const dest = join(tempDir, 'override.pdf'); - await exportPdf('

x

', dest, { chromePath: '/tmp/fake-chrome' }); - expect(launchMock).toHaveBeenCalledWith( - expect.objectContaining({ executablePath: '/tmp/fake-chrome' }), - ); - }); + it( + 'respects a chromePath override (no discovery call needed)', + async () => { + launchMock.mockClear(); + const { exportPdf } = await import('./pdf'); + const dest = join(tempDir, 'override.pdf'); + await exportPdf('

x

', dest, { chromePath: '/tmp/fake-chrome' }); + expect(launchMock).toHaveBeenCalledWith( + expect.objectContaining({ executablePath: '/tmp/fake-chrome' }), + ); + }, + CHROME_TEST_TIMEOUT_MS, + ); - it('wraps puppeteer failures in EXPORTER_PDF_FAILED', async () => { - pdfMock.mockRejectedValueOnce(new Error('boom')); - const { exportPdf } = await import('./pdf'); - await expect(exportPdf('

x

', join(tempDir, 'fail.pdf'))).rejects.toMatchObject({ - code: 'EXPORTER_PDF_FAILED', - }); - }); + it( + 'wraps puppeteer failures in EXPORTER_PDF_FAILED', + async () => { + pdfMock.mockRejectedValueOnce(new Error('boom')); + const { exportPdf } = await import('./pdf'); + await expect(exportPdf('

x

', join(tempDir, 'fail.pdf'))).rejects.toMatchObject({ + code: 'EXPORTER_PDF_FAILED', + }); + }, + CHROME_TEST_TIMEOUT_MS, + ); }); diff --git a/packages/i18n/src/locales/en.json b/packages/i18n/src/locales/en.json index 873fa91e..51ba6da7 100644 --- a/packages/i18n/src/locales/en.json +++ b/packages/i18n/src/locales/en.json @@ -241,6 +241,7 @@ "empty": "No providers configured yet. Add one to start generating.", "active": "Active", "activeNotInList": "(active, not in provider list)", + "noModel": "No model selected", "decryptionFailed": "Decryption failed", "setActive": "Set active", "reEnterKey": "Re-enter key", @@ -416,6 +417,7 @@ "data": "Data directory", "change": "Change", "restartHint": "Choose where open-codesign stores config, logs, and local design data. Changes are saved permanently and take effect after restarting the app.", + "workspaceHint": "Design workspace folders are managed per design from the Files panel. Changing these storage paths does not move or rebind existing workspace files.", "locationSavedToast": "Storage location saved. Restart the app to apply it.", "locationSaveFailed": "Could not save storage location", "onboardingTitle": "Onboarding", diff --git a/packages/i18n/src/locales/pt-BR.json b/packages/i18n/src/locales/pt-BR.json index c35721c0..5a084e6f 100644 --- a/packages/i18n/src/locales/pt-BR.json +++ b/packages/i18n/src/locales/pt-BR.json @@ -212,6 +212,7 @@ "addCustom": "Adicionar personalizado", "empty": "Nenhum provedor configurado ainda. Adicione um para começar a gerar.", "active": "Ativo", + "noModel": "Nenhum modelo selecionado", "decryptionFailed": "Falha na descriptografia", "setActive": "Definir como ativo", "reEnterKey": "Redigitar chave", @@ -381,6 +382,7 @@ "data": "Diretório de dados", "change": "Alterar", "restartHint": "Escolha onde o open-codesign guarda configuração, logs e dados locais de design. As alterações são salvas permanentemente e se aplicam depois de reiniciar o app.", + "workspaceHint": "Pastas de workspace são gerenciadas por design no painel Arquivos. Alterar estes caminhos de armazenamento não move nem revincula arquivos de workspace existentes.", "locationSavedToast": "Local de armazenamento salvo. Reinicie o app para aplicar.", "locationSaveFailed": "Não foi possível salvar o local de armazenamento", "onboardingTitle": "Onboarding", diff --git a/packages/i18n/src/locales/zh-CN.json b/packages/i18n/src/locales/zh-CN.json index 0437ad95..e0e1ea83 100644 --- a/packages/i18n/src/locales/zh-CN.json +++ b/packages/i18n/src/locales/zh-CN.json @@ -241,6 +241,7 @@ "empty": "还没有配置任何服务，添加一个即可开始生成。", "active": "当前", "activeNotInList": "（当前，不在服务返回的列表中）", + "noModel": "未选择模型", "decryptionFailed": "解密失败", "setActive": "设为当前", "reEnterKey": "重新输入 Key", @@ -416,6 +417,7 @@ "data": "数据目录", "change": "更改", "restartHint": "选择 open-codesign 保存配置、日志和本地设计数据的位置。更改会永久保存，重启应用后生效。", + "workspaceHint": "设计工作区按单个设计在文件面板中管理。修改这些存储路径不会移动或重新绑定已有工作区文件。", "locationSavedToast": "存储位置已保存。重启应用后生效。", "locationSaveFailed": "无法保存存储位置", "onboardingTitle": "首次引导", diff --git a/packages/providers/src/index.ts b/packages/providers/src/index.ts index d315f686..96cbee68 100644 --- a/packages/providers/src/index.ts +++ b/packages/providers/src/index.ts @@ -556,6 +556,7 @@ export type { export { classifyError, completeWithRetry, + isProviderAbortedTransportError, isTransportLevelError, sleepWithAbort, withBackoff, diff --git a/packages/providers/src/retry.test.ts b/packages/providers/src/retry.test.ts index ccd1504d..25c04672 100644 --- a/packages/providers/src/retry.test.ts +++ b/packages/providers/src/retry.test.ts @@ -1,7 +1,13 @@ import { type ChatMessage, CodesignError, type ModelRef } from '@open-codesign/shared'; import { describe, expect, it, vi } from 'vitest'; import type { GenerateOptions, GenerateResult } from './index'; -import { classifyError, completeWithRetry, type RetryReason, withBackoff } from './retry'; +import { + classifyError, + completeWithRetry, + isProviderAbortedTransportError, + type RetryReason, + withBackoff, +} from './retry'; const MODEL: ModelRef = { provider: 'anthropic', modelId: 'claude-sonnet-4-6' }; const MESSAGES: ChatMessage[] = [{ role: 'user', content: 'hi' }]; @@ -99,6 +105,11 @@ describe('classifyError', () => { const d = classifyError(err); expect(d.retry).toBe(true); }); + it('recognizes provider-side aborted transport messages for agent replay only', () => { + expect(isProviderAbortedTransportError('Request was aborted')).toBe(true); + expect(isProviderAbortedTransportError('Generation aborted by provider')).toBe(true); + expect(isProviderAbortedTransportError('Generation aborted by user')).toBe(false); + }); it('does not retry regular errors without transport indicators', () => { const d = classifyError(new Error('something went wrong')); expect(d.retry).toBe(false); diff --git a/packages/providers/src/retry.ts b/packages/providers/src/retry.ts index 7077f598..1225a667 100644 --- a/packages/providers/src/retry.ts +++ b/packages/providers/src/retry.ts @@ -86,12 +86,19 @@ function classifyByStatus(status: number, err: unknown, wire?: WireApi): RetryDe const TRANSPORT_ERROR_RE = /(?:fetch\s+failed.*\bterminated\b|\bterminated\b|premature\s+close|stream\s+(?:ended|closed)|ECONNRESET)\b/i; +const PROVIDER_ABORTED_TRANSPORT_RE = + /(?:request\s+was\s+aborted|generation\s+aborted\s+by\s+provider|provider\s+aborted|upstream\s+aborted)\b/i; export function isTransportLevelError(errorMessage: string | undefined): boolean { if (!errorMessage) return false; return TRANSPORT_ERROR_RE.test(errorMessage); } +export function isProviderAbortedTransportError(errorMessage: string | undefined): boolean { + if (!errorMessage) return false; + return PROVIDER_ABORTED_TRANSPORT_RE.test(errorMessage); +} + function classifyByNetwork(err: unknown): RetryDecision | undefined { if (err instanceof TypeError) return { retry: true, reason: 'network error' }; if (!(err instanceof Error)) return undefined; diff --git a/packages/runtime/src/index.test.ts b/packages/runtime/src/index.test.ts index 34d2661e..3bb6ec75 100644 --- a/packages/runtime/src/index.test.ts +++ b/packages/runtime/src/index.test.ts @@ -42,6 +42,15 @@ describe('buildSrcdoc', () => { expect(twice).toBe(once); }); + it('injects preview viewport support into full-HTML documents without duplicating viewport meta', () => { + const out = buildSrcdoc( + '

x

', + ); + expect(out).toContain('OPEN-CODESIGN-PREVIEW-VIEWPORT'); + expect(out).toContain('--codesign-preview-width'); + expect(out.match(/name="viewport"/g)).toHaveLength(1); + }); + it('injects the JSX runtime stack when a full-HTML payload uses `; +} + +function injectPreviewViewportSupportIntoHtmlDocument(html: string): string { + if (html.includes(PREVIEW_VIEWPORT_MARKER)) return html; + const tags = previewViewportSupportTags(); + const hasViewport = /]+name=["']viewport["'][^>]*>/i.test(html); + const support = hasViewport + ? tags.replace( + /\n/, + '', + ) + : tags; + if (/<\/head>/i.test(html)) { + return html.replace(/<\/head>/i, (close) => `${support}\n${close}`); + } + if (/]*>/i.test(html)) { + return html.replace(/(]*>)/i, `$1\n\n${support}\n`); + } + return `${support}\n${html}`; +} + function injectBaseHrefIntoHtmlDocument(html: string, baseHref: string | undefined): string { if (!baseHref || / Date: Sun, 3 May 2026 21:51:11 +0800 Subject: [PATCH 2/4] ci: restore DeepSeek PR review script Signed-off-by: Sun-sunshine06 --- .github/scripts/deepseek-pr-review.mjs | 118 +++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 .github/scripts/deepseek-pr-review.mjs diff --git a/.github/scripts/deepseek-pr-review.mjs b/.github/scripts/deepseek-pr-review.mjs new file mode 100644 index 00000000..68e7b7eb --- /dev/null +++ b/.github/scripts/deepseek-pr-review.mjs @@ -0,0 +1,118 @@ +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; + +const marker = '*open-codesign Bot*'; + +function requiredEnv(name) { + const value = process.env[name]; + if (!value) throw new Error(`${name} is required`); + return value; +} + +function runGit(args, fallback = '') { + try { + return execFileSync('git', args, { encoding: 'utf8', maxBuffer: 8 * 1024 * 1024 }); + } catch { + return fallback; + } +} + +async function github(path, init = {}) { + const token = requiredEnv('GITHUB_TOKEN'); + const response = await fetch(`https://api.github.com${path}`, { + ...init, + headers: { + accept: 'application/vnd.github+json', + authorization: `Bearer ${token}`, + 'content-type': 'application/json', + 'x-github-api-version': '2022-11-28', + ...init.headers, + }, + }); + if (!response.ok) { + const body = await response.text(); + throw new Error(`GitHub API ${response.status}: ${body}`); + } + return response.status === 204 ? null : response.json(); +} + +async function deepseekReview(prompt) { + const apiKey = requiredEnv('DEEPSEEK_API_KEY'); + const baseUrl = (process.env.DEEPSEEK_BASE_URL || 'https://api.deepseek.com').replace(/\/+$/, ''); + const model = process.env.DEEPSEEK_MODEL || 'deepseek-chat'; + const response = await fetch(`${baseUrl}/chat/completions`, { + method: 'POST', + headers: { + authorization: `Bearer ${apiKey}`, + 'content-type': 'application/json', + }, + body: JSON.stringify({ + model, + messages: [ + { + role: 'system', + content: + 'You are reviewing an Open CoDesign pull request. Prioritize correctness, regressions, missing tests, and security. Be concise.', + }, + { role: 'user', content: prompt }, + ], + temperature: 0.1, + }), + }); + if (!response.ok) { + const body = await response.text(); + throw new Error(`DeepSeek API ${response.status}: ${body}`); + } + const payload = await response.json(); + return payload?.choices?.[0]?.message?.content?.trim() || 'No review comments returned.'; +} + +function buildPrompt(event) { + const baseRef = event.pull_request.base.ref; + const baseSha = event.pull_request.base.sha; + const headSha = event.pull_request.head.sha; + const promptTemplate = readFileSync('.github/prompts/codex-pr-review.md', 'utf8'); + const stat = runGit(['diff', '--stat', `${baseSha}...${headSha}`]); + const diff = runGit(['diff', '--find-renames', '--unified=80', `${baseSha}...${headSha}`]); + const clippedDiff = + diff.length > 180_000 + ? `${diff.slice(0, 180_000)}\n\n[diff clipped at 180000 characters]\n` + : diff; + return `${promptTemplate} + +Repository: ${process.env.GITHUB_REPOSITORY} +Pull request: #${event.pull_request.number} +Base branch: ${baseRef} +Head SHA: ${headSha} + +Diff stat: +${stat} + +Diff: +${clippedDiff}`; +} + +async function main() { + const event = JSON.parse(readFileSync(requiredEnv('GITHUB_EVENT_PATH'), 'utf8')); + const [owner, repo] = requiredEnv('GITHUB_REPOSITORY').split('/'); + const pullNumber = event.pull_request.number; + const prompt = buildPrompt(event); + const review = await deepseekReview(prompt); + const body = `${marker} + +${review}`; + + await github(`/repos/${owner}/${repo}/pulls/${pullNumber}/reviews`, { + method: 'POST', + body: JSON.stringify({ + commit_id: process.env.CURRENT_HEAD_SHA || event.pull_request.head.sha, + event: 'COMMENT', + body, + }), + }); +} + +main().catch((error) => { + console.error(error instanceof Error ? error.stack || error.message : error); + process.exit(1); +}); From 67d36689f072d8caa5fc72104ec022d338443f9d Mon Sep 17 00:00:00 2001 From: Sun-sunshine06 Date: Sun, 3 May 2026 22:01:41 +0800 Subject: [PATCH 3/4] ci: align PR review bot marker Signed-off-by: Sun-sunshine06 --- .github/scripts/deepseek-pr-review.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/deepseek-pr-review.mjs b/.github/scripts/deepseek-pr-review.mjs index 68e7b7eb..ed4b9f98 100644 --- a/.github/scripts/deepseek-pr-review.mjs +++ b/.github/scripts/deepseek-pr-review.mjs @@ -1,7 +1,7 @@ import { execFileSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; -const marker = '*open-codesign Bot*'; +const marker = '*Open-CoDesign Bot*'; function requiredEnv(name) { const value = process.env[name]; From 9b8120c0bed5537c7fdebafecc44c493547eab84 Mon Sep 17 00:00:00 2001 From: Sun-sunshine06 Date: Sun, 3 May 2026 22:12:12 +0800 Subject: [PATCH 4/4] fix: address PR review follow-ups Signed-off-by: Sun-sunshine06 --- apps/desktop/scripts/after-pack-prune.cjs | 52 +++++++++-- apps/desktop/src/main/connection-ipc.test.ts | 30 ++++++- apps/desktop/src/main/connection-ipc.ts | 24 ++++++ apps/desktop/src/main/ipc/generate.ts | 3 +- apps/desktop/src/main/ipc/tool-log.test.ts | 56 ++++++++++++ apps/desktop/src/main/ipc/tool-log.ts | 44 ++++++++++ apps/desktop/src/main/snapshots-ipc.ts | 8 +- .../snapshots-ipc.workspace-files.test.ts | 86 +++++++++++++++++++ .../src/preview/workspace-source.test.ts | 16 ++++ .../renderer/src/preview/workspace-source.ts | 8 +- packages/core/src/agent.test.ts | 2 +- packages/core/src/design-skills/index.test.ts | 6 +- packages/core/src/tools/done.test.ts | 5 ++ packages/core/src/tools/done.ts | 4 +- packages/exporters/src/pdf.test.ts | 2 +- .../providers/src/codex/token-store.test.ts | 6 +- packages/providers/src/codex/token-store.ts | 17 +++- packages/providers/src/retry.test.ts | 5 ++ packages/providers/src/retry.ts | 2 +- 19 files changed, 351 insertions(+), 25 deletions(-) create mode 100644 apps/desktop/src/main/ipc/tool-log.test.ts create mode 100644 apps/desktop/src/main/ipc/tool-log.ts create mode 100644 apps/desktop/src/main/snapshots-ipc.workspace-files.test.ts diff --git a/apps/desktop/scripts/after-pack-prune.cjs b/apps/desktop/scripts/after-pack-prune.cjs index 983e26ea..3b7590f3 100644 --- a/apps/desktop/scripts/after-pack-prune.cjs +++ b/apps/desktop/scripts/after-pack-prune.cjs @@ -13,17 +13,53 @@ function archName(arch) { return typeof arch === 'number' ? (ARCH_NAMES[arch] ?? String(arch)) : String(arch); } +function sleepSync(ms) { + const deadline = Date.now() + ms; + while (Date.now() < deadline) { + // Busy wait is acceptable here: this is a short build-time retry delay. + } +} + +function withRmRetry(fn) { + for (let attempt = 0; attempt <= 3; attempt += 1) { + try { + fn(); + return; + } catch (err) { + if (err?.code === 'ENOENT') return; + if (!['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(err?.code) || attempt === 3) { + throw err; + } + sleepSync(50 * (attempt + 1)); + } + } +} + function rm(target) { - if (!fs.existsSync(target)) return; - const stat = fs.lstatSync(target); - if (!stat.isDirectory()) { - fs.unlinkSync(target); - return; + const pending = [target]; + const directories = []; + while (pending.length > 0) { + const current = pending.pop(); + if (!current) continue; + let stat; + try { + stat = fs.lstatSync(current); + } catch (err) { + if (err?.code === 'ENOENT') continue; + throw err; + } + if (!stat.isDirectory()) { + withRmRetry(() => fs.unlinkSync(current)); + continue; + } + directories.push(current); + for (const entry of fs.readdirSync(current)) { + pending.push(path.join(current, entry)); + } } - for (const entry of fs.readdirSync(target)) { - rm(path.join(target, entry)); + for (const dir of directories.reverse()) { + withRmRetry(() => fs.rmdirSync(dir)); } - fs.rmdirSync(target); } function existingDirs(paths) { diff --git a/apps/desktop/src/main/connection-ipc.test.ts b/apps/desktop/src/main/connection-ipc.test.ts index 6f82ca2f..6e5091fa 100644 --- a/apps/desktop/src/main/connection-ipc.test.ts +++ b/apps/desktop/src/main/connection-ipc.test.ts @@ -1056,7 +1056,12 @@ describe('runProviderTest degrade-probe (issue #179)', () => { it('anthropic: /models 404 + /v1/messages 400 degrades because Messages endpoint is alive', async () => { const { calls, restore } = installFakeFetch((url) => { if (url.endsWith('/v1/models')) return { status: 404 }; - if (url.endsWith('/v1/messages')) return { status: 400, body: { error: 'model missing' } }; + if (url.endsWith('/v1/messages')) { + return { + status: 400, + body: { error: { type: 'invalid_request_error', message: 'model missing' } }, + }; + } return { status: 500 }; }); try { @@ -1084,6 +1089,29 @@ describe('runProviderTest degrade-probe (issue #179)', () => { } }); + it('anthropic: /models 404 + generic /v1/messages 400 surfaces the 400', async () => { + const { restore } = installFakeFetch((url) => { + if (url.endsWith('/v1/models')) return { status: 404 }; + if (url.endsWith('/v1/messages')) return { status: 400, body: { error: 'bad request' } }; + return { status: 500 }; + }); + try { + const res = await runProviderTest({ + provider: 'anthropic-like', + wire: 'anthropic', + apiKey: 'sk-ant-test', + baseUrl: 'https://proxy.example.com/anthropic', + }); + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.code).toBe('NETWORK'); + expect(res.message).toBe('HTTP 400'); + } + } finally { + restore(); + } + }); + it('openai-responses: /models 404 + /responses 2xx → probeMethod=responses_degraded', async () => { const { calls, restore } = installFakeFetch((url) => { if (url.endsWith('/models')) return { status: 404 }; diff --git a/apps/desktop/src/main/connection-ipc.ts b/apps/desktop/src/main/connection-ipc.ts index 6b59aa56..ccc42755 100644 --- a/apps/desktop/src/main/connection-ipc.ts +++ b/apps/desktop/src/main/connection-ipc.ts @@ -676,10 +676,34 @@ async function probeInferenceEndpoint( // 401/403 — endpoint alive but auth rejected; surface as auth error so the // diagnostics panel shows the key-invalid hint instead of the 404 one. if (res.status === 401 || res.status === 403) return { kind: 'http', status: res.status }; + if (wire === 'anthropic') { + const body = await responseJson(res); + return hasAnthropicApiErrorShape(body) + ? { kind: 'pass' } + : { kind: 'http', status: res.status }; + } // 400/402/422/429 etc. — endpoint alive, request-level rejection. return { kind: 'pass' }; } +async function responseJson(res: Response): Promise { + try { + return await res.json(); + } catch { + return null; + } +} + +function isJsonRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +function hasAnthropicApiErrorShape(value: unknown): boolean { + if (!isJsonRecord(value)) return false; + const error = value['error']; + return isJsonRecord(error) && typeof error['type'] === 'string'; +} + export function registerConnectionIpc(): void { ipcMain.handle('connection:v1:test', (_e, raw: unknown) => handleConnectionV1Test(raw)); ipcMain.handle('models:v1:list', (_e, raw: unknown) => handleModelsV1List(raw)); diff --git a/apps/desktop/src/main/ipc/generate.ts b/apps/desktop/src/main/ipc/generate.ts index 1a3b2a6c..76834641 100644 --- a/apps/desktop/src/main/ipc/generate.ts +++ b/apps/desktop/src/main/ipc/generate.ts @@ -50,6 +50,7 @@ import { listSessionChatMessages, type SessionChatStoreOptions } from '../sessio import { type Database, getDesign, recordDiagnosticEvent } from '../snapshots-db'; import { readWorkspaceFilesAt } from '../workspace-reader'; import { allocateAssetPath, createRuntimeTextEditorFs, resolveLocalAssetRefs } from './runtime-fs'; +import { toolExecutionIsErrorForLog } from './tool-log'; /** * Pull an HTTP status code out of a caught provider error. Mirrors @@ -334,7 +335,7 @@ export function registerGenerateIpc({ db, getMainWindow }: RegisterGenerateIpcDe logIpc.info('agent.tool_end', { generationId: id, tool: event.toolName, - isError: event.toolName === 'set_todos' ? false : event.isError, + isError: toolExecutionIsErrorForLog(event), }); } else if (event.type === 'turn_end') { logIpc.info('agent.turn_end', { diff --git a/apps/desktop/src/main/ipc/tool-log.test.ts b/apps/desktop/src/main/ipc/tool-log.test.ts new file mode 100644 index 00000000..290f13f0 --- /dev/null +++ b/apps/desktop/src/main/ipc/tool-log.test.ts @@ -0,0 +1,56 @@ +import type { AgentEvent } from '@open-codesign/core'; +import { describe, expect, it } from 'vitest'; +import { toolExecutionIsErrorForLog } from './tool-log'; + +type ToolExecutionEndEvent = Extract; + +function toolEnd(overrides: Partial): ToolExecutionEndEvent { + return { + type: 'tool_execution_end', + toolCallId: 'tool-1', + toolName: 'set_todos', + isError: true, + result: { + content: [{ type: 'text', text: '[ ] Check work' }], + details: { items: [{ text: 'Check work', checked: false }] }, + }, + ...overrides, + } as ToolExecutionEndEvent; +} + +describe('toolExecutionIsErrorForLog', () => { + it('suppresses only the known successful set_todos false-positive shape', () => { + expect(toolExecutionIsErrorForLog(toolEnd({}))).toBe(false); + }); + + it('preserves genuine set_todos errors when the result includes an error signal', () => { + expect( + toolExecutionIsErrorForLog( + toolEnd({ + result: { + content: [{ type: 'text', text: '[ ] Check work' }], + details: { items: [{ text: 'Check work', checked: false }] }, + errorMessage: 'Failed to persist todos', + }, + }), + ), + ).toBe(true); + }); + + it('preserves set_todos errors when the result shape is not the tool success payload', () => { + expect( + toolExecutionIsErrorForLog( + toolEnd({ + result: { + content: [{ type: 'text', text: '[ ] Check work' }], + details: { items: [{ text: 'Check work', checked: 'no' }] }, + }, + }), + ), + ).toBe(true); + }); + + it('leaves non-set_todos tool errors untouched', () => { + expect(toolExecutionIsErrorForLog(toolEnd({ toolName: 'read' }))).toBe(true); + }); +}); diff --git a/apps/desktop/src/main/ipc/tool-log.ts b/apps/desktop/src/main/ipc/tool-log.ts new file mode 100644 index 00000000..4643df25 --- /dev/null +++ b/apps/desktop/src/main/ipc/tool-log.ts @@ -0,0 +1,44 @@ +import type { AgentEvent } from '@open-codesign/core'; + +type ToolExecutionEndEvent = Extract; + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +function isSetTodosItem(value: unknown): boolean { + return ( + isRecord(value) && + Object.keys(value).every((key) => key === 'text' || key === 'checked') && + typeof value['text'] === 'string' && + typeof value['checked'] === 'boolean' + ); +} + +function isSetTodosTextContent(value: unknown): boolean { + return ( + isRecord(value) && + Object.keys(value).every((key) => key === 'type' || key === 'text') && + value['type'] === 'text' && + typeof value['text'] === 'string' + ); +} + +function isSuccessfulSetTodosResult(result: unknown): boolean { + if (!isRecord(result)) return false; + if (!Object.keys(result).every((key) => key === 'content' || key === 'details')) return false; + + const details = result['details']; + if (!isRecord(details)) return false; + if (!Object.keys(details).every((key) => key === 'items')) return false; + const items = details['items']; + if (!Array.isArray(items) || !items.every(isSetTodosItem)) return false; + + const content = result['content']; + return Array.isArray(content) && content.length > 0 && content.every(isSetTodosTextContent); +} + +export function toolExecutionIsErrorForLog(event: ToolExecutionEndEvent): boolean { + if (event.toolName !== 'set_todos' || !event.isError) return event.isError; + return !isSuccessfulSetTodosResult(event.result); +} diff --git a/apps/desktop/src/main/snapshots-ipc.ts b/apps/desktop/src/main/snapshots-ipc.ts index 22096320..6c4c9468 100644 --- a/apps/desktop/src/main/snapshots-ipc.ts +++ b/apps/desktop/src/main/snapshots-ipc.ts @@ -763,7 +763,10 @@ export function registerWorkspaceIpc(db: Database, getWin: () => BrowserWindow | if (design === null) { throw new CodesignError('Design not found', 'IPC_NOT_FOUND'); } - if (design.workspacePath === null) return []; + if (design.workspacePath === null) { + logger.warn('files.list.workspace_missing', { designId: design.id }); + return []; + } const workspacePath = requireBoundWorkspacePath(design, 'Design is not bound to a workspace'); try { return await listWorkspaceFilesAt(workspacePath); @@ -847,6 +850,9 @@ export function registerWorkspaceIpc(db: Database, getWin: () => BrowserWindow | if (design === null) { throw new CodesignError('Design not found', 'IPC_NOT_FOUND'); } + if (design.workspacePath === null) { + throw new CodesignError('Design is not bound to a workspace', 'IPC_BAD_INPUT'); + } const workspacePath = requireBoundWorkspacePath(design, 'Design is not bound to a workspace'); let destinationPath: string; diff --git a/apps/desktop/src/main/snapshots-ipc.workspace-files.test.ts b/apps/desktop/src/main/snapshots-ipc.workspace-files.test.ts new file mode 100644 index 00000000..f09db529 --- /dev/null +++ b/apps/desktop/src/main/snapshots-ipc.workspace-files.test.ts @@ -0,0 +1,86 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { createDesign, initInMemoryDb } from './snapshots-db'; +import { registerWorkspaceIpc } from './snapshots-ipc'; + +type Handler = (event: unknown, raw: unknown) => unknown; + +const handlers = vi.hoisted(() => new Map()); + +vi.mock('./electron-runtime', () => ({ + app: { + getPath: vi.fn(() => '/tmp/open-codesign-tests'), + }, + dialog: { + showOpenDialog: vi.fn(), + }, + ipcMain: { + handle: vi.fn((channel: string, handler: Handler) => { + handlers.set(channel, handler); + }), + }, +})); + +vi.mock('./logger', () => ({ + getLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})); + +function getHandler(channel: string): Handler { + const handler = handlers.get(channel); + if (!handler) throw new Error(`Missing IPC handler: ${channel}`); + return handler; +} + +describe('workspace files IPC legacy workspace fallback', () => { + beforeEach(() => { + handlers.clear(); + vi.clearAllMocks(); + }); + + it('returns an empty file list when a legacy design has no workspace path', async () => { + const db = initInMemoryDb(); + const design = createDesign(db, 'Legacy unbound design'); + registerWorkspaceIpc(db, () => null); + + const list = getHandler('codesign:files:v1:list'); + + await expect(list(null, { schemaVersion: 1, designId: design.id })).resolves.toEqual([]); + }); + + it('returns an empty typed file result when a legacy design has no workspace path', async () => { + const db = initInMemoryDb(); + const design = createDesign(db, 'Legacy unbound design'); + registerWorkspaceIpc(db, () => null); + + const read = getHandler('codesign:files:v1:read'); + + await expect( + read(null, { schemaVersion: 1, designId: design.id, path: 'src/App.jsx' }), + ).resolves.toEqual({ + path: 'src/App.jsx', + kind: 'jsx', + size: 0, + updatedAt: new Date(0).toISOString(), + content: '', + }); + }); + + it('rejects file writes when a legacy design has no workspace path', async () => { + const db = initInMemoryDb(); + const design = createDesign(db, 'Legacy unbound design'); + registerWorkspaceIpc(db, () => null); + + const write = getHandler('codesign:files:v1:write'); + + await expect( + write(null, { + schemaVersion: 1, + designId: design.id, + path: 'src/App.jsx', + content: 'function App() { return
; }', + }), + ).rejects.toMatchObject({ + name: 'CodesignError', + code: 'IPC_BAD_INPUT', + }); + }); +}); diff --git a/apps/desktop/src/renderer/src/preview/workspace-source.test.ts b/apps/desktop/src/renderer/src/preview/workspace-source.test.ts index 8ff1a5a6..b64610f0 100644 --- a/apps/desktop/src/renderer/src/preview/workspace-source.test.ts +++ b/apps/desktop/src/renderer/src/preview/workspace-source.test.ts @@ -101,4 +101,20 @@ describe('workspace preview source resolution', () => { }), ).resolves.toEqual({ path: 'index.html', content: source }); }); + + it('falls back to original source when referenced workspace read throws', async () => { + const source = ''; + const read = vi.fn(async () => { + throw new Error('files API unavailable'); + }); + + await expect( + resolveWorkspacePreviewSource({ + designId: 'd1', + source, + read, + requireReferencedSource: false, + }), + ).resolves.toEqual({ path: 'index.html', content: source }); + }); }); diff --git a/apps/desktop/src/renderer/src/preview/workspace-source.ts b/apps/desktop/src/renderer/src/preview/workspace-source.ts index c8823f38..d096b79a 100644 --- a/apps/desktop/src/renderer/src/preview/workspace-source.ts +++ b/apps/desktop/src/renderer/src/preview/workspace-source.ts @@ -70,8 +70,14 @@ export async function resolveWorkspacePreviewSource(input: { referenced = await input.read(input.designId, referencedPath); } catch (err) { if (input.requireReferencedSource) throw err; + console.warn('Failed to read referenced preview source; falling back to original.', err); + return { content: input.source, path }; + } + if (referenced.content.trim().length === 0) { + console.warn('Referenced preview source is empty; falling back to original.', { + path: referenced.path, + }); return { content: input.source, path }; } - if (referenced.content.trim().length === 0) return { content: input.source, path }; return { content: referenced.content, path: referenced.path }; } diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index 7461834a..234fe9a5 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1571,7 +1571,7 @@ describe('loadFrameTemplates — device frame starter assets', () => { rmSync(linkPath, { force: true }); if (existsSync(linkPath)) unlinkSync(linkPath); try { - symlinkSync(path.join(outside, 'secret.jsx'), linkPath, 'file'); + symlinkSync(path.join(outside, 'secret.jsx'), linkPath); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'EPERM') return; throw err; diff --git a/packages/core/src/design-skills/index.test.ts b/packages/core/src/design-skills/index.test.ts index 0758b8af..391bf557 100644 --- a/packages/core/src/design-skills/index.test.ts +++ b/packages/core/src/design-skills/index.test.ts @@ -34,6 +34,10 @@ describe.sequential('loadDesignSkills', () => { await expect(loadDesignSkills(dir)).resolves.toEqual([]); }); + it('returns an explicit empty state when the directory has no skill files', async () => { + await expect(loadDesignSkills(dir)).resolves.toEqual([]); + }); + it('throws when a declared skill file is missing from an existing directory', async () => { for (const name of DESIGN_SKILL_FILES.slice(0, 3)) { writeFileSync(path.join(dir, name), 'body', 'utf8'); @@ -56,7 +60,7 @@ describe.sequential('loadDesignSkills', () => { if (existsSync(linkPath)) unlinkSync(linkPath); try { try { - symlinkSync(path.join(outside, 'secret.jsx'), linkPath, 'file'); + symlinkSync(path.join(outside, 'secret.jsx'), linkPath); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'EPERM') return; throw err; diff --git a/packages/core/src/tools/done.test.ts b/packages/core/src/tools/done.test.ts index e9425b64..08445b7b 100644 --- a/packages/core/src/tools/done.test.ts +++ b/packages/core/src/tools/done.test.ts @@ -29,6 +29,11 @@ function makeFs(initial: Record = {}): TextEditorFsCallbacks { } describe('done tool', () => { + it('documents unresolved-error warnings for artifact finalization', () => { + const tool = makeDoneTool(makeFs()); + expect(tool.description).toContain('surface warnings to the user'); + }); + it('returns ok when index.html parses cleanly', async () => { const fs = makeFs({ 'index.html': diff --git a/packages/core/src/tools/done.ts b/packages/core/src/tools/done.ts index d62d1455..767e5781 100644 --- a/packages/core/src/tools/done.ts +++ b/packages/core/src/tools/done.ts @@ -307,7 +307,9 @@ export function makeDoneTool( 'console errors / load failures, then replies with ' + '`{ status: "ok" | "has_errors", errors: [...] }`. If errors come back, ' + 'you MUST fix them with str_replace_based_edit_tool and call `done` again. ' + - 'Stop calling once status is "ok" or after 5 rounds.', + 'Stop calling once status is "ok" or after 5 rounds. If errors still ' + + 'remain with a valid artifact after those repair rounds, the host may ' + + 'keep the latest artifact but will surface warnings to the user.', parameters: DoneParams, async execute(_id, params): Promise> { const path = params.path ?? 'index.html'; diff --git a/packages/exporters/src/pdf.test.ts b/packages/exporters/src/pdf.test.ts index 2763df89..42b3790c 100644 --- a/packages/exporters/src/pdf.test.ts +++ b/packages/exporters/src/pdf.test.ts @@ -4,7 +4,7 @@ import { join } from 'node:path'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; const fakePdfBytes = Buffer.from('%PDF-1.4 fake'); -const CHROME_TEST_TIMEOUT_MS = process.platform === 'win32' ? 30_000 : 10_000; +const CHROME_TEST_TIMEOUT_MS = process.env['CI'] || process.platform === 'win32' ? 30_000 : 15_000; const launchMock = vi.fn(); const newPageMock = vi.fn(); diff --git a/packages/providers/src/codex/token-store.test.ts b/packages/providers/src/codex/token-store.test.ts index 56f83111..76bba178 100644 --- a/packages/providers/src/codex/token-store.test.ts +++ b/packages/providers/src/codex/token-store.test.ts @@ -379,10 +379,8 @@ describe('CodexTokenStore', () => { const authA = baseAuth({ accessToken: 'concurrent-A' }); const authB = baseAuth({ accessToken: 'concurrent-B' }); - // Fire both writes without awaiting in between. Before the fix these - // would race on the same `${path}.tmp.${pid}` and one could unlink or - // overwrite the other's tmp, potentially leaving the target file - // missing or corrupted. + // Fire both writes without awaiting in between. The store should serialize + // final-path replacement even though each write gets its own tmp file. await Promise.all([store.write(authA), store.write(authB)]); const persisted = JSON.parse(await readFile(filePath, 'utf8')) as StoredCodexAuth; diff --git a/packages/providers/src/codex/token-store.ts b/packages/providers/src/codex/token-store.ts index 349ce2ac..c1a4cd6a 100644 --- a/packages/providers/src/codex/token-store.ts +++ b/packages/providers/src/codex/token-store.ts @@ -87,6 +87,7 @@ export class CodexTokenStore { private readonly now: () => number; private cache: StoredCodexAuth | null = null; private refreshPromise: Promise | null = null; + private writeQueue: Promise = Promise.resolve(); constructor(opts: CodexTokenStoreOptions) { this.filePath = opts.filePath; @@ -122,14 +123,22 @@ export class CodexTokenStore { async write(auth: StoredCodexAuth): Promise { assertStoredCodexAuth(auth, `Invalid Codex token store write at ${this.filePath}`); + const nextWrite = this.writeQueue.then(() => this.writeAtomic(auth)); + this.writeQueue = nextWrite.catch(() => {}); + await nextWrite; + } + + private async writeAtomic(auth: StoredCodexAuth): Promise { await mkdir(dirname(this.filePath), { recursive: true, mode: 0o700 }); const body = JSON.stringify(auth, null, 2); // Write to a pid + UUID scoped tmp then atomically rename. The UUID // suffix prevents intra-process races when two write() calls overlap - // (same pid would otherwise collide on the tmp path and could unlink - // or rename each other's file). rename() itself is atomic on POSIX and - // Windows (Node >= 10). Guards against truncated writes on Win11 when - // the process is killed or antivirus interferes mid-write (issue #128). + // (same pid would otherwise collide on the tmp path). writeQueue keeps + // final-path replacements from overlapping on Windows, where concurrent + // rename() calls can intermittently fail with EPERM. rename() itself is + // atomic on POSIX and Windows (Node >= 10). Guards against truncated + // writes on Win11 when the process is killed or antivirus interferes + // mid-write (issue #128). const tmpPath = `${this.filePath}.tmp.${process.pid}.${randomUUID()}`; try { await writeFile(tmpPath, body, { encoding: 'utf8', mode: 0o600 }); diff --git a/packages/providers/src/retry.test.ts b/packages/providers/src/retry.test.ts index 25c04672..b13fc341 100644 --- a/packages/providers/src/retry.test.ts +++ b/packages/providers/src/retry.test.ts @@ -108,6 +108,11 @@ describe('classifyError', () => { it('recognizes provider-side aborted transport messages for agent replay only', () => { expect(isProviderAbortedTransportError('Request was aborted')).toBe(true); expect(isProviderAbortedTransportError('Generation aborted by provider')).toBe(true); + expect(isProviderAbortedTransportError('fetch failed: aborted')).toBe(true); + expect(isProviderAbortedTransportError('ReadTimeout while waiting for upstream')).toBe(true); + expect(isProviderAbortedTransportError('Connection reset by upstream')).toBe(true); + expect(isProviderAbortedTransportError('The user aborted a request')).toBe(false); + expect(isProviderAbortedTransportError('AbortError: The user aborted a request')).toBe(false); expect(isProviderAbortedTransportError('Generation aborted by user')).toBe(false); }); it('does not retry regular errors without transport indicators', () => { diff --git a/packages/providers/src/retry.ts b/packages/providers/src/retry.ts index 1225a667..37c37c36 100644 --- a/packages/providers/src/retry.ts +++ b/packages/providers/src/retry.ts @@ -87,7 +87,7 @@ function classifyByStatus(status: number, err: unknown, wire?: WireApi): RetryDe const TRANSPORT_ERROR_RE = /(?:fetch\s+failed.*\bterminated\b|\bterminated\b|premature\s+close|stream\s+(?:ended|closed)|ECONNRESET)\b/i; const PROVIDER_ABORTED_TRANSPORT_RE = - /(?:request\s+was\s+aborted|generation\s+aborted\s+by\s+provider|provider\s+aborted|upstream\s+aborted)\b/i; + /(?:fetch\s+failed.*\baborted\b|request\s+was\s+aborted|generation\s+aborted\s+by\s+provider|provider\s+aborted|upstream\s+aborted|read\s*timeout|connection\s+reset|socket\s+hang\s+up)\b/i; export function isTransportLevelError(errorMessage: string | undefined): boolean { if (!errorMessage) return false;