Skip to content

feat(studio): let an agent drive Studio's selection and playhead - #3515

Merged
miga-heygen merged 1 commit into
mainfrom
feat/studio-webmcp-select-seek
Aug 30, 2026
Merged

feat(studio): let an agent drive Studio's selection and playhead#3515
miga-heygen merged 1 commit into
mainfrom
feat/studio-webmcp-select-seek

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

Adds studio_select and studio_seek, so an agent and the human are looking at the same element and the same instant.

Stacked on #3514.

Why

Selecting is what makes an agent's move visible: it reveals the inspector and draws the selection box, exactly as clicking would. Seeking is what lets an agent ask "what does this look like at 2.4 seconds", which is unanswerable from source.

Both are also prerequisites for the editing tools. Most of Studio's edit handlers act on the ambient selection, so the agent has to be able to set it.

How

Selection is shared state, not a per-call argument, and that is forced rather than chosen. applyDomSelection only schedules a React state update, and the edit handlers close over the state value, so selecting and committing inside one tool call would write to whatever was selected before. Two tool calls are separated by a render, so the contract is select first, then act. That is also how a human works: click, then type.

studio_seek uses requestSeek, not setCurrentTime. The latter only moves the timeline's displayed number and leaves the composition where it was.

Two things the tools refuse to fake:

Seek does not clamp. seek() already clamps against the adapter's duration, which can differ from the store's, and clamping again would give that invariant two owners that can disagree. The tool reports where the playhead actually landed, read back afterwards.

Seek verifies. requestSeek is fire-and-forget and cannot report that no adapter was mounted to receive it. The tool compares the playhead before and after, and fails rather than claiming a seek that never happened.

Select separates three failures a single message would have merged: the preview is not mounted yet (wait), no element matches the handle (re-read), and the element cannot be selected (try a neighbour). The agent's next move differs for each.

Test plan

  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (if applicable)

10 new tests in selectionTools.test.ts:

  • Select applies the selection a click produces, reveals the inspector, and returns a handle that round-trips.
  • The three failure cases are asserted to be distinct, not just failures.
  • A failed select leaves the existing selection untouched.
  • A blank handle is rejected without touching the preview.
  • Seek reports where it landed rather than what was asked, reports that playback stopped, and fails when the playhead did not move rather than claiming a seek the player never received.
  • Seeking to where the playhead already is succeeds with moved: false, which is a different thing from failing.

Full package suite 4543 passing. bunx tsc --noEmit clean. bunx fallow audit --fail-on-issues clean.

Not verified in a real browser yet; that lands with the end-to-end capture.

@miguel-heygen
miguel-heygen marked this pull request as draft August 27, 2026 01:33
@miguel-heygen
miguel-heygen changed the base branch from feat/studio-webmcp-polyfill to main August 27, 2026 03:38
Adds `studio_select` and `studio_seek`, so an agent and the human are looking
at the same element and the same instant. Selecting reveals the inspector,
exactly as a click does, which is what makes the agent's move visible.

Selection is shared state, not a per-call argument, and that is forced rather
than chosen. Most of Studio's edit handlers read the ambient React selection,
and `applyDomSelection` only schedules a state update, so selecting and
committing inside ONE call would write to whatever was selected before. Two
tool calls are separated by a render, so the contract is select first, then
act. That is also how a human works: click, then type.

`studio_seek` uses `requestSeek`, not `setCurrentTime`. The latter only moves
the timeline's displayed number and leaves the composition where it was.

Two things the tools refuse to fake:

Seek does not clamp. `seek()` already clamps against the adapter's duration,
which can differ from the store's, and clamping again would give that
invariant two owners that can disagree. The tool reports where the playhead
actually landed instead, read back afterwards.

`requestSeek` is fire-and-forget, so it cannot report that no adapter was
mounted to receive it. The tool compares the playhead before and after and
fails rather than claiming a seek that never happened.

Select separates three failures that a single message would have merged: the
preview is not mounted yet (wait), no element matches the handle (re-read),
and the element cannot be selected (try a neighbour). The agent's next move
differs for each, so collapsing them would cost it a round trip or a retry
loop.
@miguel-heygen
miguel-heygen force-pushed the feat/studio-webmcp-select-seek branch from 68a32e9 to f4e938f Compare August 27, 2026 04:48
@miguel-heygen
miguel-heygen marked this pull request as ready for review August 30, 2026 04:55

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 hyperframes#3515 at HEAD f4e938fc — LGTM, leaving as a comment. First slice of the Studio WebMCP agent-tools batch (studio_select + studio_seek) is disciplined about the composition boundary: the agent path shares Studio's DomEdit + player machinery rather than forking it, which is what keeps the human-UI invariants riding along for free. A few small notes below — none are blockers.

