[0xdaniell]

Summary

Add --native-density for iOS simulator screenshots to retain captured pixels and report the scale and logical dimensions for tap mapping. Native density is admitted only by capture owners that report the scale; explicit runner and provider captures are refused. The option is carried consistently as a boolean.

agent-device screenshot --native-density --json

Validation

• Head: 38ec6490; base: 093341a60.
• pnpm check:affected --run --base origin/main --head HEAD passed all runnable local checks at the exact commit: format, lint, typecheck, layering, fallow, build, related tests and unit checks. Unit tests: 17,667 passed, 70 skipped; the unit check also ran its smoke and temporary-file checks.
• Fixed two pre-existing test problems encountered during validation: inefficient fake-time advancement in Apple runner lease testing, and a daemon startup race fixture that spawned an untracked second contender. The latter failed at the same assertion on the unchanged base.
• Focused screenshot and capture capability tests passed. In an isolated checkout, removing the native-density early return failed the pixel-size test (42 versus 126); removing daemon forwarding failed the metadata test.
• Live verification at this head remains outstanding: the iPhone 17 Pro simulator failed to boot twice in the device host without details. The earlier live results in this PR predate the rebases and do not validate this head. GitHub-authoritative checks remain outstanding.

[vercel]

@TUNA-NOPE is attempting to deploy a commit to the Callstack Team on Vercel.

A member of the Team first needs to authorize it.

[cubic-dev-ai]

1 issue found across 14 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/capture-kit/src/screenshot-density.ts">

<violation number="1" location="packages/capture-kit/src/screenshot-density.ts:76">
P2: A `--native-density` capture with no measured source density returns no `logicalWidth`/`logicalHeight`/`pixelDensity` and raises no error. The caller then cannot map pixels to taps, which is the purpose of native mode. Fail explicitly here instead of silently omitting the fields, or make the runner path report its density.</violation>
</file>

<sub>Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: A --native-density capture with no measured source density returns no logicalWidth/logicalHeight/pixelDensity and raises no error. The caller then cannot map pixels to taps, which is the purpose of native mode. Fail explicitly here instead of silently omitting the fields, or make the runner path report its density.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/capture-kit/src/screenshot-density.ts, line 76:

<comment>A `--native-density` capture with no measured source density returns no `logicalWidth`/`logicalHeight`/`pixelDensity` and raises no error. The caller then cannot map pixels to taps, which is the purpose of native mode. Fail explicitly here instead of silently omitting the fields, or make the runner path report its density.</comment>

