feat(studio): let an agent drive Studio's selection and playhead - #3515
Conversation
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.
68a32e9 to
f4e938f
Compare
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
🟢 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
useStudioAgentToolsinpackages/studio/src/webmcp/useStudioAgentTools.tsextendsSelectionToolDepsontoStudioAgentToolsDeps, andbuildStudioToolsreadsdepsRef.currentat execute time. The existing "register once, read latest deps at call" pattern (empty-depsuseEffect, mutabledepsRef) 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 (registerToolcall-count assertions bumped 1 → 3, which is the right shape).StudioAgentTools.tsxwiresdepsviauseMemofromuseDomEditActionsContext— theapplySelectionclosure calls the exactapplyDomSelectionthatLayersPanel.tsx:252(human click in the layers panel) already uses, andrequestSeekcloses overusePlayerStore.getState().requestSeek, which is whatBeatStrip.tsxuses for human scrubs. So the agent write path is not a fork; it's the same setter withrevealPanel: true(matching what a click in the layers panel does implicitly). No new invariants to re-prove at the agent path.studioSelectinpackages/studio/src/webmcp/tools/selectionTools.tssplits three failure kinds —blocked/preview-not-mounted,invalid/no-such-handle,blocked/Studio cannot select— and the testdistinguishes a preview that is not mounted from a handle that does not matchexplicitly asserts thereasonstrings 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.studioSeekreadsreadPlayheadtwice around arequestSeekcall and reports the actual landing position. Thefails rather than claiming a seek the player never receivedtest 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>selpriority) — thehf:abcround-trip test verifies mint-after-select returns the most-stable form. The cross-realmasHtmlElementguard (checkingdoc.defaultView?.HTMLElement, not the outer-frameHTMLElement) 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 useshf:abc.studioSelectreturnsmintElementHandle(patchTargetAddress(selection)), which will upgrade adom:fooinput tohf:xyzif 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: inputdom:headlinewhere the element also has anhfId, assert the returned handle starts withhf:. Nit. - No per-tool invocation telemetry.
useStudioAgentTools.tsemitswebmcp.native_presentandwebmcp_registration_failed, but there's notrackEventfiring onstudio_select/studio_seeksuccess or failure. Once this batch ships, distinguishing "agent invoked the tool" from "phantom no-op" post-hoc will want a per-tool counter (withkindfor 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:
applyDomSelectionis 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/#3517land 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
readNumberInputinuseStudioAgentTools.tstreats numeric strings ("5") asNaN, whichstudioSeekthen rejects asinvalid— strict on purpose, given the platform doesn't validateinputSchema. 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
timebypasses the JSON-Schemaminimum: 0sinceNumber.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) makesstudioSeekreturn adid not move; it is still at 0failure — 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_DESCRIPTIONsays "Pauses playback." I didn't trace therequestedSeekTimesubscriber 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 thatuseTimelinePlayer's pause-on-seek fires for the agent path too, or move the language to "may pause playback." Nit.annotations: { readOnlyHint: false, untrustedContentHint: true }onstudio_selectis correct (label/text flow back from user composition).studio_seekomitsuntrustedContentHint— 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
left a comment
There was a problem hiding this comment.
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:
requestSeekinplayerStore.tsisset({ requestedSeekTime: time }). It never touchescurrentTime.useTimelinePlayerconsumes it throughusePlayerStore.subscribe(...), which is Zustand's vanilla subscribe, so the listener runs synchronously insideset, and theseek()callback it invokes ends withsetCurrentTime(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
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>
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>
What
Adds
studio_selectandstudio_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.
applyDomSelectiononly 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_seekusesrequestSeek, notsetCurrentTime. 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.
requestSeekis 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
10 new tests in
selectionTools.test.ts:moved: false, which is a different thing from failing.Full package suite 4543 passing.
bunx tsc --noEmitclean.bunx fallow audit --fail-on-issuesclean.Not verified in a real browser yet; that lands with the end-to-end capture.