fix: resolve 3 protocol bugs, improve CLI and snapshot code quality (#487)
## Summary - Fix `allowFileAccess` being silently stripped from launch commands by adding it to the Zod schema in `protocol.ts` (the `--allow-file-access` CLI flag was not reaching the browser) - Fix `trace stop` requiring a path argument despite help text documenting it as optional -- now works with or without a path - Fix `addscript`/`addstyle` silently succeeding when neither `content` nor `url` is provided -- now returns a validation error - Replace hardcoded ANSI escape code with `color::error_indicator()` in `main.rs` to respect `NO_COLOR` - Fix double-parse pattern and add descriptive expect messages in `commands.rs` - Fix incomplete string escaping in `snapshot.ts` `buildSelector` (use `JSON.stringify` instead of manual quote escaping) - Simplify redundant ternary in `snapshot.ts` cursor-interactive role assignment - Sync docs changelog with CHANGELOG.md (v0.8.1 through v0.10.0)
This commit is contained in:
+4
-1
@@ -1442,7 +1442,10 @@ async function handleTraceStop(
|
||||
browser: BrowserManager
|
||||
): Promise<Response> {
|
||||
await browser.stopTracing(command.path);
|
||||
return successResponse(command.id, { path: command.path });
|
||||
return successResponse(
|
||||
command.id,
|
||||
command.path ? { path: command.path } : { traceStopped: true }
|
||||
);
|
||||
}
|
||||
|
||||
async function handleHarStart(
|
||||
|
||||
+2
-2
@@ -678,10 +678,10 @@ export class BrowserManager {
|
||||
/**
|
||||
* Stop tracing and save
|
||||
*/
|
||||
async stopTracing(path: string): Promise<void> {
|
||||
async stopTracing(path?: string): Promise<void> {
|
||||
const context = this.contexts[0];
|
||||
if (context) {
|
||||
await context.tracing.stop({ path });
|
||||
await context.tracing.stop(path ? { path } : undefined);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -582,6 +582,22 @@ describe('parseCommand', () => {
|
||||
const result = parseCommand(cmd({ id: '1', action: 'launch', ignoreHTTPSErrors: 'true' }));
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it('should parse launch with allowFileAccess true', () => {
|
||||
const result = parseCommand(cmd({ id: '1', action: 'launch', allowFileAccess: true }));
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.command.allowFileAccess).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('should parse launch with allowFileAccess false', () => {
|
||||
const result = parseCommand(cmd({ id: '1', action: 'launch', allowFileAccess: false }));
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.command.allowFileAccess).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('mouse actions', () => {
|
||||
@@ -1108,6 +1124,46 @@ describe('parseCommand', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('addscript and addstyle', () => {
|
||||
it('should parse addscript with content', () => {
|
||||
const result = parseCommand(
|
||||
cmd({ id: '1', action: 'addscript', content: 'console.log("hi")' })
|
||||
);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should parse addscript with url', () => {
|
||||
const result = parseCommand(
|
||||
cmd({ id: '1', action: 'addscript', url: 'https://example.com/script.js' })
|
||||
);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject addscript with neither content nor url', () => {
|
||||
const result = parseCommand(cmd({ id: '1', action: 'addscript' }));
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it('should parse addstyle with content', () => {
|
||||
const result = parseCommand(
|
||||
cmd({ id: '1', action: 'addstyle', content: 'body { color: red }' })
|
||||
);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should parse addstyle with url', () => {
|
||||
const result = parseCommand(
|
||||
cmd({ id: '1', action: 'addstyle', url: 'https://example.com/style.css' })
|
||||
);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject addstyle with neither content nor url', () => {
|
||||
const result = parseCommand(cmd({ id: '1', action: 'addstyle' }));
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('invalid commands', () => {
|
||||
it('should reject unknown action', () => {
|
||||
const result = parseCommand(cmd({ id: '1', action: 'unknown' }));
|
||||
|
||||
+14
-2
@@ -47,6 +47,7 @@ const launchSchema = baseCommandSchema.extend({
|
||||
userAgent: z.string().optional(),
|
||||
provider: z.string().optional(),
|
||||
ignoreHTTPSErrors: z.boolean().optional(),
|
||||
allowFileAccess: z.boolean().optional(),
|
||||
profile: z.string().optional(),
|
||||
storageState: z.string().optional(),
|
||||
});
|
||||
@@ -369,7 +370,7 @@ const traceStartSchema = baseCommandSchema.extend({
|
||||
|
||||
const traceStopSchema = baseCommandSchema.extend({
|
||||
action: z.literal('trace_stop'),
|
||||
path: z.string().min(1),
|
||||
path: z.string().min(1).optional(),
|
||||
});
|
||||
|
||||
const harStartSchema = baseCommandSchema.extend({
|
||||
@@ -989,7 +990,18 @@ export function parseCommand(input: string): ParseResult {
|
||||
return { success: false, error: `Validation error: ${errors}`, id };
|
||||
}
|
||||
|
||||
return { success: true, command: result.data as Command };
|
||||
const command = result.data as Command;
|
||||
|
||||
// Post-parse validation for commands that need cross-field checks
|
||||
if (
|
||||
(command.action === 'addscript' || command.action === 'addstyle') &&
|
||||
!command.content &&
|
||||
!command.url
|
||||
) {
|
||||
return { success: false, error: 'Either content or url must be provided', id };
|
||||
}
|
||||
|
||||
return { success: true, command };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+3
-3
@@ -132,8 +132,8 @@ const STRUCTURAL_ROLES = new Set([
|
||||
*/
|
||||
function buildSelector(role: string, name?: string): string {
|
||||
if (name) {
|
||||
const escapedName = name.replace(/"/g, '\\"');
|
||||
return `getByRole('${role}', { name: "${escapedName}", exact: true })`;
|
||||
const escapedName = JSON.stringify(name);
|
||||
return `getByRole('${role}', { name: ${escapedName}, exact: true })`;
|
||||
}
|
||||
return `getByRole('${role}')`;
|
||||
}
|
||||
@@ -307,7 +307,7 @@ export async function getEnhancedSnapshot(
|
||||
existingTexts.add(elTextLower);
|
||||
|
||||
const ref = nextRef();
|
||||
const role = el.hasCursorPointer ? 'clickable' : el.hasOnClick ? 'clickable' : 'focusable';
|
||||
const role = el.hasCursorPointer || el.hasOnClick ? 'clickable' : 'focusable';
|
||||
|
||||
refs[ref] = {
|
||||
selector: el.selector,
|
||||
|
||||
+1
-1
@@ -576,7 +576,7 @@ export interface TraceStartCommand extends BaseCommand {
|
||||
|
||||
export interface TraceStopCommand extends BaseCommand {
|
||||
action: 'trace_stop';
|
||||
path: string;
|
||||
path?: string;
|
||||
}
|
||||
|
||||
// HAR recording
|
||||
|
||||
Reference in New Issue
Block a user