<file context>
@@ -55,11 +63,17 @@ export async function readScreenshotResultMetadata(params: {
+  const pixelDensity = params.nativeDensity
+    ? params.sourcePixelDensity
+    : resolveScreenshotPixelDensity(params.requestedPixelDensity);
+  if (pixelDensity === undefined) return result;
   return {
     ...result,
</file context>

Suggested change

	if (pixelDensity === undefined) return result;
	if (pixelDensity === undefined) {
	throw new AppError('COMMAND_FAILED', 'Native screenshot did not report its source pixel density');
	}

[thymikee]

I read 80cdf2c. The change needs one more test before it is ready. The only failing check is Vercel, which asks a Callstack team member to authorize a fork deployment, so it is not a build failure and this diff touches no website code. Only two checks ran, so the repo gates (format, layering, closure budget, Fallow, wire ledger) have not run on this head yet.

The behavior that makes the feature work has no test. That is the early return that skips the resample and reports the measured scale (https://github.com/callstack/agent-device/blob/80cdf2c/packages/platform-apple/src/core/screenshot.ts#L489), plus the route that carries the density to the command result. The new tests only call readScreenshotResultMetadata with a hand-fed density and assertSupportedScreenshotPixelDensity directly. If someone deletes the native early return, or the 'native' to nativeDensity translations at screenshot-runtime.ts:367-375 and :403-410 and src/commands/capture/runtime/screenshot.ts:49,56, every added test stays green, so a regression that resamples a native capture back to 1x would pass CI. Please add one case beside the existing density tests in screenshot-simulator-capture.test.ts: screenshotIos with pixelDensity: 'native' on a 3x stub keeps the PNG size and returns facts with sourcePixelDensity 3. Add one daemon-route case through the screenshot-runtime fixture: the screenshotNativeDensity flag yields the result pixelDensity and logical size from the stub's facts. Please confirm each case fails when the native branch or the translation is removed.

Not blocking, and you can take or leave these: the PR body gives live iPhone 17 Pro results without a tested SHA, commands or output (docs/agents/pull-requests.md asks for them, and the first "Tested" block predates this commit); sourcePixelDensity leaks into the public daemon response through the ...captured spread and DaemonScreenshotResult (https://github.com/callstack/agent-device/blob/80cdf2c/packages/contracts/src/daemon-command-surface.ts#L70) although nothing reads it and the value is renamed along the route; the -readonly mapped type at packages/platform-apple/src/core/screenshot.ts:153 could go if captureAndNormalize returned the density and the facts were built once; screenshot-density.test.ts hand-writes a PNG header where encodeFixturePng exists and is not formatted as pnpm format would write it (as are screenshot-density.ts:66-69 and screenshot-options.test.ts:172-179); and the help in packages/command-surface/src/capture/screenshot.ts:43 names only --pixel-density, without the new flag, its iOS-simulator-only scope or its exclusion with --pixel-density.

One design question. Could native be a value of the existing density option end to end, as --pixel-density native with screenshotPixelDensity: number | 'native' through flags, options, daemon and backend? That would remove the screenshotNativeDensity flag key and definition, its script-table entry, the nativeDensity field in six option types, the INVALID_ARGS exclusion branch, and the four 'native' to boolean ternary pairs at screenshot-runtime.ts:367-375, src/commands/capture/runtime/screenshot.ts:49 and src/cli/commands/screenshot.ts:123-124. The refusal would stay in assertSupportedScreenshotPixelDensity. The cost is that the flag definition type is int (packages/contracts/src/screenshot.ts:133), the command field is integerField, and the script parser and journal projection would each need an int-or-keyword case. If you keep the boolean flag, please keep the boolean through every layer, including the backend contract and ScreenshotOptions, and drop the 'native' string value. Either way, one spelling of the idea should cross the layers.

The open inline thread on the script flag round-trip test still stands (#4111 (comment)), and four lower-priority threads on the help text, the field description, the journal spec and ScreenshotCommandResult still apply. The thread on the undefined density branch does not apply: on the simctl and runner-fallback routes normalizeDensity always returns a number, and the undefined branch is reachable only through the Maestro-only runner path, which never carries this flag. Please resolve it: #4111 (comment)

I did not run a simulator or reproduce the 3005-passed unit run, so the live numbers, the tap-mapping claim and the refused flag combination are taken from the PR body. I read --native-density with --crop-on and --overlay-refs only. Crop projects by actual image size (screenshot-crop.ts:70-77), so it should not depend on density, but no live run covers it.

Before merge, please add the platform-apple and daemon-route tests that fail without the native branch, settle the single-spelling question, and run pnpm format and pnpm check:affected.

[thymikee]

This conflicts with main now. Please rebase it so review can continue.

[thymikee]

Rebased onto main at c96a2d5 to resolve the conflict. New head a542688.

Conflicts in:

• contracts/src/screenshot.ts, contracts/src/backend.ts, contracts/src/client-capture.ts, contracts/src/interactor-types.ts, command-runtime runtime types and runtime/screenshot.ts, command-surface screenshot metadata and options test: main added screenMask next to the density options. Both are kept; the native-density flag, option and key sit beside it.
• daemon/src/execution/screenshot-runtime.ts: kept main's assertSupportedScreenshotCaptureBackend call and the native-density argument to the density check; screenMask is passed to the backend next to the density.
• platform-apple/src/core/screenshot.ts: main reworked the simulator capture flow (status bar holder, runner backend choice, screenMask argument). The density note from this PR is re-applied on top of that flow.

One small added commit, "chore: adapt to main after rebase": the Revyl iOS screenshot now skips the density resample for a native capture (the type is now number | 'native'), and the status-bar concurrency test stub returns undefined from normalizeDensity.

No other changes.

[thymikee]

a542688 is the rebase onto main. The earlier conflict is gone, but two routes still admit --native-density without reporting a scale, and the earlier test and live-run requests are still open. CI is green, but it does not run the repo gates the earlier review named (format, closure budget, wire ledger), so it does not cover the known format failures in the density files. Before merge, --native-density must be admitted only where the scale is reported, the regression tests must be added, and a live run at the new head must be posted.

On an iOS simulator, screenshot --native-density --capture-backend runner now reaches the runner branch in platform-apple/src/interactor.ts. That branch returns {} with no facts, so readScreenshotResultMetadata gets no sourcePixelDensity and returns only width and height (screenshot-density.ts#L76). The user gets a native capture without pixelDensity or logical size, which is the data the PR offers for tap mapping. Main's assertSupportedScreenshotCaptureBackend at screenshot-runtime.ts:339 refuses runner with --pixel-density, but it does not cover screenshotNativeDensity. The rule should be: a native request is admitted only on an owner that reports the scale of the pixels it keeps. Please apply that rule to the refusal, its message and the captureBackend field help, and add a daemon-route test for the refused combination.

Revyl iOS has the same gap. A Revyl iOS device is built as an Apple simulator (device.ts), so the daemon admits --native-density. RevylInteractor.screenshot returns void (interactor.ts#L310), so no sourcePixelDensity reaches the daemon. The native branch also skips the provider-viewport-unavailable check that the numeric path makes. The result is native pixels with no scale, and the flag is accepted instead of refused. Revyl already computes the scale in normalizeRevylIosDensity. Either return { sourcePixelDensity } from that same computation, keeping the missing-screen refusal, and add a native case next to interactor.test.ts:549, or refuse native for Revyl through the owner's capability declaration.

The earlier test request is still unanswered. The head adds screenshot-density.test.ts (which feeds density to the helpers), a flag-projection test, and a return-value change in a stub. No test drives screenshotIos with pixelDensity: "native" through the real normalizeIosSimulatorScreenshotDensity, and none drives the daemon route from screenshotNativeDensity to the result. Deleting the early return at screenshot.ts:502, or the translations at screenshot-runtime.ts:387-388 and :395 and runtime/screenshot.ts:49, leaves every added test green. A change that resamples a native capture back to 1x would pass CI. Please add the two cases the earlier review named: native on a 3x stub in screenshot-simulator-capture.test.ts keeps the PNG size and returns sourcePixelDensity 3, and one daemon-route case in the screenshot-runtime fixture. Confirm each goes red when the native branch or the translation is removed. Please add the Revyl and runner-backend cases too.

The only live run is in the PR body for the pre-rebase head, with no SHA, command or output. This head re-applies the density change onto main's reworked simulator flow, which is device-facing. Please post a run at a542688 on a booted iPhone simulator through the default simctl route, with exact commands and JSON output. screenshot --native-density should give file dimensions equal to physical pixels (for example 1206x2622), pixelDensity 3 and logical size 402x874. Plain screenshot should stay 402x874 at density 1. --native-density --pixel-density 2 should be refused with INVALID_ARGS. After the fix above, also show --native-density --capture-backend runner refused.

Could one nativeDensity trait on ScreenshotCaptureCapabilities replace these special cases? It would be true only where the owner reports the scale of what it kept (platform-apple simctl and the runner-fallback-provider captures), and false for the runner backend branch, the macOS helper and Revyl. assertScreenshotCaptureSupport would refuse the rest with a typed reason. If the backend and interactor contracts also keep one spelling, either the boolean throughout or pixelDensity: number | 'native' end to end as proposed earlier, the four boolean-to-'native' ternary pairs go away. Then the Revyl special case, the hand-extended --capture-backend guard and the undefined-density branch all go away too, because every admitted owner reports a scale. The first decision is the single spelling, and whether the capability or the daemon device-type check owns admission.

The open inline threads still apply. Two are P2: the round-trip test for appendScreenshotScriptFlags and the undefined-density branch, which is now reachable through the runner backend and Revyl iOS, so please fix it at those owners instead of throwing generically. The four P3 threads also still apply: cliDetail and captureBackend help, nativeDensity description scope, journal projection and ScreenshotCommandResult type. None can be resolved yet.

This review traced the head source only. I did not run a simulator, the unit suite or typecheck. I traced only Revyl, so other providers such as Limrun iOS may need the same check.

[0xdaniell]

Update on head 38ec6490d2ec8157d05a57b3d430706c9e3befa8 (base 093341a60):

• The implementation restricts native-density capture to owners that report the retained pixel scale, refuses unsupported runner/provider captures, and uses a boolean consistently for the option.
• Added focused simulator-capture and daemon-route regression coverage. Removing the native-density early return caused the pixel-size test to fail (42 versus 126); removing daemon forwarding caused the metadata test to fail.
• pnpm check:affected --run --base origin/main --head HEAD passed every runnable local check at this exact commit: format, lint, typecheck, layering, fallow, build, related tests and unit checks. Unit tests: 17,667 passed, 70 skipped. GitHub-authoritative and manual checks were not run by that command.
• Fixed two test problems found during validation: excessive fake-time advancement in an Apple runner lease test, and a daemon startup race test that created an untracked second process. The latter failed at the same assertion on the unchanged base.
• The requested live iOS verification remains outstanding. The iPhone 17 Pro simulator failed to boot twice in the device host, which reported: “The simulator or emulator could not start. The device host sent no details.” Earlier live results predate the rebases and do not validate this head. The PR remains a draft pending that verification.

[thymikee]

Thanks for the update. The code looks right at 38ec649, and the replay, help, journal and result-type gaps are fixed. The live simulator run from the earlier review is still missing, so this is not ready yet.

The simctl capture path has no live run at this head. The earlier results predate two rebases onto main's reworked simulator flow, and unit stubs cannot show that a real capture keeps physical pixels or that a tap from native pixels lands. Please post a run at this commit on a booted iPhone simulator, on the default simctl route, with commands and --json output:

1. screenshot --native-density gives the physical pixels (for example 1206x2622), pixelDensity 3 and logical 402x874.
2. Plain screenshot gives 402x874 at pixelDensity 1.
3. A control's center (px, py) in the native image, then press at (px/3, py/3), with the snapshot or screen state that proves it activated.
4. --native-density --pixel-density 2 fails with INVALID_ARGS.
5. --native-density --capture-backend runner fails with INVALID_ARGS.

The unreachable-branch comment does not apply, because an admitted native request always gets a measured scale on the simctl path. These are fixed and can be resolved: replay, help text, simulator-only wording, journal field, result type.

Not blocking: commits 1edaf2e and 38ec649 fix test problems unrelated to screenshot density, so a separate PR would keep this one focused.

[thymikee]

This branch now conflicts with main at 38ec649. Please rebase or merge main and resolve it. The live evidence my last review asked for still applies after the rebase.