-
-
Notifications
You must be signed in to change notification settings - Fork 38
Suppress Calendar AX capture to prevent event popover dismissal #628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
akramj13
wants to merge
1
commit into
FuJacob:main
Choose a base branch
from
akramj13:fix/544-apple-calendar-bug
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+153
−0
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| --- | ||
| name: ship | ||
| description: Ship the current work to main via a squash-merged PR. Branches off latest main (unless a branch is given), pushes, opens a PR using the repo template, then squash-merges with admin bypass. Use when the user says "ship it", "/ship", or wants to land changes on main end-to-end. | ||
| --- | ||
|
|
||
| # /ship | ||
|
|
||
| End-to-end "land this on main" workflow for Cotabby. The goal is a **single linear | ||
| commit on main** — squash merge, never a merge commit. Cotabby's `main` ruleset | ||
| rejects merge commits, so squashing is what keeps history clean; `--admin` bypasses | ||
| the required-status-check protection so the owner can merge directly. | ||
|
|
||
| `$ARGUMENTS` is optional: | ||
| - empty → branch off the latest `origin/main` with a derived name | ||
| - a branch name (e.g. `feat/foo`) → use/create that branch instead of deriving one | ||
| - `from <ref>` → base the new branch on `<ref>` instead of `origin/main` | ||
|
|
||
| ## Steps | ||
|
|
||
| 1. **Establish the branch.** | ||
| - `git fetch origin`. | ||
| - If the user is already on a feature branch that holds the work, keep it. | ||
| - Otherwise (on `main`/detached, or a branch name was given), create the branch | ||
| off the latest base: `git checkout -b <name> origin/main` (or the `from <ref>` | ||
| base). Derive `<name>` from the change: `feat/`, `fix/`, or `chore/` + a short | ||
| kebab slug. Never do the work directly on `main`. | ||
|
|
||
| 2. **Commit the work.** Stage and commit anything pending. Follow the repo's | ||
| GitHub rules in `.Codex/AGENTS.md`: **no `Co-Authored-By` trailers.** Write a | ||
| concise, real commit message. | ||
|
|
||
| 3. **Validate before pushing.** Run the narrowest useful checks, broaden if shared | ||
| behavior changed: | ||
| ```bash | ||
| swiftlint lint --quiet | ||
| xcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build | ||
| ``` | ||
| For test-affecting changes also run `build-for-testing`. Local `test` execution | ||
| often fails on a **Team ID / signing mismatch** — that's an environment issue, not | ||
| a code failure; report it and rely on `build-for-testing` succeeding. | ||
| - **XcodeGen:** `project.yml` is the source of truth and `Cotabby.xcodeproj` is | ||
| generated. New files under `Cotabby/` and `CotabbyTests/` are auto-discovered — | ||
| no project edit needed. Only structural changes (targets, build settings, | ||
| packages, scheme) require editing `project.yml` then `xcodegen generate` and | ||
| committing the regenerated project. Fix all lint/build errors before continuing. | ||
|
|
||
| 4. **Push.** `git push -u origin <branch>`. | ||
|
|
||
| 5. **Open the PR using the repo template.** Read `.github/PULL_REQUEST_TEMPLATE.md` | ||
| and fill in **every** section — Summary (what + why), Validation (what you | ||
| actually ran and saw), Linked issues (`Fixes #N` / `Refs #N`), Risk / rollout | ||
| notes. Do not invent a format. Use a heredoc body: | ||
| ```bash | ||
| gh pr create --base main --head <branch> --title "<title>" --body "$(cat <<'EOF' | ||
| ## Summary | ||
| ... | ||
| EOF | ||
| )" | ||
| ``` | ||
|
|
||
| 6. **Confirm, then squash-merge with admin bypass.** Merging to `main` is an | ||
| irreversible outward action — show the PR URL and the one-line summary, and get an | ||
| explicit go-ahead unless the user already said to merge in this turn. Then: | ||
| ```bash | ||
| gh pr merge <branch-or-#> --squash --admin --delete-branch | ||
| ``` | ||
| `--squash` keeps main linear (no merge commit → satisfies the ruleset); | ||
| `--admin` bypasses required checks; `--delete-branch` cleans up the remote branch. | ||
|
|
||
| 7. **Sync local main.** | ||
| ```bash | ||
| git checkout main && git pull --ff-only origin main | ||
| ``` | ||
| Confirm `main` now contains the squashed commit and report the result. | ||
|
|
||
| ## Guardrails | ||
|
|
||
| - **Never force-push `main` or rebase published history to "fix" merges.** If | ||
| `origin/main` moved while you worked, integrate it (rebase the branch onto the new | ||
| `origin/main`) and re-validate — don't clobber others' commits. | ||
| - **Never delete or overwrite work you didn't create** without checking it first. | ||
| - If validation fails, stop and surface the failure — don't merge red. | ||
| - If the user named a target other than `main`, ship there instead, but keep the | ||
| squash-merge shape. |
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
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
30 changes: 30 additions & 0 deletions
30
Cotabby/Support/AccessibilityCaptureSuppressionPolicy.swift
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import Foundation | ||
|
|
||
| /// File overview: | ||
| /// Centralizes app-level exceptions where Cotabby must not inspect the focused Accessibility tree. | ||
| /// | ||
| /// Most app compatibility rules should live in the normal availability pipeline so users can still | ||
| /// choose where Cotabby runs. This policy is narrower: it protects host apps whose transient UI is | ||
| /// destabilized by AX attribute enumeration itself. The caller should consult it before any deep | ||
| /// candidate walk, because once that walk starts the host popover may already have closed. | ||
| enum AccessibilityCaptureSuppressionPolicy { | ||
| /// Bundle identifiers whose focused AX tree is not safe to enumerate continuously. | ||
| /// | ||
| /// Apple Calendar's event-detail popover can dismiss itself when Cotabby polls text capability | ||
| /// on its temporary editor hierarchy. Suppressing capture at the app boundary is conservative, | ||
| /// but it keeps Calendar's own editing controls usable while leaving keyboard monitoring and the | ||
| /// rest of Cotabby untouched. | ||
| private static let unsafeBundleIdentifiers: Set<String> = [ | ||
| "com.apple.iCal" | ||
| ] | ||
|
|
||
| /// Returns true when focus polling should stop after the cheap focused-element query and before | ||
| /// `FocusSnapshotResolver` performs AX candidate enumeration. | ||
| static func shouldSuppressCapture(bundleIdentifier: String?) -> Bool { | ||
| guard let bundleIdentifier else { | ||
| return false | ||
| } | ||
|
|
||
| return unsafeBundleIdentifiers.contains(bundleIdentifier) | ||
| } | ||
| } | ||
26 changes: 26 additions & 0 deletions
26
CotabbyTests/AccessibilityCaptureSuppressionPolicyTests.swift
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import XCTest | ||
| @testable import Cotabby | ||
|
|
||
| final class AccessibilityCaptureSuppressionPolicyTests: XCTestCase { | ||
| func testCalendarCaptureIsSuppressedByDefault() { | ||
| XCTAssertTrue( | ||
| AccessibilityCaptureSuppressionPolicy.shouldSuppressCapture( | ||
| bundleIdentifier: "com.apple.iCal" | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| func testOrdinaryAppCaptureIsNotSuppressed() { | ||
| XCTAssertFalse( | ||
| AccessibilityCaptureSuppressionPolicy.shouldSuppressCapture( | ||
| bundleIdentifier: "com.apple.Safari" | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| func testMissingBundleIdentifierIsNotSuppressed() { | ||
| XCTAssertFalse( | ||
| AccessibilityCaptureSuppressionPolicy.shouldSuppressCapture(bundleIdentifier: nil) | ||
| ) | ||
| } | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unsafeBundleIdentifiersis a private constant, so there is no way for a user to re-enable AX capture for Calendar (e.g., when they are typing in a plain text area outside the event-editor popover). The policy fires before all user-preference checks inisCaptureSuppressedForBundle, meaning even an explicit per-app user preference cannot override it. The doc comment acknowledges this is "conservative," but unlike theisApplicationDisabledpath, no corresponding UI toggle exists. Consider surfacing this as a per-app override in the accessibility settings pane, or at minimum document the intentional no-override behaviour in a follow-up issue so the decision is tracked.