Skip to content

Commit ed037cb

Browse files
fix(web-components): preserve radio state after deferred upgrade
chore(web-components): use component tag name constants in playwright tests (#36183) docs(headless): revamp docsite with Overview, Getting Started, and Accessibility pages (#36120) refactor(react-headless-components-preview): improve positioning types and performance (#36192) fix: clean up fast-element dependency hierarchy and update to 2.10.4 (#36184) fix(web-components): fix keyboard navigation regressions for tree and menu-list (#36118)
1 parent de337cf commit ed037cb

1,501 files changed

Lines changed: 6894 additions & 32311 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/review-pr/SKILL.md

Lines changed: 8 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -132,80 +132,6 @@ Check 3 lines above each match for a guard (`canUseDOM`, `typeof window !== 'und
132132
| `// @ts-ignore` without explanation | WARNING |
133133
| `any` type in new code | INFO |
134134

135-
### I. Documentation coverage
136-
137-
A code change frequently lands something that **someone downstream** needs to learn about. The "someone" splits into two audiences with very different reading habits, so this check actually walks two passes:
138-
139-
- **Pass 1 — user-facing docs.** Component consumers (storybook stories, migration guides, docsite, MDX, change-file comments).
140-
- **Pass 2 — harness / agent-facing docs.** Future agent sessions reading the skills, `AGENTS.md`, `docs/workflows/*`, and `docs/architecture/*`. When a PR changes a build target, a script, a CI step, a label taxonomy, a project-board field, or an assumption that a skill makes, the skill or harness doc has to be updated **in the same PR** — otherwise every fresh `/triage-issues`, `/visual-test`, `/review-pr`, etc. invocation pays the discovery cost over again.
141-
142-
Both passes ask the same two questions: should this PR have updated docs, and if so, did it? Bug fixes that restore documented behavior, internal refactors, and test-only PRs default to PASS in both passes — the interesting cases are below.
143-
144-
#### Pass 1 — user-facing docs
145-
146-
| Source change | Expected doc surface |
147-
| --------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- |
148-
| New prop on a public component (added in `*.types.ts`) | A Storybook story or MDX entry under `packages/react-components/react-<name>/stories/` that exercises the prop |
149-
| New public export added to a `library/src/index.ts` barrel | A story or MDX entry covering it (or a justified note in the PR body) |
150-
| New component package (new directory under `packages/react-components/react-*`) | Stories package, `library/docs/Spec.md`, and an entry on the docsite (`apps/public-docsite-v9`) |
151-
| Behavior change to a public API's defaults (e.g. a new conditional in `use*_unstable` that changes observable output) | Mention in `library/docs/MIGRATION.md` or a "BREAKING CHANGE / behavior change" section in the PR body |
152-
| Removal or deprecation of a public export | `library/docs/MIGRATION.md` entry and/or a `@deprecated` JSDoc on the symbol |
153-
| New design token, classname constant, or CSS custom property | A line in the relevant docs (often the component's stories MDX) explaining how to use/override it |
154-
155-
#### Pass 2 — harness engineering / agent-facing docs
156-
157-
This pass exists because skills are part of the contract, not just convenience. When the skills are stale, the agents are wrong — quietly. Symptoms: a `/visual-test` invocation that points at a nonexistent project name, a `/triage-issues` flow that recommends a label the repo no longer has, a `/triage-board` skill that misses a new view filter the maintainers added.
158-
159-
Walk the diff for any of these and check whether the matching agent-facing doc was updated:
160-
161-
| Source change | Expected harness update |
162-
| --------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
163-
| New / renamed nx target (`project.json`, `nx.json`, generators) | Any skill that invokes that target — `visual-test`, `lint-check`, `package-info`, `v9-component`. The skill literally types the target name; it's not auto-discovered. |
164-
| New / renamed top-level `package.json` script | Same. Plus `docs/workflows/contributing.md` if it's contributor-facing. |
165-
| New label, label rename, or label taxonomy change in the repo (`gh api repos/.../labels`) | `triage-issues/references/triage-labels.md` — the skill uses an allow-list and `gh issue edit` rejects unknown labels. |
166-
| New / changed project-board field, view filter, or option ID | `triage-board/references/team-mapping.md` (option IDs) and the view-filter mirror in `triage-board/SKILL.md`. |
167-
| New convention contributors are expected to follow (file layout, naming, "always do X") | `AGENTS.md` / `CLAUDE.md` (they're symlinked) — the rules section is the agent's first read. |
168-
| New layered dependency rule, package-tier reshuffle, or CODEOWNERS rewrite | `docs/architecture/layers.md`, `docs/team-routing.md`, plus `triage-board/references/team-mapping.md` if a CODEOWNERS handle gained or lost a confident mapping. |
169-
| New first-time-setup requirement (e.g. a workspace package whose `lib-commonjs/` must be pre-built) | `docs/workflows/contributing.md` "First-time setup" section AND the troubleshooting block of any skill that hits the failure mode (`visual-test` already does this for the unstable-deps case). |
170-
| New skill, agent hook, or `.claude/`-level configuration | `AGENTS.md` skills table + the `.claude/skills/<name>/SKILL.md` bridge file. |
171-
| Removal or move of a path that any skill greps for or hard-codes | The skill that referenced it. Search the skills (`grep -rn "<old-path>" .agents/skills/`) before merging. |
172-
| Authentication / token-scope / permission changes that affect `gh` calls | The skill's preflight section. Concrete past examples: the EMU vs non-EMU active-account gotcha and the `read:project` vs `project` scope distinction now baked into `triage-board`'s preflight. |
173-
| New external-tool name or behavior change (e.g. assignee identity for an automated agent) | The relevant skill. Concrete past example: assigning to `Copilot` literally fails with "Bot does not have access" — only `copilot-swe-agent` works. That belongs in any skill that assigns to Copilot. |
174-
175-
#### When docs are NOT expected (default to PASS)
176-
177-
- Pure bug fix that restores documented behavior
178-
- Internal refactor with no exported-symbol or behavior change
179-
- Build / CI changes that have no contributor-facing or skill-facing impact
180-
- Test-only PRs
181-
- Style-only PRs (typo, formatting) on internal code
182-
183-
#### Severity
184-
185-
| Situation | Severity |
186-
| ----------------------------------------------------------------------------------------------------------------------- | -------- |
187-
| Public export removed or behavior-changed without `MIGRATION.md` entry | BLOCKER |
188-
| Skill references a renamed or removed path / target / label that this PR changes (skill will literally fail next run) | BLOCKER |
189-
| New public component / new public hook with no story or MDX | WARNING |
190-
| New public prop with no story exercising it | WARNING |
191-
| New design token / className constant with no usage example | WARNING |
192-
| Behavior change to public defaults with no PR-body callout | WARNING |
193-
| New label / project field / nx target / convention without the matching skill or `AGENTS.md` / `docs/workflows/` update | WARNING |
194-
| New first-time-setup requirement without `contributing.md` mention | WARNING |
195-
| PR description explicitly defers docs to a follow-up (link cited) | INFO |
196-
| Docs / harness change exists but feels minimal — could be more thorough | INFO |
197-
198-
#### How to actually run this check
199-
200-
The model should walk the diff, not guess.
201-
202-
1. **Group changed files into surfaces.** Code surfaces (`library/src/**`, `index.ts` barrels, `*.types.ts`); user-doc surfaces (`stories/**`, `library/docs/**`, `apps/public-docsite-v9/**`); harness surfaces (`.agents/skills/**`, `.claude/**`, `AGENTS.md`, `CLAUDE.md`, `docs/workflows/**`, `docs/architecture/**`, `docs/team-routing.md`, `project.json`, `nx.json`, root `package.json` scripts, `.github/CODEOWNERS`, `.github/labeler.yml`).
203-
2. **If the PR is purely a doc/harness surface**, the check is PASS — it's documentation already.
204-
3. **For each row in the user-facing table that matches a code change in the diff**, verify the corresponding doc surface was also touched. If not, raise the matching severity.
205-
4. **For each row in the harness-engineering table that matches**, verify the corresponding skill or harness doc was also touched. Be specific: name the skill that needs updating, not just "a skill."
206-
5. **Cross-search**: when a PR renames a path, target, or label, run `grep -rn "<old-name>" .agents/skills/ docs/ AGENTS.md` to find references that will break. Anything that turns up is a BLOCKER unless this PR also updates it.
207-
6. **Read the PR body** before reporting. Authors often pre-empt with "docs follow-up tracked in #NNNN" or "harness skills updated in commit X" — those move findings from WARNING to INFO when the deferral is explicit and reasonable.
208-
209135
## Phase 4: Calculate Confidence Score
210136

211137
```
@@ -232,11 +158,16 @@ Score interpretation:
232158

233159
## Phase 5: Produce Output
234160

235-
Use this exact format (also used verbatim as the PR-comment body in Phase 6 — don't duplicate work).
236-
237-
Start directly with the score. Skip a header title, author, type, packages-affected, and CI-status preamble — when this is posted as a PR comment, all of that is already visible in the GitHub UI immediately above the comment, so repeating it just pushes the actually-useful content (score + findings) below the fold. The classification work from Phase 2 still happens; it just isn't echoed back at the reader.
161+
Use this exact format:
238162

239163
```
164+
## PR Review: #<number> — <title>
165+
166+
**Author:** <author>
167+
**Type:** <detected PR type>
168+
**Packages affected:** <list>
169+
**CI Status:** <passing/failing/pending>
170+
240171
### Confidence Score: <score>/100
241172
242173
<one-sentence summary>
@@ -264,7 +195,6 @@ Start directly with the score. Skip a header title, author, type, packages-affec
264195
| API surface | PASS/WARN | ... |
265196
| Accessibility | PASS/WARN | ... |
266197
| Security/Quality | PASS/WARN | ... |
267-
| Docs coverage | PASS/WARN/FAIL | ... |
268198
269199
### Recommendation
270200
@@ -273,41 +203,6 @@ APPROVE / REQUEST_CHANGES / COMMENT
273203
<brief rationale>
274204
```
275205

276-
## Phase 6: Post the review back to the PR
277-
278-
After presenting the output in the chat, post the same text as a comment on the PR so the review is visible to maintainers and to Copilot (when the PR author is `copilot-swe-agent`, the comment becomes actionable feedback).
279-
280-
Save the output from Phase 5 to a temp file so the markdown isn't mangled by shell quoting, then:
281-
282-
```bash
283-
gh pr comment $ARGUMENTS --repo microsoft/fluentui --body-file /tmp/pr-review-$ARGUMENTS.md
284-
```
285-
286-
Append a single trailer line to the body so the post is identifiable:
287-
288-
```
289-
---
290-
*Posted via the `/review-pr` skill.*
291-
```
292-
293-
The posted comment should be **identical** to what you rendered in chat — don't paraphrase or summarize. The chat output and the PR comment must match so the user can trust that what they saw is what the maintainers see.
294-
295-
**Pre-checks before posting:**
296-
297-
1. Confirm the active `gh` account has write access to the PR's repo (EMU accounts read fine but silently fail on writes):
298-
```bash
299-
gh api graphql -f query='{ viewer { login } repository(owner:"microsoft", name:"fluentui") { viewerPermission } }'
300-
```
301-
If `viewerPermission` is `NONE`, stop and ask the user to `gh auth switch --user <non-emu-account>`.
302-
2. Don't post on PRs from your own branches unless explicitly asked — self-review comments are noise.
303-
3. If `REQUEST_CHANGES` is the recommendation, still post the comment but note to the user that only a formal review (`gh pr review --request-changes`) actually blocks merge; a comment is advisory.
304-
305-
**When to skip posting:**
306-
307-
- The user explicitly asks for a review without posting ("review but don't post").
308-
- The PR has an existing comment from this skill within the last day on the same head SHA — avoid duplicate noise. Look for the `*Posted via the \`/review-pr\` skill.\*` trailer.
309-
- Draft PRs where the user is clearly still iterating (state=OPEN, isDraft=true, recent force-push) — offer to post but don't do it by default.
310-
311206
## Notes
312207

313208
- For large PRs (50+ files), prioritize: published source files > test files > config. Note reduced confidence due to review scope.

0 commit comments

Comments
 (0)