fix(macos): refuse --fullscreen on every helper-captured surface - #2849
Merged
Merged
Conversation
desktop and menubar always capture the main display through the macOS helper, so an explicit --fullscreen on either named a frame the capture could never vary. It was parsed, ignored, and echoed back unchanged. Refuse it before any capture with a typed INVALID_ARGS reason instead. macOS app sessions (runner path) and every other platform keep accepting --fullscreen unchanged. Remove the now-dead --fullscreen plumbing: the helper's argument parse, its capture parameter, and the fullscreen response field; the TS helper argv and its return type.
…uter The provider-scenario suite still sent screenshotFullscreen:true on the desktop surface and asserted a --fullscreen helper argv, so it exercised the pre-fix contract and was the only test reaching the refusal through daemon -> bound screenshot runtime -> interactor. Add a direct refusal check (INVALID_ARGS, details.reason SCREENSHOT_FULLSCREEN_MACOS_HELPER_SURFACE_FIXED_FRAME, no helper call), strip screenshotFullscreen from the two desktop capture steps and their expected argv, and stop echoing the retired fullscreen field from the fake helper. Update the desktop-inspection recipe and add a prose note documenting that desktop/menubar refuse --fullscreen. Record the behavior change in the changelog, including its effect on recorded scripts and project config that set screenshotFullscreen for those surfaces.
…cate rejectsMacOsHelperFullscreen hand-listed desktop/menubar as a second, independently maintained copy of usesMacOsSurfaceScreenshot's routing condition, so it silently missed frontmost-app: that surface also captures through the macOS helper's fixed main-display frame but kept accepting and ignoring --fullscreen. Key the refusal directly off usesMacOsSurfaceScreenshot instead, so every surface it routes to the helper today or in the future refuses the flag, and delete the separate list.
Derive the covered surfaces from SESSION_SURFACES filtered to non-'app' (the routing predicate's own domain) instead of a hand list, so screenshot-macos-surface.test.ts now exercises frontmost-app alongside desktop/menubar and stays complete if a future surface is added. Add a matching frontmost-app refusal check to the provider-scenario daemon-router test, next to the existing desktop one.
…d surface The screenshot command's cliDetail, the commands.md prose, and the Unreleased CHANGELOG entry named only desktop/menubar; widen them to match the fix now that frontmost-app also refuses.
|
Size Report
Startup median (7 runs, lower is better):
|
Member
Author
|
Reviewed at 477bcd1. This looks good and is ready for human review. The refusal comes from Not blocking: the All 20 checks are green. The live macOS results are the ones reported in the PR body. |
thymikee
added
the
ready-for-human
This was referenced Sep 24, 2026
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
1 participant
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
screenshot --fullscreenon a macOS helper-captured surface (desktop,menubar,frontmost-app) now fails withINVALID_ARGS(details.reason: SCREENSHOT_FULLSCREEN_MACOS_HELPER_SURFACE_FIXED_FRAME) before any capture. Before, the helper parsed the flag, ignored it (_ = fullscreen), always captured the main display, and echoed the flag back as if honored.The refusal is derived from
usesMacOsSurfaceScreenshot, the same predicate that routes a screenshot to the helper, so a newly routed surface refuses too. The scope is wider than the issue, which named only desktop/menubar: live validation showedfrontmost-appgoes through the same helper path. macOS app sessions (runner path) keep--fullscreen. The helper's ignored flag and response echo are removed. The CHANGELOG notes the.ad/project-config impact.Closes #2799. 14 files.
Validation
cf47df65b6, rebased onto main twice with CHANGELOG-only conflicts, no code change. Rebased head477bcd120d: build, typecheck and targeted vitest green (17 tests).pnpm check:affected --rungreen oncf47df65b6(one earlierscreenshot-density.test.tstimeout was a host-load flake: outside the diff, green alone and on the rerun).pnpm check:macos-helpergreen (7 Swift tests).--fullscreeneach refuse before capture and write no file; Calculator app session captures 460x816 without the flag and the full display with it.