fix: harden combined text review and MCP privacy - #154
Merged
Merged
Conversation
`copy()` inherited the previous attempt's outcome. Once a clip turned protected mid-review, the red "remove unavailable or incompatible clips" line stayed up after the user removed it, and the orange "review and copy again" line never cleared. Both reset at the start of every attempt now, the way ManualOCRReview.run() already does. The same function also returned silently when the durable store was missing, leaving an enabled Copy button that did nothing and said nothing; it reports the failure instead. `composed` joins every part (a documented 1 MiB ceiling) and body read it three times — for the preview, for the error condition and for the Copy button's disabled state — so typing a custom separator re-joined the whole document three times per keystroke. It is hoisted into one value per render. The preview ScrollView carried a bare `.accessibilityIdentifier`, which propagates onto every descendant and overwrites the preview text's own element identity, so a query for `combined-text-preview` was ambiguous between the container and the text inside it. Contained first. In the UI test, the row count was snapshotted once the *first* seeded row existed, racing the fire-and-forget task that inserts three, so the later comparisons could run against a count of 1 or 2. It waits for the last seeded row. The `-ui-test-paste-sink` argument also passed the invented `copiedOnly` instead of the documented `copy-only`; the two parse the same today, which is exactly what makes the drift invisible. Verified: make lint clean, make build succeeds, and GanchoUITests/CombinedTextUITests passes locally.
johnny4young
changed the title
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
1 participant
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.
Purpose
This follow-up is worth retaining: it computes the combined string once per render, resets per-attempt feedback, contains the preview accessibility identity, and waits for all three synthetic seed rows before recording the UI test's baseline. The paste-sink argument now uses its documented
copy-onlyspelling.The missing-store branch now reports failure defensively. This is not evidence of a reachable enabled-but-inert button:
AppModel.fullStoreis immutable and initial loading already disables Copy when it is absent.Review remediation
jwt,creditCard,secret) could leave MCP in clear whenisSensitivewas false. All five tool boundaries now useClipSafePresentation.requiresMasking. Search also excludes masked kinds before SQL LIMIT, preserving useful safe results. Context-miss denials still precede the privacy veto; scope and write authorization are unchanged.mainat358dd1c1939091614e2f3cf22ecd22582c2039b4without conflicts. Screen OCR changes are preserved.Validation at
3437614bf344713db12ef81501380a347997189emake format,make lint: passed, 499 Swift files, zero violations.make test: 1,222 tests across 186 suites passed (all eight test products, not only the last product's 95 tests).GanchoUITests/CombinedTextUITests: 2 passed, 0 failed, 0 skipped; screenshots inspected. The first attempt failed at screenshot creation on a secondary display; controlled placement fixed the harness without removing screenshots or assertions.Findings triaged without speculative changes
.xLarge) with the extra custom-separator field. Both actions are visible, hittable and exercised. This does not certify every arbitrary accessibility size/localization combination.autocorrectionDisabled()controls every AppKit AutoFill configuration.No merge, release, installation, signing-policy change, or gate weakening was performed.