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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed 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.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
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. |
Choose the first on-screen normal-layer app window with positive alpha and nonempty bounds. Keep interaction window selection unchanged and refuse if an eligible front window is absent from ScreenCaptureKit. Review: #3761 (comment)
|
Follow-up head:
Previous-head CI: only iOS Smoke Tests failed, at the unchanged masked-field assertion (13 bullets vs 8). PR #3722 has the identical failure. That runner XCTest exercises neither changed capture route. Android/Linux/macOS smoke, Coverage, Integration, package/guards, docs/preview, CodeQL, Size, and cubic passed; the fixture-cache matrix job was skipped. New-head CI is pending. Live check remains with maintainer: preflight is false; native TextEdit screenshot returned |
|
Live macOS evidence at Setup:
Not run: a sheet or popover, a fullscreen Space, and the iOS 27.1 iPhone Duo (this host has no iOS 27.1 runtime). Cleanup: test documents closed, |
|
Addressed all three follow-up findings in separate commits: M1 At Live on this Mac with Prior-head CI is green; new-head CI is pending. |
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
You’re at about 96% 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.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Share the on-screen window-list query while preserving nil versus empty point refusals. Pin layer-8 alert exclusion at the shared selection boundary. Review: #3761 (comment)
|
Round 4 head:
Validation: exact-head affected gate passed ( Live native TextEdit, rebuilt helper: 1312×844 PNG visibly captures New-head CI pending; monitoring for up to ~40 minutes. No merge or approval. |
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
You’re at about 96% 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="apple/macos-helper/Sources/AgentDeviceMacOSHelper/BackgroundInteraction.swift">
<violation number="1" location="apple/macos-helper/Sources/AgentDeviceMacOSHelper/BackgroundInteraction.swift:515">
P3: The `?? []` default turns a failed window-list read into "no front window", so screenshot and scroll report `window-not-found`. That tells the caller to open a window, which is wrong when the read itself failed. `requireAppWindowPoint` already keeps this case distinct, so the two paths now disagree. Pass the optional through so an unreadable list is refused with its own reason, or document the collapse if it is intended.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| as? [[String: Any]] | ||
| } | ||
|
|
||
| func frontWindow(pid: pid_t, windowInfo: [[String: Any]] = appWindowInfo() ?? []) -> OnScreenWindow? { |
There was a problem hiding this comment.
P3: The ?? [] default turns a failed window-list read into "no front window", so screenshot and scroll report window-not-found. That tells the caller to open a window, which is wrong when the read itself failed. requireAppWindowPoint already keeps this case distinct, so the two paths now disagree. Pass the optional through so an unreadable list is refused with its own reason, or document the collapse if it is intended.
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 apple/macos-helper/Sources/AgentDeviceMacOSHelper/BackgroundInteraction.swift, line 515:
<comment>The `?? []` default turns a failed window-list read into "no front window", so screenshot and scroll report `window-not-found`. That tells the caller to open a window, which is wrong when the read itself failed. `requireAppWindowPoint` already keeps this case distinct, so the two paths now disagree. Pass the optional through so an unreadable list is refused with its own reason, or document the collapse if it is intended.</comment>
<file context>
@@ -509,13 +507,17 @@ func onScreenWindows(pid: pid_t, windowInfo: [[String: Any]]) -> [OnScreenWindow
+ as? [[String: Any]]
+}
+
+func frontWindow(pid: pid_t, windowInfo: [[String: Any]] = appWindowInfo() ?? []) -> OnScreenWindow? {
+ onScreenWindows(pid: pid, windowInfo: windowInfo).first
}
</file context>
This branch has not been deployed
1 participant
Summary
snapshotremedy.capture-panel-unidentifiedwhen neither panel power nor primary identity selects a panel. Help warns that an ambiguous primary panel may be dark.12 files. Part of #3744. ADR 0031 and modal-panel eligibility remain outside this PR.
Validation
5e16f816b5:pnpm check:affected --run --head 5e16f816b5passed (exactCommit=true,checkoutChanged=false) (receiptrun-3OxBK9).pr3761-r4-front.txtand its unique marker, with the existing Untitled document behind it. Test document and isolated sessionpr3761-r4closed; daemon stopped. State:/private/tmp/agent-device-pr3761-r4.