CI (green on latest attempt) — all 8 ruleset-required contexts green: Format, Lint, Typecheck, Fallow audit, Build, Producer: unit tests, Producer: integration tests, Test. Nothing pending. WIP marketplace check green. reviewDecision: REVIEW_REQUIRED, mergeable: MERGEABLE, not draft.

Peer signals at HEAD — none. gh api .../pulls/3515/reviews is empty; no Vai / tai / Somu / Genesis / Jerrai on this one yet. Sibling PRs #3516 (studio_frame) and #3517 (studio_inspect) stack on top; I only looked at #3515.

Composition — what I actually traced

  • useStudioAgentTools in packages/studio/src/webmcp/useStudioAgentTools.ts extends SelectionToolDeps onto StudioAgentToolsDeps, and buildStudioTools reads depsRef.current at execute time. The existing "register once, read latest deps at call" pattern (empty-deps useEffect, mutable depsRef) covers the two new tools — no new registration lifecycle, and the existing test "executes against the LATEST deps, not the ones present at registration" still enforces it (registerTool call-count assertions bumped 1 → 3, which is the right shape).
  • StudioAgentTools.tsx wires deps via useMemo from useDomEditActionsContext — the applySelection closure calls the exact applyDomSelection that LayersPanel.tsx:252 (human click in the layers panel) already uses, and requestSeek closes over usePlayerStore.getState().requestSeek, which is what BeatStrip.tsx uses for human scrubs. So the agent write path is not a fork; it's the same setter with revealPanel: true (matching what a click in the layers panel does implicitly). No new invariants to re-prove at the agent path.
  • studioSelect in packages/studio/src/webmcp/tools/selectionTools.ts splits three failure kinds — blocked/preview-not-mounted, invalid/no-such-handle, blocked/Studio cannot select — and the test distinguishes a preview that is not mounted from a handle that does not match explicitly asserts the reason strings differ, not just that both fail. That test is doing real work: collapsing these back into one message would cost the agent its retry strategy.
  • studioSeek reads readPlayhead twice around a requestSeek call and reports the actual landing position. The fails rather than claiming a seek the player never received test drives the fire-and-forget-with-no-adapter case. Nice: the intended clamping owner is the adapter, not the wrapper, and this file's comment ("clamping again would give that invariant two owners that can disagree") is the right instinct.
  • handles.ts (pre-existing, hf > dom > sel priority) — the hf:abc round-trip test verifies mint-after-select returns the most-stable form. The cross-realm asHtmlElement guard (checking doc.defaultView?.HTMLElement, not the outer-frame HTMLElement) is exactly the fix you need when the preview lives in an iframe.

What's NOT in the diff, worth naming

  • No sel: or mixed-scheme round-trip test. The single round-trip test uses hf:abc. studioSelect returns mintElementHandle(patchTargetAddress(selection)), which will upgrade a dom:foo input to hf:xyz if the element has both — that's the intended stability upgrade, not a bug, but the test suite doesn't guard the resulting shape. Worth adding one negative case: input dom:headline where the element also has an hfId, assert the returned handle starts with hf:. Nit.
  • No per-tool invocation telemetry. useStudioAgentTools.ts emits webmcp.native_present and webmcp_registration_failed, but there's no trackEvent firing on studio_select / studio_seek success or failure. Once this batch ships, distinguishing "agent invoked the tool" from "phantom no-op" post-hoc will want a per-tool counter (with kind for failures). Worth-fixing downstream, not this PR — the framing here is the first slice.
  • Real-browser verification is called out in the PR body as coming with the end-to-end capture. Fine by pattern.
  • Interleaved human + agent selection: applyDomSelection is last-write-wins by React setter semantics, same as any two human interactions racing. Not a bug in this PR because selection-is-shared-state is by design, but as #3516 / #3517 land and edit tools start reading "the current selection," this becomes a class-of-bug worth an integration test at the batch level (agent selects, human clicks elsewhere, agent-invoked edit tool runs → edit target). Flag for the sibling PRs, not this one.

Non-blocking notes

  • readNumberInput in useStudioAgentTools.ts treats numeric strings ("5") as NaN, which studioSeek then rejects as invalid — strict on purpose, given the platform doesn't validate inputSchema. Fine, but worth being explicit in the tool description that the arg is a JSON number, not a numeric string, so an agent that stringified doesn't spend a round trip figuring it out. Nit.
  • Negative time bypasses the JSON-Schema minimum: 0 since Number.isFinite(-1) is true. requestSeek(-1) passes through to the adapter, which clamps to 0. If the playhead was already at 0, before.currentTime !== time (0 !== -1) makes studioSeek return a did not move; it is still at 0 failure — a slightly wrong verdict (the seek did land at the clamped floor). Cheap fix: pre-clamp to [0, duration] before comparing, or reject negatives up front. Nit / worth-fixing on your next pass.
  • STUDIO_SEEK_DESCRIPTION says "Pauses playback." I didn't trace the requestedSeekTime subscriber chain to confirm the pause is guaranteed on every mount state — the human-scrub path presumably behaves the same, so I'm relying on symmetry. Worth a one-line confirmation in the PR body that useTimelinePlayer's pause-on-seek fires for the agent path too, or move the language to "may pause playback." Nit.
  • annotations: { readOnlyHint: false, untrustedContentHint: true } on studio_select is correct (label/text flow back from user composition). studio_seek omits untrustedContentHint — correct, only numeric fields return. Confirming for the record.

