Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/layout-checks-in-done-and-preview.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@open-codesign/core": patch
"@open-codesign/desktop": patch
---

Check layout and accessibility in `preview` and `done`. After a page loads, a deterministic measurement looks for a page that scrolls sideways, text that its box clips, text with too little contrast (WCAG AA, 4.5:1, or 3:1 for large text), tap targets under 24 by 24 px that have another target next to them, and controls with no accessible name. It measures at the current width and again at a 390 px phone width. `preview` lists up to 20 findings in `layoutIssues`; `done` shows them under "Non-blocking warnings", and when one is present it lets the agent fix it and check it with `preview` instead of telling it to stop. Findings never change `ok` or the `done` status, so a false positive cannot cost a repair round. `done:verify:v1` now returns `warnings` and a `schemaVersion` next to `errors`.
17 changes: 13 additions & 4 deletions apps/desktop/src/main/done-verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ import { mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join, resolve } from 'node:path';
import { fileURLToPath, pathToFileURL, URL } from 'node:url';
import type { DoneError, DoneRuntimeVerifier } from '@open-codesign/core';
import type { DoneError, DoneRuntimeResult, DoneRuntimeVerifier } from '@open-codesign/core';
import { findSystemChrome } from '@open-codesign/exporters';
import { buildSrcdoc } from '@open-codesign/runtime';
import type { Browser, ConsoleMessage, HTTPRequest, Page } from 'puppeteer-core';
import { inspectLayout, layoutFailureWarning, layoutWarnings } from './layout-probe';
import { boundedPreview } from './preview-interactions';
import {
buildWorkspacePreviewDocument,
Expand Down Expand Up @@ -181,12 +182,13 @@ async function verifyWithSystemChrome(
html: string,
workspaceRoot?: string,
signal?: AbortSignal,
): Promise<DoneError[]> {
): Promise<DoneRuntimeResult> {
signal?.throwIfAborted();
const executablePath = await findSystemChrome();
const puppeteer = (await import('puppeteer-core')).default;
const userDataDir = await mkdtemp(join(tmpdir(), 'codesign-done-chrome-'));
const errors: DoneError[] = [];
const warnings: DoneError[] = [];
const seen = new Set<string>();
let browser: Browser | null = null;
let page: Page | null = null;
Expand Down Expand Up @@ -245,6 +247,13 @@ async function verifyWithSystemChrome(
signal,
);
await boundedPreview(settleAfterLoad(), SETTLE_AFTER_LOAD_MS + 100, signal);
// Layout findings are advice: a failed measurement is reported, never turned into an error.
try {
warnings.push(...layoutWarnings(await inspectLayout(page, signal)));
} catch (err) {
signal?.throwIfAborted();
warnings.push(layoutFailureWarning(err));
}
} catch (err) {
signal?.throwIfAborted();
pushUniqueError(
Expand All @@ -259,11 +268,11 @@ async function verifyWithSystemChrome(
await rm(userDataDir, { recursive: true, force: true });
}

return errors;
return { errors, warnings };
}

export function makeRuntimeVerifier(options?: { workspaceRoot: string }): DoneRuntimeVerifier {
return async (artifactSource, context): Promise<DoneError[]> => {
return async (artifactSource, context): Promise<DoneRuntimeResult> => {
context?.signal?.throwIfAborted();
const workspaceRoot = options ? resolve(options.workspaceRoot) : undefined;
const srcdoc = workspaceRoot
Expand Down
9 changes: 6 additions & 3 deletions apps/desktop/src/main/done-verify.workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ describe.skipIf(!chromeAvailable)('done workspace assets in system Chrome', () =
const prefix = path.startsWith('screens/') ? '../' : '';
const source = sourceFor(`${prefix}logo.svg`, `${prefix}art/poster.svg`);
await writeFile(join(workspace, path), source);
expect(await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { path })).toEqual([]);
expect(await makeRuntimeVerifier({ workspaceRoot: workspace })(source, { path })).toEqual({
errors: [],
warnings: [],
});
const preview = await runPreview({ workspaceRoot: workspace, path, vision: false });
expect(preview.ok).toBe(true);
expect(preview.assetErrors).toEqual([]);
Expand All @@ -49,7 +52,7 @@ describe.skipIf(!chromeAvailable)('done workspace assets in system Chrome', () =
it('reports a missing relative asset with its URL', async () => {
const source = sourceFor('logo.svg', 'missing.svg');
await writeFile(join(workspace, 'App.jsx'), source);
const errors = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, {
const { errors } = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, {
path: 'App.jsx',
});
expect(errors.some((error) => error.message.includes('missing.svg'))).toBe(true);
Expand All @@ -60,7 +63,7 @@ describe.skipIf(!chromeAvailable)('done workspace assets in system Chrome', () =
await writeFile(outside, svg);
const source = sourceFor('logo.svg', pathToFileURL(outside).href);
await writeFile(join(workspace, 'App.jsx'), source);
const errors = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, {
const { errors } = await makeRuntimeVerifier({ workspaceRoot: workspace })(source, {
path: 'App.jsx',
});
expect(
Expand Down
75 changes: 41 additions & 34 deletions apps/desktop/src/main/harness-document.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,39 +21,46 @@ const browser = vi.hoisted(() => ({

vi.mock('@open-codesign/exporters', () => ({ findSystemChrome: async () => 'synthetic-chrome' }));
vi.mock('./logger', () => ({ getLogger: () => ({ warn: vi.fn(), error: vi.fn() }) }));
vi.mock('puppeteer-core', () => ({
default: {
launch: async () => {
const handlers = new Map<string, (arg: unknown) => void>();
const page = {
setViewport: async () => {},
setRequestInterception: async () => {},
evaluateOnNewDocument: async () => {},
on: (event: string, handler: (arg: unknown) => void) => handlers.set(event, handler),
goto: async (url: string) => {
const request: FakeRequest = {
url: () => url,
isNavigationRequest: () => true,
resourceType: () => 'document',
respond: vi.fn(async () => {}),
continue: vi.fn(async () => {}),
abort: vi.fn(async () => {}),
};
browser.navigations.push({
url,
existedOnDisk: existsSync(fileURLToPath(url)),
request,
});
handlers.get('request')?.(request);
await vi.waitFor(() => expect(request.respond).toHaveBeenCalled());
},
evaluate: async () => ({ nodes: 1, width: 1280, height: 800 }),
close: async () => {},
};
return { newPage: async () => page, close: async () => {} };
vi.mock('puppeteer-core', async () => {
const { samples } = await import('./layout-test-fixtures');
return {
default: {
launch: async () => {
const handlers = new Map<string, (arg: unknown) => void>();
const page = {
viewport: () => ({ width: 1280, height: 800 }),
setViewport: async () => {},
setRequestInterception: async () => {},
evaluateOnNewDocument: async () => {},
on: (event: string, handler: (arg: unknown) => void) => handlers.set(event, handler),
goto: async (url: string) => {
const request: FakeRequest = {
url: () => url,
isNavigationRequest: () => true,
resourceType: () => 'document',
respond: vi.fn(async () => {}),
continue: vi.fn(async () => {}),
abort: vi.fn(async () => {}),
};
browser.navigations.push({
url,
existedOnDisk: existsSync(fileURLToPath(url)),
request,
});
handlers.get('request')?.(request);
await vi.waitFor(() => expect(request.respond).toHaveBeenCalled());
},
evaluate: async (fn: { name: string }) =>
fn.name === 'collectLayoutSamples'
? samples({ viewportWidth: 1280, documentWidth: 1280 })
: { nodes: 1, width: 1280, height: 800 },
close: async () => {},
};
return { newPage: async () => page, close: async () => {} };
},
},
},
}));
};
});

const workspace = mkdtempSync(join(tmpdir(), 'codesign-harness-doc-'));

Expand All @@ -74,12 +81,12 @@ function onlyNavigation() {

describe('harness documents for sandboxed system Chrome (#455)', () => {
it('serves the done verifier document from memory instead of a shared temp file', async () => {
const errors = await makeRuntimeVerifier()('<main id="verify-marker">ok</main>', {
const result = await makeRuntimeVerifier()('<main id="verify-marker">ok</main>', {
path: 'index.html',
});

const navigation = onlyNavigation();
expect(errors).toEqual([]);
expect(result).toEqual({ errors: [], warnings: [] });
expect(navigation.url).toMatch(/^file:.*verify\.html$/);
expect(navigation.existedOnDisk).toBe(false);
expect(navigation.request.continue).not.toHaveBeenCalled();
Expand Down
6 changes: 4 additions & 2 deletions apps/desktop/src/main/ipc/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1093,8 +1093,10 @@ export function registerGenerateIpc({ db, getMainWindow }: RegisterGenerateIpcDe
) {
throw new CodesignError('done:verify:v1 expects { artifact: string }', 'IPC_BAD_INPUT');
}
const errors = await sharedRuntimeVerifier((raw as { artifact: string }).artifact);
return { errors };
const { errors, warnings } = await sharedRuntimeVerifier(
(raw as { artifact: string }).artifact,
);
return { schemaVersion: 1, errors, warnings };
});

ipcMain.handle('codesign:v1:generate', async (_e, raw: unknown) => {
Expand Down
22 changes: 21 additions & 1 deletion apps/desktop/src/main/ipc/generate.workspace-rename.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ vi.mock('../memory-ipc', () => ({
}));

vi.mock('../done-verify', () => ({
makeRuntimeVerifier: vi.fn(() => async () => []),
makeRuntimeVerifier: vi.fn(() => async () => ({ errors: [], warnings: [] })),
}));

vi.mock('../preview-runtime', () => ({
Expand Down Expand Up @@ -420,6 +420,26 @@ describe('generate IPC workspace rename coordination', () => {
expect(vi.mocked(generateViaAgent)).toHaveBeenCalledOnce();
});

it('returns verifier errors and warnings from done:verify:v1 with a schema version', async () => {
const error = { message: 'ReferenceError: x is not defined', source: 'pageerror' };
const warning = {
message: 'The page is 132 px wider than a 390 px viewport',
source: 'layout',
};
vi.mocked(makeRuntimeVerifier).mockReturnValueOnce(async () => ({
errors: [error],
warnings: [warning],
}));
registerGenerateIpc({ db: initTestDb(), getMainWindow: () => null });

await expect(
getHandler('done:verify:v1')(null, { artifact: 'function App(){return null;}' }),
).resolves.toEqual({ schemaVersion: 1, errors: [error], warnings: [warning] });
await expect(getHandler('done:verify:v1')(null, { artifact: 42 })).rejects.toThrow(
'done:verify:v1 expects { artifact: string }',
);
});

it('persists preparation failures and validates recovery cursors', async () => {
vi.mocked(preparePromptContext).mockRejectedValueOnce(new Error('Reference unavailable'));
const db = initTestDb();
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/main/ipc/tool-log.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('done diagnostic durability', () => {

it('keeps warnings visible without changing successful completion to failure', () => {
const result = {
content: [{ type: 'text', text: 'ok\nNon-blocking design metadata warnings' }],
content: [{ type: 'text', text: 'ok\nNon-blocking warnings' }],
details: {
status: 'ok',
path: 'App.jsx',
Expand Down
Loading
Loading