Repository navigation
AppSnap: attach the window's accessibility text beside the PNG (#1531) - #1540
Conversation
|
Leaving this one open, as a draft, while the rest of #1531 merged. Before it lands:
🤖 Generated with Claude Code |
9d58f81 to
69a22af
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69a22af389
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (var i = 0; i < wins.length && want && !win; i++) { | ||
| if (text(wins[i], "AXTitle") === want) win = wins[i]; | ||
| } | ||
| if (!win) win = attr(app, "AXFocusedWindow") || wins[0]; |
There was a problem hiding this comment.
When an application has multiple windows with duplicate titles, this selects the first match; when the desktop-capturer title differs from AXTitle, it silently selects the focused or first window. The resulting window-text.txt can therefore contain unrelated, potentially sensitive text from another window beside the requested screenshot. Disambiguate using the captured window ID, or skip the text rather than attaching a fallback window.
Useful? React with 👍 / 👎.
| void onCaptureSnapWindowText?.(sourceId).then((res) => { | ||
| if (!isLiveScreenshotHandoff(originThreadId, generation)) return; | ||
| setSnapNote(res.skipped ?? null); | ||
| if (res.attachment) { | ||
| deliverIncomingAttachment(originThreadId, generation, res.attachment, true); | ||
| } |
There was a problem hiding this comment.
If the user submits after the PNG chip appears but before the accessibility read finishes, submission clears the composer attachments without changing this generation. The callback then adds window-text.txt to the now-empty composer, so the screenshot's prompt is sent without its text and the stale text can be included in the user's next unrelated prompt. Track the originating composer submission or otherwise prevent a late result from being delivered after its screenshot was consumed.
Useful? React with 👍 / 👎.
On macOS an AppSnap capture now also walks the window AX tree with a JXA script (osascript, 5 s timeout, 3000-node cap), flattens it into indented role "text" lines capped at 20k chars, and attaches it as window-text.txt. A missing Accessibility permission or failed read keeps the PNG and returns a one-line textSkipped reason. Other platforms skip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1531) The accessibility read ran inside captureWindow, so the screenshot chip waited up to the 5 s osascript timeout. Split it: captureWindow is PNG-only again, and a new attachments.captureWindowText runs after the PNG is attached and appends window-text.txt when it is ready. A skipped read (no Accessibility permission, timeout, window gone) now shows as 'Last capture: …' in the AppSnap picker instead of only the console. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
69a22af to
f6fa79c
Compare
Part of #1531.
On macOS, an AppSnap capture now also attaches
window-text.txt: the window's accessibility (AX) tree, flattened to indentedrole "text"lines and capped at 3000 nodes and 20k characters. The tree is read with a JXA script throughosascript, with a 5 s timeout. Other platforms are unchanged.The screenshot never waits on the text.
attachments.captureWindowis PNG-only, exactly as on main. A separateattachments.captureWindowTextcall runs after the PNG chip has landed and the picker has closed. When it finishes, it appends the text chip to whatever is pending in the composer rather than replacing it.A skipped read is visible. When there's no Accessibility permission, or the read times out or fails, or the window is gone, the AppSnap picker shows
Last capture: Window text skipped: …the next time it opens, including how to grant access.Not yet tried against a real window. Neither shell here has Accessibility permission, so the JXA walker is covered only by stubbed tests with a canned tree. Because of the split, the worst case of a bad AX read is the skip note. The screenshot path is identical to main.
Tests
electron/test/appsnap.test.jscovers flattening, truncation, the skip reasons and off-macOS.test/screenshotThreadOwnership.test.tsxchecks that the PNG lands while the text read is pending, that the text chip joins it, and that a skip note shows in the picker.answerSecret/setFastalready on main and fixed in Fix main: classify answerSecret and setFast in the web scope table #1553.🤖 Generated with Claude Code