Repository navigation
feat(agents): add AGENTS.md knowledge system and rnmacos-organize-knowledge skill - #3157
Conversation
|
Copilot Could you locally use Matt Pococks retro skill, go through my comments on PRs (in particular, ones doing the big version updates) and make some suggestions (as comments on this PR or the other one Jason has open) for more knowledge? |
The retro skill wasn’t installed in this environment, so I read Matt Pocock’s public
The first looks like a new topic; the second could be a small addition to |
Saad Najmi (Saadnajmi)
left a comment
There was a problem hiding this comment.
Left some comments, mostly ok. I feel I won't "feel" the workflow till I start using it so I'm more wanting to just try it and iterate I think
…wledge skill Add a structured, validated knowledge system for coding agents: - AGENTS.md is the single instruction entry point; its fork content lives in one [macOS] block so upstream AGENTS.md changes merge cleanly. .github/copilot-instructions.md only points to it. - .agents/knowledge/ holds one topic per concern (fork, tooling, testing, process, evals), a taxonomy.yaml, and a generated index.md. - .claude/skills/rnmacos-organize-knowledge keeps new instructions placed, deduplicated, and indexed, with detail in references/. - .agents/knowledge/tools/knowledge.mts checks structure, links, source paths, yarn commands, duplicates, budgets, and fork-block rules, and generates the index. CI runs it, its tests, and markdownlint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…inks Skills now live in .agents/skills, which Copilot and Codex both discover. .claude/skills/<name> is a relative symlink for Claude Code, and KC-08 requires that link and rejects real files or dangling links there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
daff2ab to
3ef471e
Compare
<!-- 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: > Stacked on #3157. Review only the top commit (`fd13876983`). macOS accessibility in this fork depends on AppKit behavior that isn't obvious from headers. For example, a getter can return `AXUnknown` while 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.md` row for accessibility, focus, or Tab-order work: - **`a11y.appkit`:** AppKit AX behavior observed out of process. Rules: - Never copy an unset role. - Read a control's default role from its cell. - Gate state attributes. - Target the AX element. - Verify out of process. - **`a11y.roles`:** Role resolution in Paper and in Fabric, side by side. Rules: - Resolve `role`, then `accessibilityRole`, then the element's default role. - Restore the default role when both props are unset. - Don't import Paper's `RCTConvert` into Fabric. - **`a11y.state`:** How `accessibilityState` maps to AX attributes and value words. Rules: - Stop advertising state that's been removed. - Advertise `AXPress` only 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 on `main`. - **`testing.ax-probes`:** A procedure with a small Swift AX probe. It dumps elements by `testID`, reports focus, and clicks Fabric views, all driven with `osascript` keys. 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 start` fails under the repository's `nodeLinker: pnpm`, because Yarn resolves the `react-native-macos` binary under `.yarn/__virtual__`, which the pnpm linker never creates. Running `cli.js` directly without `NODE_PATH` starts Metro without its `/status` route, 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):** | Command | Result | | --- | --- | | `node .agents/knowledge/tools/knowledge.mts check --ci` | Passed: `26 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) | **Fact verification:** - Checked every code claim against `main`. The `asserts` front matter keeps key lines tied to source, and I confirmed it's enforced: a deliberately broken assert produced a warning. - Verified AppKit claims with the probe against small standalone AppKit apps: `NSView`, `NSTextField`, and `NSTextView`, before and after setting roles and state. **Procedure verification in RNTester-macOS on `main`:** | Step | Result | | --- | --- | | `RCT_NEW_ARCH_ENABLED=1 pod install` (Homebrew CocoaPods 1.17.0; local Ruby couldn't `bundle install`), then `.ado/scripts/xcodebuild.sh packages/rn-tester/RNTesterPods.xcworkspace macosx Debug RNTester-macOS build` | `Build Succeeded` | | `NODE_PATH="$PWD/node_modules" node ../react-native/cli.js start` from `packages/rn-tester`, then `curl localhost:8081/status` | `packager-status:running` (without `NODE_PATH`: `Cannot GET /status`) | | `testing.ax-probes` procedure, run verbatim from a fresh launch | Navigated APIs → Accessibility; `example_search role=AXTextField enabled=1` | | `ax dump` of the TextInput module | Both `secureTextEntry` inputs report `role=AXUnknown enabled=1`, which matches the `RCTCopyBackedTextInput` fact | | Tab from `example_search` in the TextInput module, with `autorecalculatesKeyViewLoop = YES` added to RNTester's `AppDelegate` locally (not committed) | Focus alternates `AXScrollArea`, `AXTextField`, `AXScrollArea`, `AXTextField`…; Shift-Tab returns to `AXScrollArea`. This matches `a11y.keyboard` | | Same Tab test without that change | Focus stays on `example_search`, which matches the `a11y.keyboard.test-recalculated-loop` rule |
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
yarncommands 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.mdis the single instruction entry point. All fork content sits in one[macOS]block, so upstreamAGENTS.mdchanges merge cleanly..github/copilot-instructions.mdjust points to it..agents/knowledge/holds one topic per concern, each with front matter (id,summary,load_when,sources,rules). Topics are listed intaxonomy.yamland in a generatedindex.md, so agents load only the topics they need. First topics:fork.diff-tagsandfork.ownershiptooling.commandsandtooling.format-linttesting.rntester-macosprocess.pull-requestsevals.knowledge-checks.claude/skills/rnmacos-organize-knowledgekeeps new instructions placed, deduplicated, and indexed. Thernmacos-prefix avoids collisions with skills upstream may add. The skill stays short, and detail lives inreferences/..agents/knowledge/tools/knowledge.mtsgenerates the index and checks:yarncommands[macOS]fork-block rulesmicrosoft-pr.ymlruns 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:node .agents/knowledge/tools/knowledge.mts check --ci20 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-checkThe new CI steps in
microsoft-pr.ymlrun the first three commands. No native code changes, so no native build was needed.