Repository navigation
feat(screenshot): add a per-request iOS simulator screen mask - #4206
Conversation
|
@sync is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
|
At c7ef225, the simulator capture change does what it says. The live evidence is good: the same iPhone 12 mini simulator, with the setting black and with it unset, went through a checkout-built daemon. All four corners came out The open question is where the policy lives. Every other screenshot option is request-scoped, but this one is read from the daemon's startup environment, so it silently changes pixels for every client of that daemon, including live diff-screenshot captures. Could it instead be an explicit screenshot option, carried through the CLI, Node and MCP surfaces and the daemon capture request, with the Apple backend translating it to The full unit gate failed on two Android fill-verification assertions. The base reproduced the line-117 failure, and the line-143 failure is a timing assertion (3380 ms against a 1500–1649 ms window), so both look unrelated to this change; a clean rerun would settle it. The only failing check is the Vercel deployment authorization, which is not a code problem. No conflicts are known. Before merge, please settle where the capture policy lives and get a passing full gate. |
On it, thanks for the quick reply |
|
Agreed, making this a per-request flag was the better call. I’ve replaced the env var with |
There was a problem hiding this comment.
All reported issues were addressed across 28 files
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
|
At 0bbe8a6, the earlier environment-to-request concern is fixed, but the capture support rule and live verification still need work. Could you provide live evidence for the public client and diff capture route? Replay does not exercise the changed client or live-diff path, which the supplied validation describes as mocked. From a checkout-built daemon, capture black/default/black through the public screenshot client on one simulator and daemon, and show the commands, source SHA, device identity, route, dimensions and corner pixels. Then compare a masked live screenshot with a masked baseline, force a mismatch with --overlay-refs, and inspect the overlay capture's corners. Attribute the unchanged backend evidence to 35fba70 and run the changed diff route at head. Could the existing capture-operation binding receive the selected implementation's mask capability, default to unsupported, and enforce it at the common capture invocation? Keep the per-request API and one enum/value reader in the screenshot contract. Apple can determine support from the handheld simctl, runner, macOS or physical route, and masked captures must use a flow that cannot fall back to runner. Extend the binding's owning type first, then remove the superseded daemon family test and provider wrapper; keep saved-image rejection in diff because it has no capture invocation. The open threads on direct Apple calls and lower-level mask handling still apply. The simulator family scope needs clarification or native evidence before it can be treated as a defect. The CLI diff guard does not apply because daemon admission guards that route; the test assertion does not apply because it also checks that interactor resolution was not called; the optional path does not apply because enum parsing rejects the invalid value first. The only failing check is the Vercel deployment authorization, which is not a code problem. The local affected gate also failed, and the second Android assertion has no conclusive base comparison yet. No conflicts are known. Before merge, please provide the live evidence, settle mask support at the capture owner, and get a passing exact-head affected gate or show that the failure also happens on main. |
|
At da8b703, the code looks good. Mask support is now decided once, by the Apple screenshot support check, and enforced where the capture runs. A masked capture cannot fall back to a route that ignores the mask. The two remaining threads, on diff capture and optional paths, no longer apply: diff goes through the guarded capture, and the parser rejects the invalid value first. The live evidence covers what I asked for. A checkout-built daemon went through the public client with black, then default, then black again on one simulator. The corners came back The local gate failures also happen on main (the Apple runner symlink removal and the Android fill-verification line 117). The line-143 failure is a timing assertion. No GitHub Actions checks have run on this head yet; the only reported check is the Vercel deployment authorization. No conflicts are known. Before merge, please mark the PR ready so the CI lanes run on this head. |
There was a problem hiding this comment.
All reported issues were addressed across 31 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
Thanks Michael. The live results and screenshots are inline, and the PR is ready for CI. I also tightened the runner assertion and removed the ambiguous tvOS/visionOS wording in 7173938. Production code is unchanged from the live-tested commit. The exact-head gate still hits the same four Apple-runner failures reproduced on main; the new receipt is linked in the description. |
|
At 7173938, the code looks good. The screenshot backend test now asserts the exact refusal reason, and the docs describe the refusal, so the reason assertion thread and the documentation thread no longer apply. The live evidence from da8b703 still covers this head, since only a test and docs changed. The local gate failures match main, as noted before. The only failing check is the Vercel deployment authorization, which is not a code problem, and the GitHub Actions lanes have not run on this head yet. No conflicts are known. |
Summary
Adds request-scoped
--screen-mask blackfor iOS/iPadOS simulator outlines and deterministic visual baselines. Apple declares support for handheld simctl captures; other capture bindings refuse masks. Masked requests cannot fall back to runner.Node/MCP:
screenMask: "black". Saved-image diffs reject it; defaults and thresholds stay unchanged. tvOS/visionOS are unsupported.31 files. Scope expanded to per-request APIs, live diffs, and capture-owner enforcement following review. No dedicated issue; related #4184 addresses runner capture.
Validation
Head:
71739389ed2b21d7188d11a7055fdcea797e58cc.Main:
035c2b6cae0b5a2d6b769c36f63daa1e99f92420.pnpm check:affected --run --head 71739389ed2b21d7188d11a7055fdcea797e58cc(Node 24.13): failed,exactCommit:true. Identical related-suite command on unchanged main: 1,256 passed/4 failed on both, all Apple-runner symlink-removal failures.Same regression command: prior head
0bbe8a6f1148daa7bb743c85d739c2c273d6946bplus tests 11 failed/33 passed; head 44 passed. Eager-loading/capture suites: 936 passed. Separate unit diagnosis reproduced host-kit failures and Android line 117 on main; Android line 143 remains inconclusive.Receipt and commands.
Live at
da8b7036d8874d7322e07026d50ab7c4a18b759f(production unchanged): iPhone 12 mini, iOS 18.6, 360×780. Black/default/black confirms request isolation. Same-screen diff matched: 0/280,800 pixels; changed screen differed: 40,549 (14.44%). Overlay preserves the outline; its red annotation crosses the top-left corner.Live evidence.
Ready for CI as requested by @thymikee; local gate remains base-blocked. CI/native/provider lanes remain required. Vercel needs maintainer authorization.