mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
fix(core): Stop code editors mangling values typed by browser_type (no-changelog) (#36804)
This commit is contained in:
committed by
GitHub
parent
47adccc0bf
commit
9c4b439122
@@ -536,6 +536,40 @@ describe('slack fixture served to a real browser', () => {
|
||||
await page.close();
|
||||
});
|
||||
|
||||
it('mangles a manifest entered key by key, and Next refuses it', async () => {
|
||||
// The other half of the editor trap: auto-close fires per keystroke, so the
|
||||
// manifest's own closers pile up and the result is no longer JSON. Guards the
|
||||
// keydown handler — without it the box is a plain textarea and nothing traps.
|
||||
const page = await fx.ctx.newPage();
|
||||
await page.goto('https://api.slack.com/apps');
|
||||
await page.getByRole('button', { name: 'Create New App' }).click();
|
||||
await page.getByRole('button', { name: 'From a manifest' }).click();
|
||||
|
||||
const manifest = '{\n "display_information": {\n "name": "n8n"\n }\n}';
|
||||
await page.locator('#manifest-input').pressSequentially(manifest);
|
||||
expect(await page.locator('#manifest-input').inputValue()).not.toBe(manifest);
|
||||
|
||||
await page.getByRole('button', { name: 'Next' }).click();
|
||||
expect(await page.locator('#manifest-error').isVisible()).toBe(true);
|
||||
expect(new URL(page.url()).pathname).toBe('/apps');
|
||||
await page.close();
|
||||
});
|
||||
|
||||
it('accepts the same manifest inserted in one operation', async () => {
|
||||
const page = await fx.ctx.newPage();
|
||||
await page.goto('https://api.slack.com/apps');
|
||||
await page.getByRole('button', { name: 'Create New App' }).click();
|
||||
await page.getByRole('button', { name: 'From a manifest' }).click();
|
||||
|
||||
const manifest = '{\n "display_information": {\n "name": "n8n"\n }\n}';
|
||||
await page.locator('#manifest-input').fill(manifest);
|
||||
expect(await page.locator('#manifest-input').inputValue()).toBe(manifest);
|
||||
|
||||
await page.getByRole('button', { name: 'Next' }).click();
|
||||
await page.waitForURL(/\/apps\/A0\w+\/general/);
|
||||
await page.close();
|
||||
});
|
||||
|
||||
it('leaves Next enabled — nothing here is gated on a required field', async () => {
|
||||
const page = await fx.ctx.newPage();
|
||||
await page.goto('https://api.slack.com/apps');
|
||||
|
||||
@@ -14,6 +14,11 @@
|
||||
click on it times out while typing into it works. That is the real
|
||||
CodeMirror behaviour and the live reproduction for LangTracer case 599 —
|
||||
replacing it with a plain visible textarea deletes the trap
|
||||
- the editor auto-closes brackets ON KEYSTROKE. Entering the manifest one key
|
||||
at a time therefore leaves stray closers behind and "Next" rejects it; one
|
||||
atomic insert is the only way through. Dropping that handler deletes the
|
||||
trap. It keys off real key events, so a typer built on Input.insertText
|
||||
sails past it — that is the difference under test, not an oversight
|
||||
- "Next" is ENABLED throughout. The real run assumed it was disabled and spent
|
||||
three browser_evaluate scripts on that theory; it never was (not NODE-5755)
|
||||
-->
|
||||
@@ -118,6 +123,25 @@
|
||||
dialog.hidden = false;
|
||||
});
|
||||
|
||||
// Auto-close, as the real editor does. It fires on keydown only, so one
|
||||
// atomic insert is unaffected — that difference is the point.
|
||||
const PAIRS = { '{': '}', '[': ']', '"': '"' };
|
||||
|
||||
input.addEventListener('keydown', (event) => {
|
||||
if (event.metaKey || event.ctrlKey || event.altKey) return;
|
||||
if (!PAIRS[event.key]) return;
|
||||
|
||||
event.preventDefault();
|
||||
const start = input.selectionStart;
|
||||
input.value =
|
||||
input.value.slice(0, start) +
|
||||
event.key +
|
||||
PAIRS[event.key] +
|
||||
input.value.slice(input.selectionEnd);
|
||||
input.selectionStart = input.selectionEnd = start + 1;
|
||||
input.dispatchEvent(new Event('input', { bubbles: true }));
|
||||
});
|
||||
|
||||
// Mirror typed text into the rendered surface, as the editor does.
|
||||
input.addEventListener('input', () => {
|
||||
const lines = input.value.split('\n');
|
||||
|
||||
@@ -308,6 +308,13 @@ describe('AgentBrowserAdapter', () => {
|
||||
expect(getRunArgs(1)).toEqual(['press', 'Enter']);
|
||||
});
|
||||
|
||||
it('accepts paste mode but still types, having no atomic insert of its own yet', async () => {
|
||||
stubRun({ success: true });
|
||||
await adapter.type('t1', { ref: 'e1' }, '{"a": 1}', { mode: 'paste' });
|
||||
|
||||
expect(getRunArgs(0)).toEqual(['type', '@e1', '{"a": 1}']);
|
||||
});
|
||||
|
||||
it('splits a single leading dash into a separate type call', async () => {
|
||||
stubRun({ success: true }); // type '-'
|
||||
stubRun({ success: true }); // type '5'
|
||||
|
||||
@@ -357,6 +357,11 @@ export class AgentBrowserAdapter implements Adapter {
|
||||
const ref = this.resolveTarget(target);
|
||||
const baseCmd = options?.clear ? 'fill' : 'type';
|
||||
|
||||
// TODO: `mode: 'paste'` is accepted but not honoured here — this adapter still
|
||||
// types key by key, so code editors are still corrupted. Implementing it needs an
|
||||
// atomic insert that replaces existing content (the CLI's `fill` does not, on a
|
||||
// contenteditable) and that reports failure (the `eval` channel does not).
|
||||
|
||||
// agent-browser's CLI scans all raw args for "--help"/"-h" before parsing,
|
||||
// so any arg starting with "-" would be misinterpreted as a flag.
|
||||
// Peel each leading "-" into a separate type call so no single arg starts with "-".
|
||||
|
||||
@@ -445,11 +445,16 @@ export class PlaywrightAdapter {
|
||||
const locator = await this.resolveLocator(pageId, target);
|
||||
await this.ensureActionable(locator, target, 'editable');
|
||||
|
||||
if (options?.clear) {
|
||||
await locator.clear();
|
||||
}
|
||||
if (options?.mode === 'paste') {
|
||||
// Code editors mangle key-by-key entry: auto-close and auto-indent fire per keystroke.
|
||||
await locator.fill(text);
|
||||
} else {
|
||||
if (options?.clear) {
|
||||
await locator.clear();
|
||||
}
|
||||
|
||||
await locator.pressSequentially(text, { delay: options?.delay });
|
||||
await locator.pressSequentially(text, { delay: options?.delay });
|
||||
}
|
||||
|
||||
if (options?.submit) {
|
||||
await locator.press('Enter');
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { adapterWithLocator } from './test-helpers';
|
||||
import { configureLogger } from '../logger';
|
||||
|
||||
configureLogger({ level: 'silent' });
|
||||
|
||||
function typeLocator() {
|
||||
return {
|
||||
count: vi.fn().mockResolvedValue(1),
|
||||
isEditable: vi.fn().mockResolvedValue(true),
|
||||
clear: vi.fn().mockResolvedValue(undefined),
|
||||
fill: vi.fn().mockResolvedValue(undefined),
|
||||
pressSequentially: vi.fn().mockResolvedValue(undefined),
|
||||
press: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
}
|
||||
|
||||
const MANIFEST = '{\n "display_information": {\n "name": "n8n"\n }\n}';
|
||||
|
||||
describe('PlaywrightAdapter.type', () => {
|
||||
describe('paste mode', () => {
|
||||
it('inserts the value in a single operation', async () => {
|
||||
const locator = typeLocator();
|
||||
const adapter = adapterWithLocator('p1', locator);
|
||||
|
||||
await adapter.type('p1', { ref: 'e3' }, MANIFEST, { mode: 'paste' });
|
||||
|
||||
expect(locator.fill).toHaveBeenCalledWith(MANIFEST);
|
||||
expect(locator.pressSequentially).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still submits when asked', async () => {
|
||||
const locator = typeLocator();
|
||||
const adapter = adapterWithLocator('p1', locator);
|
||||
|
||||
await adapter.type('p1', { ref: 'e3' }, MANIFEST, { mode: 'paste', submit: true });
|
||||
|
||||
expect(locator.press).toHaveBeenCalledWith('Enter');
|
||||
});
|
||||
|
||||
it('does not clear separately, because the insert already replaces', async () => {
|
||||
const locator = typeLocator();
|
||||
const adapter = adapterWithLocator('p1', locator);
|
||||
|
||||
await adapter.type('p1', { ref: 'e3' }, MANIFEST, { mode: 'paste', clear: true });
|
||||
|
||||
expect(locator.clear).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('type mode', () => {
|
||||
it('is the default, and still enters the value key by key', async () => {
|
||||
const locator = typeLocator();
|
||||
const adapter = adapterWithLocator('p1', locator);
|
||||
|
||||
await adapter.type('p1', { ref: 'e3' }, 'hello', { clear: true });
|
||||
|
||||
expect(locator.clear).toHaveBeenCalled();
|
||||
expect(locator.pressSequentially).toHaveBeenCalledWith('hello', { delay: undefined });
|
||||
expect(locator.fill).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -137,6 +137,18 @@ describe('createInteractionTools', () => {
|
||||
expect(() => getTool().inputSchema.parse({ text: 'hello' })).toThrow();
|
||||
});
|
||||
|
||||
it('accepts a paste mode for values that must land in one operation', () => {
|
||||
expect(() =>
|
||||
getTool().inputSchema.parse({ element: { ref: 'e1' }, text: '{}', mode: 'paste' }),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('rejects an unknown mode', () => {
|
||||
expect(() =>
|
||||
getTool().inputSchema.parse({ element: { ref: 'e1' }, text: '{}', mode: 'slowly' }),
|
||||
).toThrow();
|
||||
});
|
||||
|
||||
it('accepts optional clear, submit, delay', () => {
|
||||
expect(() =>
|
||||
getTool().inputSchema.parse({
|
||||
@@ -168,6 +180,20 @@ describe('createInteractionTools', () => {
|
||||
expect(data.ref).toBe('e1');
|
||||
});
|
||||
|
||||
it('passes the paste mode down to the adapter', async () => {
|
||||
await getTool().execute(
|
||||
{ element: { ref: 'e1' }, text: '{"a": 1}', mode: 'paste' },
|
||||
TOOL_CONTEXT,
|
||||
);
|
||||
|
||||
expect(mockConnection.adapter.type).toHaveBeenCalledWith(
|
||||
'page1',
|
||||
{ ref: 'e1' },
|
||||
'{"a": 1}',
|
||||
expect.objectContaining({ mode: 'paste' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('routes a failure through the connection so it can be explained', async () => {
|
||||
// The wrapper stays out of the error taxonomy; BrowserConnection owns it.
|
||||
const failure = new Error('locator.pressSequentially: Timeout 30000ms exceeded.');
|
||||
|
||||
@@ -81,6 +81,12 @@ const browserTypeSchema = z
|
||||
.object({
|
||||
element: elementTargetSchema.describe('Element to type into'),
|
||||
text: z.string().describe('Text to type'),
|
||||
mode: z
|
||||
.enum(['type', 'paste'])
|
||||
.optional()
|
||||
.describe(
|
||||
'"type" (default) sends one keystroke per character. "paste" inserts the whole value in one operation and replaces any existing content, so `clear` is redundant with it. Use "paste" for multi-line values, code or JSON editors, and whenever typed text comes back garbled, duplicated or wrongly indented.',
|
||||
),
|
||||
clear: z.boolean().optional().describe('Clear existing text first'),
|
||||
submit: z.boolean().optional().describe('Press Enter after typing'),
|
||||
delay: z.number().optional().describe('Delay between keystrokes in ms'),
|
||||
@@ -103,6 +109,7 @@ function browserType(connection: BrowserConnection): ToolDefinition {
|
||||
browserTypeSchema,
|
||||
async (state, input, pageId) => {
|
||||
await state.adapter.type(pageId, input.element, input.text, {
|
||||
mode: input.mode,
|
||||
clear: input.clear,
|
||||
submit: input.submit,
|
||||
delay: input.delay,
|
||||
|
||||
@@ -244,6 +244,8 @@ export interface ClickOptions {
|
||||
}
|
||||
|
||||
export interface TypeOptions {
|
||||
/** 'paste' inserts the value in one operation, replacing existing content. */
|
||||
mode?: 'type' | 'paste';
|
||||
clear?: boolean;
|
||||
submit?: boolean;
|
||||
delay?: number;
|
||||
|
||||
Reference in New Issue
Block a user