Solid start on the batch. Ready to ship from where I sit.

Review by Rames D Jusso

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVED at f4e938fce4e05c5725cfa5b83fce1db16552db00.

Read the six changed files in full, plus playerStore.ts, useTimelinePlayer.ts and the seek() callback they lean on. The composition-boundary discipline is the right call and it is real: studioSelect goes through the same applyDomSelection the Layers panel uses, and studioSeek goes through the same requestSeek a human scrub uses, so neither tool forks a path whose invariants would then need re-proving at the agent entry point.

Two notes, neither blocking.

1. The read-back in studioSeek is correct, and correct for a reason nothing here records

studioSeek calls requestSeek(time) and then immediately reads the playhead back to decide moved. That works only because of a two-link chain outside this file:

  • requestSeek in playerStore.ts is set({ requestedSeekTime: time }). It never touches currentTime.
  • useTimelinePlayer consumes it through usePlayerStore.subscribe(...), which is Zustand's vanilla subscribe, so the listener runs synchronously inside set, and the seek() callback it invokes ends with setCurrentTime(nextTime).

So the new value lands in the store before requestSeek returns, and after.currentTime sees it. The logic is sound.

The fragility is that this invariant is invisible from selectionTools.ts. If that subscription is ever rewritten as a useEffect keyed on requestedSeekTime, which is the more idiomatic-looking form, the store write becomes deferred by a render, after.currentTime === before.currentTime for every seek, and studio_seek begins returning ok: false, kind: "blocked", "the playhead did not move" on every successful seek. A total inversion of the tool's contract, caused by an edit in another package that reads as a cleanup.

The tests do not pin it. selectionTools.test.ts fakes requestSeek as a closure mutating a local currentTime synchronously, which assumes the property rather than demonstrating it. A sentence in the studioSeek comment naming the synchronous-subscribe dependency would be enough.

2. The "did not move" guard misfires at a clamp boundary, and the schema does not prevent it

The guard compares against the requested time, but seek() clamps with Math.max(0, duration > 0 ? Math.min(duration, time) : time). When the request clamps to exactly where the playhead already sits, the seek fully succeeded and the tool reports failure.

Concretely: with duration: 10 and the playhead already at 10, studio_seek(999) clamps to 10, nothing moves, and the agent gets blocked plus the hint "The preview may not be ready. Check studio_look, then retry." Retrying returns the identical failure indefinitely, so it is a trap rather than a transient.

Rames D flagged the negative-time version of this. The reason I would not fix it by tightening input validation: the upper-bound case above is schema-valid (999 satisfies minimum: 0), so enforcing the schema inside readNumberInput would close the negative case and leave this one standing. The fix belongs in the comparator, which wants "did the playhead end up where a clamp would put it" rather than "does it equal the request". Honest caveat: readPlayhead().duration is the store's duration while seek() clamps against the adapter's, and this file's own comment notes those can differ, so a clamp-comparison is close but not exact.

No test covers it. The existing "reports where the playhead landed, not what was requested" case requests 999 but has the fake move the playhead from 0 to 10, exercising the clamped-and-moved path only. The missing member is clamped-and-already-there.

Checked clean

All three tools register in order with the deps-identity regression test intact; studioSelect's three failure kinds stay distinct and the tests assert the reasons differ rather than merely that both fail; the toolFailure export is the whole of the toolResult.ts change; no failure path reaches applySelection; and the useMemo dependency list in StudioAgentTools.tsx covers every captured value.

State at review time: OPEN, mergeable, BLOCKED on review-required, 38 checks green with none in flight. Mine is currently the only non-COMMENTED review here, so this approval is the review gate rather than a second opinion. Merging stays with your lane.

Review by Rames

@miga-heygen
miga-heygen merged commit 0e558d5 into main Aug 30, 2026
45 checks passed
@miga-heygen
miga-heygen deleted the feat/studio-webmcp-select-seek branch August 30, 2026 05:12
miga-heygen added a commit that referenced this pull request Aug 30, 2026
Resolve conflicts from squash-merged #3515 (selection tools).
Branch retains frame + inspect tools from the stack.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
miga-heygen added a commit that referenced this pull request Aug 30, 2026
Resolve conflicts from squash-merged #3515 (selection tools).
Branch retains the full WebMCP tool stack from the PR chain.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

4 participants