Repository navigation
Conversation
ADR 0031 decides that a native macOS app screenshot captures only the app's front window through ScreenCaptureKit (#3188). When the front window's WindowServer ID was missing from the shareable candidates, the helper captured the app's largest window instead, so the image could show a window the user was not looking at. The helper now selects the front window by ID or refuses with a typed reason: `window-not-found` when the app has no on-screen window, `front-window-not-shareable` when ScreenCaptureKit does not offer the front window. The shared outcomes fixture pins both reasons for Swift and TypeScript, and the host adds a hint for each. Evidence: the audit at bd4fd68 (AppWindowScreenshot.swift:21); re-verified on 4c1c786. Part of #3744.
ADR 0025 decides that only CoreDevice facts identify a panel: the sole lit panel, otherwise the `primary` panel, and never array position. When no panel reported `primary` and power did not decide one lit panel, the inventory captured the first lit or first integrated panel in list order. That inventory now names no panel, and capture-target resolution refuses with COMMAND_FAILED, reason `capture-panel-unidentified`, and a hint. The sole-lit-panel path and the primary-panel path are unchanged. The foldable help topic states the refusal. Evidence: the audit at bd4fd68 (display-inventory.ts:261); re-verified on 4c1c786. Part of #3744.
There was a problem hiding this comment.
3 issues found across 10 files
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="src/commands/system/index.ts">
<violation number="1" location="src/commands/system/index.ts:281">
P3: The help text says the lit panel is always the only capturable one, but the sentence added here allows the capture to name the primary panel when power is ambiguous, and that panel may be dark and return an all-black PNG. State that the fallback can be the dark panel and that the pose must be checked before trusting the capture.</violation>
</file>
<file name="packages/platform-apple/src/os/macos/helper.test.ts">
<violation number="1" location="packages/platform-apple/src/os/macos/helper.test.ts:250">
P2: This mock ignores the helper arguments, so the test passes even if the native app screenshot omits `--bundle-id com.apple.TextEdit` or uses the wrong surface. Assert the expected helper argv here to pin app-target routing.</violation>
</file>
<file name="packages/platform-apple/src/os/macos/helper.ts">
<violation number="1" location="packages/platform-apple/src/os/macos/helper.ts:157">
P3: The `front-window-not-shareable` hint tells users to retry, but the excluded-from-capture cause will not clear on retry. Name that cause as a non-retryable remedy, or split the hint so the excluded-window case does not suggest retrying.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| test('an app screenshot that cannot reach the front window refuses with its reason and a hint', async () => { | ||
| const provider = createLocalAppleToolProvider({ | ||
| macosHelper: { | ||
| run: async () => ({ |
There was a problem hiding this comment.
P2: This mock ignores the helper arguments, so the test passes even if the native app screenshot omits --bundle-id com.apple.TextEdit or uses the wrong surface. Assert the expected helper argv here to pin app-target routing.
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/platform-apple/src/os/macos/helper.test.ts, line 250:
<comment>This mock ignores the helper arguments, so the test passes even if the native app screenshot omits `--bundle-id com.apple.TextEdit` or uses the wrong surface. Assert the expected helper argv here to pin app-target routing.</comment>
<file context>
@@ -241,6 +244,45 @@ test('macOS helper screenshot argv carries only --out and --surface', async () =
+test('an app screenshot that cannot reach the front window refuses with its reason and a hint', async () => {
+ const provider = createLocalAppleToolProvider({
+ macosHelper: {
+ run: async () => ({
+ exitCode: 1,
+ stdout: JSON.stringify({
</file context>
|
|
||
| Screens are handled for you: | ||
| Each iOS simulator capture resolves the CoreDevice display table, captures the lit panel explicitly, and normalizes density with that panel's own point scale. Do not add a screen flag to the normal loop; there is none, because the lit panel is always the only capturable one: the dark panel yields an all-black PNG. | ||
| Each iOS simulator capture resolves the CoreDevice display table, captures the lit panel explicitly, and normalizes density with that panel's own point scale. Do not add a screen flag to the normal loop; there is none, because the lit panel is always the only capturable one: the dark panel yields an all-black PNG. When panel power names no single lit panel, the capture names CoreDevice's primary panel; with no primary panel either, it fails with COMMAND_FAILED and reason capture-panel-unidentified rather than pick a panel by list order. |
There was a problem hiding this comment.
P3: The help text says the lit panel is always the only capturable one, but the sentence added here allows the capture to name the primary panel when power is ambiguous, and that panel may be dark and return an all-black PNG. State that the fallback can be the dark panel and that the pose must be checked before trusting the capture.
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 src/commands/system/index.ts, line 281:
<comment>The help text says the lit panel is always the only capturable one, but the sentence added here allows the capture to name the primary panel when power is ambiguous, and that panel may be dark and return an all-black PNG. State that the fallback can be the dark panel and that the pose must be checked before trusting the capture.</comment>
<file context>
@@ -278,7 +278,7 @@ export const foldableHelpTopic = {
Screens are handled for you:
- Each iOS simulator capture resolves the CoreDevice display table, captures the lit panel explicitly, and normalizes density with that panel's own point scale. Do not add a screen flag to the normal loop; there is none, because the lit panel is always the only capturable one: the dark panel yields an all-black PNG.
+ Each iOS simulator capture resolves the CoreDevice display table, captures the lit panel explicitly, and normalizes density with that panel's own point scale. Do not add a screen flag to the normal loop; there is none, because the lit panel is always the only capturable one: the dark panel yields an all-black PNG. When panel power names no single lit panel, the capture names CoreDevice's primary panel; with no primary panel either, it fails with COMMAND_FAILED and reason capture-panel-unidentified rather than pick a panel by list order.
The two panels are different sizes (iPhone Duo: 466x678 points closed on the outer panel, 669x951 open on the inner). A pose change therefore invalidates every ref and coordinate. Re-snapshot after any pose change and never carry coordinates or refs across one.
Check which panel is lit before trusting a geometry claim: agent-device screenshot reports its point size, and 466x678 versus 669x951 says which panel you captured.
</file context>
| Each iOS simulator capture resolves the CoreDevice display table, captures the lit panel explicitly, and normalizes density with that panel's own point scale. Do not add a screen flag to the normal loop; there is none, because the lit panel is always the only capturable one: the dark panel yields an all-black PNG. When panel power names no single lit panel, the capture names CoreDevice's primary panel; with no primary panel either, it fails with COMMAND_FAILED and reason capture-panel-unidentified rather than pick a panel by list order. | |
| Each iOS simulator capture resolves the CoreDevice display table, captures the lit panel explicitly, and normalizes density with that panel's own point scale. Do not add a screen flag to the normal loop; there is none, because the lit panel is the only capturable one when panel power decides it: the dark panel yields an all-black PNG. When panel power names no single lit panel, the capture names CoreDevice's primary panel, which may be dark, so check the pose with UIHinge.status before trusting it; with no primary panel either, it fails with COMMAND_FAILED and reason capture-panel-unidentified rather than pick a panel by list order. |
| 'front-window-not-shareable': | ||
| 'The front window may still be opening or closing, or the app excludes it from screen capture. Retry after the window settles.', |
There was a problem hiding this comment.
P3: The front-window-not-shareable hint tells users to retry, but the excluded-from-capture cause will not clear on retry. Name that cause as a non-retryable remedy, or split the hint so the excluded-window case does not suggest retrying.
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/platform-apple/src/os/macos/helper.ts, line 157:
<comment>The `front-window-not-shareable` hint tells users to retry, but the excluded-from-capture cause will not clear on retry. Name that cause as a non-retryable remedy, or split the hint so the excluded-window case does not suggest retrying.</comment>
<file context>
@@ -142,6 +142,21 @@ export const MACOS_HELPER_REFUSAL_REASONS = [
+const MACOS_SCREENSHOT_REFUSAL_HINTS: Record<MacOsScreenshotRefusalReason, string> = {
+ 'window-not-found':
+ 'Open or unminimize a window of the app on the current Space, then retry the screenshot.',
+ 'front-window-not-shareable':
+ 'The front window may still be opening or closing, or the app excludes it from screen capture. Retry after the window settles.',
+};
</file context>
| 'front-window-not-shareable': | |
| 'The front window may still be opening or closing, or the app excludes it from screen capture. Retry after the window settles.', | |
| 'front-window-not-shareable': | |
| 'The front window may still be opening or closing. If the app excludes its window from screen capture, retrying will not help.', |
Size Report
Startup median (7 runs, lower is better):
|
|
|
The Swift change looks right, but the native-backend screenshot route has no live evidence yet, so it needs one run on a real host before merge (reviewed at dec7c9b). AppWindowScreenshot.swift:19 changes the native-backend route for every macOS app capture. When there is no front window ID, or ScreenCaptureKit does not list the front ID, it now throws. It used to return the largest window. If a live app has a CGWindowList front window that SCShareableContent drops (for example a zero-area or transient layer-0 window), every screenshot of that app would now fail. No fixture can show whether that happens, and no live run reached this route because the author's host lacks Screen Recording permission. Could you run it on a host with Screen Recording granted? Set I did not run the Swift or Vitest suites. The regression value of the Swift tests relies on the author's planted-fallback report, because Smoke Tests failed in the iOS runner suite on RunnerTests.testSynthesizedReplacementLeavesAMaskedFieldUnverified (RunnerTests+SynthesizedTextEntryTests.swift:169), where the masked-field bullet count was 13 instead of 8. This PR changes nothing under apple/runner or the text-entry route, so the failure looks unrelated. No conflicts. Of the open inline threads, the two docs-wording notes still apply: system/index.ts:281 (lit panel wording) and helper.ts:157-158 (retry hint wording). The helper.test.ts:247 thread does not apply: the PR leaves screenshot argv unchanged, and the existing argv test owns that shape. You can resolve it. Before merge, this PR needs the live native-backend evidence above: a front-window PNG from on-screen TextEdit, and the window-not-found refusal with the hint when every TextEdit window is minimized. |
Summary
window-not-foundandfront-window-not-shareable. A shared fixture pins them for Swift and TypeScript, and the host adds a hint.COMMAND_FAILED, reasoncapture-panel-unidentified, when no panel is primary and power does not identify one lit panel. Before, it picked a panel by list order. The sole-lit-panel and primary-panel paths are unchanged. Simulator video recording uses the same resolver.10 files. Help and user docs are updated. ADR text is unchanged. Part of #3744.
Validation
dec7c9ba41:pnpm check:affected --runpassed withexactCommit=true(format, lint, typecheck, layering, fallow, build, vitest-related, docs).pnpm test:macos-helper: 11 passed.display-inventory.test.ts: 2 failed, 35 passed, then 37 passed.helper.test.ts: 1 failed, 10 passed, then 11 passed.CGPreflightScreenCaptureAccess()is false in the agent host and in Ghostty. The case where the front window is missing could not be provoked live.macOS / Smoke TestsCI job covers the Swift helper build.Live checks before merge
An independent Codex review found no code findings. These live checks are still open, because this agent host has no Screen Recording permission and no iOS 27.1 simulator runtime: