Repository navigation
Writing: Screen Memory reads only the focused window - #1930
Merged
Merged
Conversation
Setup tells people Screen Memory "reads the window you're replying in", but full-display capture could pull text from other visible windows and apps, including apps outside the Writing app scope. Owner decision: read only the window the user is typing in. - New pure FocusedWindowCapturePolicy picks the capture target at capture time: the typing target's window, owned by the frontmost app, that app's frontmost normal window, not excluded (password managers included), in the app scope, and not contradicted by Accessibility's focused app. Anything unprovable refuses and nothing is captured. - Full-display capture, CaptureKindPolicy and multi-window attribution are gone. The AX reader reads only the app's focused window when its frame matches the chosen one; no other-window search, no unmatched fallback. - The service re-checks the app scope itself and drops held window text as soon as the user moves to another window or leaves the field. - Lock screen, secure input, the master toggle and the any-visible-window exclusion gate are unchanged.
…eason Review fixes for the focused-window change: - A nil keyboard-focus answer from Accessibility now refuses (keyboardFocusUnknown). Without it a non-activating launcher's typing could read the frontmost window behind it. - Each FocusedWindowCapturePolicy refusal logs a fixed reason string, and those plus no-target-window and target-changed are in the diagnostics allowlist, so skips no longer log as a redacted length. - Stale full-display wording in comments and test titles; ledger records the CaptureTriggerPolicy doc change and the owner decision as a deviation from Tilde.
Owner
Author
|
Independent review: writing-reviewer agent reviewed the full diff (REQUEST CHANGES: fail closed on unknown keyboard focus; allowlisted per-refusal log reasons). Both addressed in the final commit; coordinator re-checked the summary. APPROVE on green CI. Owner decision honored: Screen Memory reads only the focused window. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Screen Memory now reads only the window you're typing in. Owner decision, before release. Setup already says "Reads the window you're replying in, on this Mac." The code didn't do that: with no conversation in the focused window it read the whole display, so text from other visible windows and apps could get in, including apps outside the Writing app scope.
How
FocusedWindowCapturePolicy(Sources/TranscriptedWriting/Core/ScreenMemory/). At capture time it picks the typing target's window, or refuses. It captures only when all of these hold:ScreenCaptureService: full-display capture,CaptureKindPolicyand the second window-snapshot slot are gone. It's the AX read orSCContentFilter(desktopIndependentWindow:)of the chosen window, and nothing else. It re-checks the app scope itself instead of trusting the app bridge alone. It also drops held text as soon as the user moves to another window or leaves the field.AXWindowTextReaderreads only the app's focused window, and only when its frame matches the chosen one. It no longer walks the app's other windows and has no unmatched fallback.KeyboardFocusProbe(new, pids only). It readsNSWorkspace.frontmostApplicationplus the system-wide AX focused application. It never sets a messaging timeout on the system-wide element, because that changes the timeout for every AX element in the process.not-frontmost-app,keyboard-focus-unknown,not-focused-window, …). Those reasons, plusno-target-windowandtarget-changed, are now inDiagnosticsMetadataRedactor's allowlist. No bundle IDs, titles, text or paths get logged.Sources/Writing/WritingController.swiftisn't touched (another PR owns the poll wiring there). Setup copy still matches, and no visuals changed.What's lost
referenceSnippetsfrom other windows. By design, that was exactly the full-display read.keyboard-focus-unknownin the log). Transcripted already holds Accessibility for paste-back, so shipped installs should be fine. A dev build whose TCC grant broke on re-signing will just go quiet.Tests
FocusedWindowCapturePolicyTests, one row per case: focused window in scope, other apps visible (incl. in front of the typing app), keyboard focus elsewhere, keyboard focus unknown, back window of the same app, off-screen sibling, focused window excluded, password manager with an empty list, out of scope, no identifiable window (no target, missing id/pid, gone, unknown frontmost, unranked, no bundle), owner mismatch, overlay layer.ScreenCaptureServiceTests: held text follows the typing window. Refusal → outcome mapping, plus every refusal reason is distinct, never contains the bundle ID, and survives the redactor literally.DiagnosticsMetadataRedactorTestsupdated in lockstep.?? true). The other one (<→<=) is equivalent, since two windows can't share a z-rank.swift test --filter TranscriptedWritingTestson the final head (covers ScreenMemory and DiagnosticsMetadataRedactor): 925 tests in 111 suites, green.bash check.shpassed ond78ee347, the head before the review fixes: build.sh, run-tests, test-shape, concurrency census, build-deps, integration smoke, fullswift test, source pins and doc paths. It wasn't rerun on the final head (release timing). The review-fix commit only touches ScreenMemory, the redactor allowlist, tests, comments and docs, so CI covers the rest.Not proven here (manual): live capture on a real display. That covers Slack/Messages in front with another app visible, a launcher panel over an in-scope app, and AX revoked. Synthetic tests can't cover ScreenCaptureKit or real focus.
Review
Independent review by a separate reviewer thread came back REQUEST CHANGES (small):
All addressed in the second commit.
🤖 Generated with Claude Code