Skip to content

chore(recorder): move signals from the action onto the action context#41755

Open
dgozman wants to merge 2 commits into
microsoft:mainfrom
dgozman:chore-recorder-signals
Open

chore(recorder): move signals from the action onto the action context#41755
dgozman wants to merge 2 commits into
microsoft:mainfrom
dgozman:chore-recorder-signals

Conversation

@dgozman

@dgozman dgozman commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #41732 — its commit will drop out of this diff once it lands.

Summary

  • signals moves off ActionBase onto ActionInContext ({ pageGuid, action, signals }), since signals describe the context of an action, not the action itself. generateAction reads actionInContext.signals; each SignalAdded consumer keeps its own ActionInContext[] and appends the signal to the matching entry.
  • Removed the batch collapseActions() / shouldMergeAction() helpers. The merge (fill same-selector, navigate) is now an inline replace-or-push of the last action in each client — RecorderApp, DebugController, and recorder-api's RecorderLog.
  • ProgrammaticRecorderApp no longer merges: it always emits a fresh actionAdded/signalAdded. actionUpdated is removed from the protocol enum, and signalAdded now carries the regenerated last-action code (was '').
  • RecorderEventSink hands clients pure Action/Signal instead of the in-context wrappers.

Generated output is unchanged; JSONL still serializes signals.

dgozman added 2 commits July 12, 2026 16:57
JsonRecordActionTool recorded every click as it arrived, so a double click
produced two actions that shouldMergeAction() had to collapse. Give it the
same stalled-click handling RecordActionTool has, and drop the click branch
of shouldMergeAction().

While stalling a click, a following click cancelled the pending one instead
of committing it, silently swallowing the first click. Both tools now commit
the pending click when a new single click arrives, and only cancel it when
the click continues a multi-click, which 'dblclick' reports instead.

Also remove ActionInContext.startTime/endTime and SignalInContext.timestamp.
endTime and timestamp were never read; startTime had a single consumer, the
navigation signal threshold, which now lives in
RecorderSignalProcessor._lastActionTimestamp.
Signals (popup, download, dialog, expect) are not part of a recorded
action, so drop `ActionBase.signals` and put `signals` on `ActionInContext`
instead. Each `RecorderEvent.SignalAdded` consumer keeps its own
`ActionInContext[]` and appends the signal to the matching entry, and codegen
reads `actionInContext.signals`.

Collapse where it is consumed: remove the batch `collapseActions()` /
`shouldMergeAction()` helpers and instead replace-or-push the last action
inline in each client (RecorderApp, DebugController, recorder-api).
`ProgrammaticRecorderApp` no longer merges — it always emits a fresh
actionAdded/signalAdded, so `actionUpdated` is gone from the protocol, and
signalAdded now carries the regenerated last-action code instead of ''.

`RecorderEventSink` now hands clients pure `Action`/`Signal` rather than the
in-context wrappers.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [firefox] › mcp/annotate.spec.ts:173 › user-initiated annotate downloads zip with feedback.md @mcp-macos-latest-firefox
❌ [webkit] › mcp/annotate.spec.ts:110 › should abort annotation when last screenshot is removed @mcp-macos-latest-webkit

7758 passed, 1249 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

10 flaky ⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@frozen-time-library-chromium-linux`
⚠️ [chromium-page] › page/page-screenshot.spec.ts:602 › page screenshot › mask option › should remove elements based on attr `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:371 › should produce network events, routing, and annotations for Service Worker `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:337 › screencast › should work for popups `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/chromium/connect-to-worker.spec.ts:19 › should connect, evaluate, receive console and disconnect `@chromium-ubuntu-22.04-node22`
⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/heap.spec.ts:223 › should not leak workers `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/browsercontext-clearcookies.spec.ts:52 › should remove cookies by name `@webkit-ubuntu-22.04-node20`

49539 passed, 1168 skipped


Merge workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant