Repository navigation
feat(automation): pointer, keyboard, hover, scroll, drag and wait in the app window - #1018
Conversation
… keyboard input Extend the shared page engine with pointer clicks, hover, wheel scrolling, stepped pointer dragging, keyboard chords and bounded text waits. Keep tool commands flat and validate them at both bridge boundaries. Native input is marked by an expiring page lease so trusted automation never qualifies as a human Mission answer; page functions retain their depth marker. Preserve exact window/run ownership and stale-ref checks. Focused Preferences and remote windows fall back to the most recently used local application window; Preferences itself remains outside automation. Extract the real Chromium test harness and verify the Mission editor, pointer-only menus, scrolling, dragging, keys, and the actual InterruptionDock human-answer boundary. Validation: 43 targeted server tests, five real Chromium checks, full server suite, isolated automation fixture against private OpenCode 2.0.31 and native bundle regression pass. Tauri cargo check passes with existing warnings; server, UI and Electron typecheck diagnostics match baseline. No installed application or shared daemon was restarted.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review of 26a1d69 (git diff github/dev...26a1d695).
Validation on this SHA.
- Server:
src/automation/page/*.test.ts,developer-cdp,opencode/automation-pluginandroutes/automation-pluginpass 43/43. - UI:
automation-forms.test.tsandautomation-input.test.tspass 5/5 in real Chromium.
What looks correct.
- Parsing is idempotent across both sides of the bridge.
- Refs are resolved before any page access.
- The checks between protocol steps are preserved.
- Hit testing correctly refuses covered elements.
- Pointer coordinates stay correct under 150 % page zoom (checked with a zoomed Chromium profile).
- The Electron and Tauri changes only fall back to the most recently used local application window (Preferences stays out).
All findings below were reproduced with real Chromium driven by the real DeveloperCdp, in scratch tests outside the repo.
P1
1. press with a ref sends keys to whatever is focused when the ref cannot take focus.
packages/server/src/automation/page/actions.ts:91-97runsFOCUS(actions.ts:188) and then dispatches the chord without checking that focus moved.- This is the same misdirection as the PR #1015 round-1 P1 for
type, now with trusted keys. - Reproduction: a composer
<textarea>with a draft is focused and sends on Enter. AConfirmbutton's ref goes stale because its dialog is hidden.press({ key: "Enter", ref: confirm })then delivers Enter to the composer: the page loggedPROMPT SENT: user draft, and the result{ focused: 'textarea "Prompt user draft"' }only reveals this afterwards. - The same happens for an
inerttarget behind a modal, or a disabled control. - Fix: in
FOCUS, reuse PREPARE's check. Refuse:disabled, and afterfocus()requireelement.getRootNode().activeElement === element(or a focused contenteditable host that contains it). Otherwise throwcodenomad:unfocusablebefore any key is sent. Add a browser regression for a hidden, an inert and a disabled ref.
P2
2. Security: the expiring input lease can lapse before Chromium processes an automation event, which then counts as a human answer.
protocol.ts:98-113writes__codenomadAutomationInputUntil = Date.now() + 15000before each event.lib/automation-input.ts:4-7compares that withDate.now()when the event is handled. That check is whatform-answer-interaction.ts:14(capture) and:47(consume) rely on.- If the renderer's main thread is busy for longer than the lease between the lease write and the event's dispatch, the trusted event is handled after the lease expired. Long tasks in a large transcript, or work an agent deliberately schedules, can cause this.
- Reproduction with a simulated 16 s main-thread task starting right after the lease write:
- A plain page logged
keydown:Enter:trusted=true:marked=false,click:BUTTON:trusted=true:marked=false,submit:marked=false. Thepressitself reportedCDP command Input.dispatchKeyEvent timed out. - With the real
InterruptionDockfixture (the same setup asanswerDock("press")), the automation's answer{"q0":"Automation answer"}was sent withx-codenomad-human-answer: 1.
- A plain page logged
- So AGENTS.md's "automation can never produce the trusted dock click/key press" does not hold.
- Fix: base the check on when the event was created, not when it is handled. Chromium sets
event.timeStampwhen DevTools creates the event; in the 4 s stall, the keydown'stimeStampwas 4000 ms old when handled.- Have the lease record
performance.now()values. - On clear, close the window at its end time (
until = performance.now()) instead of setting0. - Pass the event into
automationInputActive(event), and treat it as automation whenevent.timeStamp <= until. - Keep the depth check, and the
Date.now()-free check at the consume boundary. - Add a regression that delays dispatch beyond the lease.
- Have the lease record
3. drag presses a different element when the drop target must be scrolled into view.
- At
actions.ts:75-80,aim(from)measures the source, thenaim(to)runsDOM.scrollIntoViewIfNeededon the target. That scroll moves the page under the storedfrompoint, which is no longer hit-tested. - Reproduction: 40 rows of 50 px in an 800×600 viewport,
dragfrom "Row 1" to "Row 35". The page loggedpressed r30,released over r35, and the action returned{}. - In CodeNomad this applies to dragging Changes rows between staged/unstaged zones, or reordering long lists. The wrong item is moved and success is reported.
- Fix: after both aims, re-measure both boxes without scrolling and require both in the viewport. Re-run the source hit test, or refuse with "scroll so both … are visible".
P3
4. A failed click, drag or chord leaves the mouse button or key pressed.
clickAt/dragBetween/pressKeys(input.ts:70-109) send the release only on the success path.- When a dispatch times out, or a checkpoint fails after
mousePressed/rawKeyDown,markedInputonly clears the lease. - Reproduction: a 2.5 s main-thread task during a drag made the action fail with
Input.dispatchMouseEvent timed out, with nopointerup. A later plain human mouse move, with no button held, still dragged the handle (184.4px → 680px; the log contains onlydown). - Fix: in a
finally, track what was pressed and send a best-effortmouseReleased(buttons 0) and key-ups for held keys and modifiers, still under the lease.
No other actionable issues found. Please re-request review after the fixes, with regressions for #1–#4.
… cleanup Refuse press targets that cannot acquire focus instead of delivering keys to an unrelated composer. Drag only when both endpoints are currently visible, without scrolling under a previously measured source point. Use monotonic input intervals and the originating DOM event timestamp to reject automated human-answer provenance even after a renderer stall outlasts the lease. Retain the interval end and propagate provenance through an automated key's same-turn implicit button click. Best-effort release all potentially held buttons and keys after failed input sequences, on the original channel only and without replaying the action. Validation: 44 targeted server tests and six real Chromium regressions pass, including hidden-focus refusal, offscreen drag refusal, failed input release and a real InterruptionDock answer delayed by a 16-second renderer stall. Addresses the gatekeeper's P1, two P2 and P3 findings for PR #1018.
|
Gatekeeper round-1 findings fixed at 6af0691. Validation: full server suite 2,400 pass / 0 fail / 7 existing skips; targeted automation server suites 44/44; real Chromium suites 6/6, including a 16-second stalled renderer human-answer regression. Round-2 independent review is in progress; no merge before zero actionable P0–P3 findings. |
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper re-review of 6af0691 (full diff github/dev...6af06919).
Validation on this SHA. The server suites (src/automation/page/*.test.ts, developer-cdp, opencode/automation-plugin, routes/automation-plugin) pass 44/44. automation-forms.test.ts and automation-input.test.ts pass 6/6 in real Chromium.
Round-1 findings, re-run against this SHA:
- #1 (P1) is resolved.
presswith a hidden, a disabled (fieldset) and aninertref is refused in all three cases. The focused composer received no key. - #2 (P2) is resolved. I re-ran my variant where the 16 s renderer stall starts after the lease write and before Chromium dispatches the key (the PR's test stalls inside the keydown handler). The real InterruptionDock reply carried no human-answer header.
event.timeStampis the creation time, confirmed in Chromium: a delayed keydown/keypress keepts=86when handled at 2586.- The implicit submit click gets a new timestamp but fires in the same task, so the one-turn flag is required and correct.
press Spaceon Submit, with the stall before its keyup, also produced no human header.
- #3 (P2) is resolved.
dragno longer scrolls and refuses an end that is not visible.
P3
4 (round-1 #4) is only partly fixed. When a renderer stall makes the press time out, the cleanup either completes the click behind a reported failure or still leaves the button held.
- The release at
packages/server/src/automation/page/protocol.ts:118-123sendsMARK_INPUT(line 120) and the release in the sametry. The release is therefore gated on the mark's own round trip, which cannot complete while the renderer is stalled. The repro scales the controller timeout to 1 s, so the thresholds are 1×/2× of the 5 s production timeout. - Stall longer than 2× the command timeout: the mark times out, the
catchskips the release, and the button stays held.- My round-1 repro is unchanged: a drag with a 2.5 s stall and a 1 s timeout fails, the page sees only
down, and a later plain human mouse move, with no button held, still drags the handle (184.4px → 680px). - Likewise, the stalled dock
clickleft the press pending and sent nothing.
- My round-1 repro is unchanged: a drag with a 2.5 s stall and a 1 s timeout fails, the page sees only
- Stall between 1× and 2× the timeout: the mark succeeds after the stall and the queued press plus release form a full click. The action still returns
CDP command Input.dispatchMouseEvent timed out.- Repro: a 1.5 s stall with a 1 s timeout logged
CLICK handled trusted=trueafter the reported failure. - An agent that retries then double-acts, for example a second Create or Send.
clickCount: 0does not prevent this: Chromium still clicks on press/release withclickCount: 0.
- Repro: a 1.5 s stall with a 1 s timeout logged
- This contradicts
dev-docs/BROWSER_AUTOMATION.md:28("best-effort release held buttons/keys … without replaying the action"). - The same ambiguity applies to keys. A timed-out Enter is still delivered later and submits, which is now provenance-safe but reported as a plain failure.
- Fix:
- Send the release even if the mark write fails or times out. Provenance is already covered by the timestamp lease from the last applied mark, and the release is created within it.
- Report input timeouts as an unknown outcome, e.g. "the page was busy; the input may still be delivered — inspect before retrying", rather than a plain failure.
- Cover both thresholds with a unit test whose fake channel times out on
Runtime.evaluateduring cleanup.
No other actionable issues found in the fix commit. The performance.now() lease, Math.min closing, event.timeStamp capture, one-turn implicit-click flag, the FOCUS checks and non-scrolling drag are all sound.
…ertain outcomes Do not suppress a pending button/key release when its cleanup provenance-mark write times out on a frozen page. Still queue a best-effort release on the original channel. Report admitted input timeouts as an unknown outcome with explicit inspection-before-retry guidance, since the renderer may deliver the input or complete its click later. Add regressions for recovery before release and continued freezing through release: both attempt cleanup exactly once, never replay the press, and expose unknown results rather than proven failures. Targeted server suites pass 45/45; six real Chromium checks including delayed human-provenance protection pass.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper re-review of 08af4c2 (full diff github/dev...08af4c21).
Validation on this SHA. The server suites (src/automation/page/*.test.ts, developer-cdp, opencode/automation-plugin, routes/automation-plugin) pass 45/45. automation-forms.test.ts and automation-input.test.ts pass 6/6 in real Chromium.
Round-2 P3 is resolved. My freeze repros, with a 1 s controller timeout:
- 1.5 s and 2.5 s stalls after the press lease: both return "Input timed out: its outcome is unknown … inspect before retrying". The click completes once (
pointerdown,pointerup,click) with no replayed press. - Drag with a 2.5 s stall: the page now gets
down,up, and a later plain human mouse move no longer drags the handle (184.4px → 184.4px).
P3 (security-relevant)
Releasing after a failed lease write lets the cleanup click come back with a human-answer header when the controller timeout is 7.5 s or more.
- At
packages/server/src/automation/page/protocol.ts:125-134, cleanup now sends the release even when its mark write times out. That release is created about press-dispatch timeout + mark-write timeout ≈ 2 ×timeoutMsafter the press's lease write. - The click that the release completes carries that creation time, so its provenance holds only while 2 ×
timeoutMs<INPUT_MARK_LEASE_MS(15 s,protocol.ts:138). - Production is safe: the default
DEFAULT_TIMEOUT_MS = 5_000(developer-cdp.ts:7) puts the release at about T+10 s, inside the lease. - But nothing enforces this relationship.
DeveloperCdpacceptstimeoutMs, and the PR's own browser harness usestimeoutMs: 10_000(packages/ui/tests/browser/automation-session.ts:34). - Repro: I clicked the real InterruptionDock Submit through
DeveloperCdp, with a 25 s renderer stall starting right after the press's lease write.- With
timeoutMs: 5_000: unknown outcome, one reply, nox-codenomad-human-answer. - With
timeoutMs: 10_000: unknown outcome, one reply withx-codenomad-human-answer: 1. The automation click was recorded as a human answer.
- With
- A held
Spacereleased the same way (its keyup activation click) depends on the same margin. - Fix: make the bound structural rather than numerical. For example:
- Dispatch cleanup releases with the original press's
timestamp.Input.dispatchMouseEvent/dispatchKeyEventaccept one, so the release and its click stay inside the interval that was already applied. Or - Derive the lease from the controller timeout (lease > 2 ×
timeoutMs+ margin) and reject constructions that violate it.
- Dispatch cleanup releases with the original press's
- Add a regression with the 10 s harness timeout and a stall longer than the lease.
No other actionable issues found. The focus fence, timestamp-based lease, one-turn implicit-click flag, non-scrolling drag and unknown-outcome reporting are sound.
…put cleanup Read the page's epoch timestamp alongside its monotonic provenance marker and use it explicitly on native mouse/key events. Held-input cleanup retains the original press timestamp even if renewal times out, eliminating the implicit safety dependency between command timeout and marker lifetime. Refuse dispatch when the page cannot establish a valid provenance timestamp. Add timestamp-retention and invalid-provenance unit checks, and a real InterruptionDock click frozen for 25 seconds with the test helper's 10-second command timeout. Its cleanup-completed reply never carries the human-answer header. Targeted server and six real Chromium checks pass; baseline typecheck diagnostics are unchanged. Addresses the remaining security-relevant P3 from gatekeeper round three on #1018.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper re-review of 0ffb5a5 (full diff github/dev...0ffb5a53).
Zero actionable P0–P3 findings.
What I verified on this SHA:
- Tests. The server suites (
src/automation/page/*.test.ts,developer-cdp,opencode/automation-plugin,routes/automation-plugin) pass 46/46.automation-forms.test.tsandautomation-input.test.tspass 6/6 in real Chromium, including the 16 s key stall and the new 25 sclick-stalleddock case. - Typecheck.
tsc -p packages/serverreports no diagnostics in the non-testautomation/*ordeveloper-cdpsources.
Round-3 P3 is resolved. I re-ran my 25 s repro against the real InterruptionDock. The renderer stall starts right after the input's lease write and before Chromium handles the input:
| Input | Controller timeout | Result | Replies | Human header |
|---|---|---|---|---|
| Click Submit | 10 000 ms | unknown outcome | 1 | none |
| Click Submit | 5 000 ms | unknown outcome | 1 | none |
press Space on Submit, stall before its keyup |
10 000 ms | unknown outcome | 1 | none |
press Enter, stall before keydown dispatch |
10 000 ms | unknown outcome | 1 | none |
At 10 000 ms the click case produced x-codenomad-human-answer: 1 on 08af4c2; it no longer does.
The cleanup release now keeps the press's own CDP timestamp:
- In my 1 s-timeout click repros (1.5 s and 2.5 s stalls),
pointerdown,pointerupandclickall carry the press'stimeStamp. The action still reports the unknown outcome and completes exactly once. - The interrupted drag releases (
down,up), and a later plain mouse move leaves the handle still (184.4px → 184.4px). - A following normal drag and click behave normally.
- An invalid page timestamp refuses before any input, per the new unit test.
Earlier fixes still hold:
pressfocus fencing for hidden, disabled and inert refs.- Non-scrolling
drag. - Timestamp-based provenance with the one-turn implicit-click flag.
- Unknown-outcome reporting with no automatic retry.
- The local-only most-recently-used window fallback.
Scope
Second step after #1015: complete app-window interactions on the shared page engine. Adds real pointer clicks (covered-element checks), hover, stepped pointer drag, wheel scrolling, key chords, and bounded text waits. Flat commands are validated at both bridge boundaries.
Trust and targeting
Every trusted automation event carries an expiring page lease; fixed page functions retain their depth marker. The real InterruptionDock regression proves automated clicks and Enter never carry the human-answer header, while user clicks do. Preferences remains outside the automation target; focusing it or a remote window falls back to the MRU local app window.
Validation
43 targeted server tests, five real Chromium tests (including the Mission editor and dock), full server suite, isolated native automation on OpenCode 2.0.31, and bundle regression pass. Tauri cargo check passes. Typecheck diagnostics unchanged from baseline (server 14, UI 1, Electron 17). No shared daemon or installed app restart.
Next
The dependent third PR migrates Electron and Tauri browser previews to this same page engine, removing their duplicated action/snapshot implementations.
Refs #1008