Repository navigation
feat(agents): add macOS accessibility knowledge topics - #3158
Conversation
1cf8630 to
c64c608
Compare
Saad Najmi (Saadnajmi)
left a comment
There was a problem hiding this comment.
My first look feels like this is quite specific to the PR we submitted, which makes me worry how fast this might grow if we do a similar thing for every PR. Ex: It seems very focused on TextInput, because that PR was probably fixing some TextInput bugs
c64c608 to
eae48a3
Compare
…wledge skill (#3157) <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The four fields below are mandatory. --> <!-- This fork of react-native provides React Native for macOS for the community. It also contains some changes that are required for usage internal to Microsoft. We are working on reducing the diff between Facebook's public version of react-native and our microsoft/react-native-macos fork. Long term, we want this fork to only contain macOS concerns and have the other iOS and Android concerns contributed upstream. If you are making a new change then one of the following should be done: - Consider if it is possible to achieve the desired behavior without making a change to microsoft/react-native-macos. Often a change can be made in a layer above in facebook/react-native instead. - Create a corresponding PR against [facebook/react-native](https://github.com/facebook/react-native) **Note:** Ideally you would wait for Facebook feedback before submitting to Microsoft, since we want to ensure that this fork doesn't deviate from upstream. --> ## Summary: Coding agents working in this fork have no shared, fork-specific guidance. Nothing tells them how to tag fork diffs for clean upstream merges, which files the fork owns, how CI builds RNTester-macOS, or which `yarn` commands to run. Ad hoc instruction files also tend to duplicate and contradict each other over time. This PR adds a small, validated knowledge system: - **`AGENTS.md`** is the single instruction entry point. All fork content sits in one `[macOS]` block, so upstream `AGENTS.md` changes merge cleanly. `.github/copilot-instructions.md` just points to it. - **`.agents/knowledge/`** holds one topic per concern, each with front matter (`id`, `summary`, `load_when`, `sources`, `rules`). Topics are listed in `taxonomy.yaml` and in a generated `index.md`, so agents load only the topics they need. First topics: - `fork.diff-tags` and `fork.ownership` - `tooling.commands` and `tooling.format-lint` - `testing.rntester-macos` - `process.pull-requests` - `evals.knowledge-checks` - **`.claude/skills/rnmacos-organize-knowledge`** keeps new instructions placed, deduplicated, and indexed. The `rnmacos-` prefix avoids collisions with skills upstream may add. The skill stays short, and detail lives in `references/`. - **`.agents/knowledge/tools/knowledge.mts`** generates the index and checks: - structure and front matter - links and source paths - `yarn` commands - duplicate rules - size budgets - `[macOS]` fork-block rules `microsoft-pr.yml` runs the checker, its tests, and markdownlint. This is agent tooling specific to the fork, so there's no corresponding facebook/react-native PR. It's the base of a stack; #3158 builds the macOS accessibility topics on it. ## Test Plan: Run at the head of this branch (`447e205e55`), with Node 24.18.0: | Command | Result | | --- | --- | | `node .agents/knowledge/tools/knowledge.mts check --ci` | Passed: `20 files checked: 0 errors, 0 warnings.` | | `node --test .agents/knowledge/tools/knowledge.test.mts` | Passed: 21 pass, 0 fail | | `yarn markdownlint-cli2 --no-globs AGENTS.md ".agents/knowledge/**/*.md" ".claude/skills/rnmacos-*/**/*.md" .github/copilot-instructions.md` | Passed: `Summary: 0 error(s)` | | `yarn format-check` | Passed (exit 0) | The new CI steps in `microsoft-pr.yml` run the first three commands. No native code changes, so no native build was needed.
Add verified a11y topics (appkit, roles, state, text-input, keyboard) and a testing.ax-probes procedure with an out-of-process AX probe. Route accessibility work from AGENTS.md. Fix the RNTester Metro command, since `yarn start` fails under the pnpm linker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eae48a3 to
f891c50
Compare
That's actually ok. The goal of the knowledge system is that it builds it up in a way that is structured and progressively loaded. As we fix issues and add skills the knowledge base gets bigger and agents get smarter. It's ok to have 20 different a11y topics, as long as they are only loaded when needed. That's the idea of the file budgeting. |
Summary:
macOS accessibility in this fork depends on AppKit behavior that isn't obvious from headers. For example, a getter can return
AXUnknownwhile AX clients see a different role. Paper and Fabric also resolve roles differently. Agents and contributors keep rediscovering these details, or they copy iOS patterns that don't hold on macOS. This PR records the verified behavior as knowledge topics. It also adds a repeatable way to check what VoiceOver and other AX clients actually see.New topics, loaded from a new
AGENTS.mdrow for accessibility, focus, or Tab-order work:a11y.appkit: AppKit AX behavior observed out of process. Rules:a11y.roles: Role resolution in Paper and in Fabric, side by side. Rules:role, thenaccessibilityRole, then the element's default role.RCTConvertinto Fabric.a11y.state: HowaccessibilityStatemaps to AX attributes and value words. Rules:AXPressonly when the element handles it.a11y.text-input:editable={false}semantics on every platform, plus the single-line (RCTUITextField) and multiline (RCTWrappedTextView) backings.a11y.keyboard:focusable, first responder, and the key view loop, including why a Fabric TextInput takes two Tab stops onmain.testing.ax-probes: A procedure with a small Swift AX probe. It dumps elements bytestID, reports focus, and clicks Fabric views, all driven withosascriptkeys.The topics state facts and the open questions in #3136 without recommending how to resolve them. They watch #3136, so they'll be updated when it lands.
This PR also corrects
testing.rntester-macos.yarn startfails under the repository'snodeLinker: pnpm, because Yarn resolves thereact-native-macosbinary under.yarn/__virtual__, which the pnpm linker never creates. Runningcli.jsdirectly withoutNODE_PATHstarts Metro without its/statusroute, so the app reports "No script URL provided". The topic now gives a command that works.Knowledge files only, with no product code changes, so there's no corresponding facebook/react-native PR.
Test Plan:
Environment: macOS 27.2 (26B123), Xcode 26.4.1, Node 24.18.0. Run at
fd13876983.Knowledge checks (the steps CI runs):
node .agents/knowledge/tools/knowledge.mts check --ci26 files checked: 0 errors, 0 warnings.node --test .agents/knowledge/tools/knowledge.test.mtsyarn markdownlint-cli2 --no-globs AGENTS.md ".agents/knowledge/**/*.md" ".claude/skills/rnmacos-*/**/*.md" .github/copilot-instructions.mdSummary: 0 error(s)yarn format-checkFact verification:
main. Theassertsfront matter keeps key lines tied to source, and I confirmed it's enforced: a deliberately broken assert produced a warning.NSView,NSTextField, andNSTextView, before and after setting roles and state.Procedure verification in RNTester-macOS on
main:RCT_NEW_ARCH_ENABLED=1 pod install(Homebrew CocoaPods 1.17.0; local Ruby couldn'tbundle install), then.ado/scripts/xcodebuild.sh packages/rn-tester/RNTesterPods.xcworkspace macosx Debug RNTester-macOS buildBuild SucceededNODE_PATH="$PWD/node_modules" node ../react-native/cli.js startfrompackages/rn-tester, thencurl localhost:8081/statuspackager-status:running(withoutNODE_PATH:Cannot GET /status)testing.ax-probesprocedure, run verbatim from a fresh launchexample_search role=AXTextField enabled=1ax dumpof the TextInput modulesecureTextEntryinputs reportrole=AXUnknown enabled=1, which matches theRCTCopyBackedTextInputfactexample_searchin the TextInput module, withautorecalculatesKeyViewLoop = YESadded to RNTester'sAppDelegatelocally (not committed)AXScrollArea,AXTextField,AXScrollArea,AXTextField…; Shift-Tab returns toAXScrollArea. This matchesa11y.keyboardexample_search, which matches thea11y.keyboard.test-recalculated-looprule