Open
Conversation
This was referenced Apr 30, 2026
Collaborator
📊 Performance Test ResultsComparing 175ffc2 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
…cal-site # Conflicts: # apps/cli/ai/tools.ts
After PR #3272 (CLI: send screenshots from studio code to Telegram remote sessions, merged 2026-05-01) the codebase has two screenshot tools that capture a URL: take_screenshot for agent-internal visual reasoning, and share_screenshot for fire-and-forget delivery to the user. This branch's resolveScreenshotUrl helper was originally only applied to take_screenshot, but share_screenshot has the same URL-resolution gap: agents that know a local Studio site by name must already know its public URL to share a screenshot of it. Apply the same nameOrPath/path schema and resolveScreenshotUrl call to share_screenshot. Both tools now accept the same shape, both errors out cleanly when neither url nor nameOrPath is provided. Slim the test diff to a single regression test. The original branch added four behavior tests plus a mockScreenshotBrowser helper, but the regression we're guarding against is specifically 'agent passed nameOrPath, screenshot resolved correctly'. The other cases (explicit URL, path composition, error path) are nice-to-haves and can land in follow-up coverage if needed; types and existing trunk tests cover the gating and tool-list shape. ## AI assistance - **AI assistance:** Yes - **Tool(s):** Claude Code (Sonnet 4.5) - **Used for:** Drafted the share_screenshot extension and slimmed the test diff under Chris's direction. Chris reviewed the scope trade-off (extend to both tools vs leave share_screenshot for later) and chose the unconditional both-tools fix per the fix-upstream-first rule.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
take_screenshotandshare_screenshotto target local Studio sites with the samenameOrPathcontract used by the rest of the local-site AI tool surface.resolveScreenshotUrl()helper consumed by both tools.urlscreenshot behavior; emit a clean error when neitherurlnornameOrPathis provided.Why
The local Studio AI prompt tells agents to use
take_screenshotafter building a local site, but the tool originally only acceptedurl. Neighboring local-site tools such aswp_cliandvalidate_blocksacceptnameOrPath, so agents can naturally calltake_screenshotwithnameOrPath+pathand hit MCP schema validation before the screenshot handler runs.share_screenshot(added by #3272) has the same URL-resolution gap: agents that know a local Studio site by name must already know its public URL to share a screenshot of it. Per the standing "fix upstream first, never paper over" rule, fixing one tool but not the other would calcify the workaround pattern. Both tools now share the same resolver and the same fallback behavior — passurl, or passnameOrPath(+ optionalpath), or get a clean error.Closes #3306.
What changed since the original draft
resolveScreenshotUrl()toshare_screenshotin addition totake_screenshot. Both tools now accept the same{ url?, nameOrPath?, path? }shape and call the same resolver.nameOrPathresolves correctly. The previous draft had four behavior tests plus a mock helper; the regression we need to guard against is specifically "agent passednameOrPath, screenshot resolved against the local site." Types and existing trunk tests cover gating and tool-list shape; explicit-URL / path-composition / missing-input error cases can land in follow-up coverage if needed.Tests
npx vitest run apps/cli/ai/tests/tools.test.ts— 22 passednpx eslint apps/cli/ai/tools.ts apps/cli/ai/tests/tools.test.ts— cleannpm run typecheck --workspace apps/cli— cleanAI assistance