Repository navigation
docs(claude): restructure .claude/ into path-scoped rules and mirror them to Copilot - #652
Conversation
The three reference docs under .claude/ were pulled in by @-imports from CLAUDE.md. Imports are expanded at launch, so ARCHITECTURE.md, PATTERNS.md, and DEBUGGING.md loaded in full on every session — ~60KB (~15k tokens) of context regardless of the task. Reorganise around Anthropic's documented .claude/ layout, where each mechanism is defined by when it loads: - Add .claude/rules/ with nine path-scoped rules distilled from the three docs. Each declares `paths:` frontmatter, so it costs nothing until Claude opens a file it applies to. git-workflow.md is deliberately unscoped — commit conventions are needed even in sessions that open no source file. - Move the debugging playbook to a .claude/skills/debug-maidr/ skill, since it is a multi-step procedure rather than a convention. Named debug-maidr so it does not shadow the bundled /debug skill. - Trim CLAUDE.md to 92 lines with no heavy imports, and correct commands that did not exist: there is no `npm run dev` or `npm run preview`, only the per-example dev servers and `build:preview`. - Delete the three docs; their normative content now lives in the rules. Also consolidate the subagents, which had grown two overlapping pairs: - Merge react-ts-code-reviewer into code-reviewer and react-ts-engineer into implementer. Overlapping descriptions make delegation ambiguous, and both descriptions were ~1500 chars of inline examples loaded on every request. All eight remaining descriptions are now under 250 chars. - Since subagents inherit project rules, the agent bodies no longer restate the codebase conventions and describe the role instead. - feature-builder used the legacy `Task(...)` alias, whose agent-type allowlist is ignored in a subagent definition; use `Agent` and name the delegates in the body. - debugger preloads the debug-maidr skill via `skills:`; test-runner gains the Skill tool so it can invoke it on demand. - Repoint the stale doc links in .github/agents/ at the new locations. Add .claude/README.md documenting what goes where and why agents/ and rules/ coexist. Every session now loads ~4.5KB instead of ~60KB, with per-layer detail arriving only when the matching files are opened.
🔎 MAIDR example preview is readyOpen the preview: https://pr-652.maidr-preview.pages.dev
Built from |
ReviewThis is a config/docs-only PR ( Verification performedI cross-checked a large sample of claims against the actual repo state and the current Claude Code docs:
I didn't find any inaccurate path, broken glob, or misused frontmatter field in the sampling I checked. Minor observations (non-blocking)
OverallWell-scoped, and the technical claims are accurately grounded in both the actual repo layout and the current Claude Code mechanism semantics — I verified these independently rather than taking the PR description at face value. The consolidation of the two overlapping subagent pairs and the fix for the stale Generated by Claude Code (automated review) |
…skills Two follow-ups to the .claude/ restructure. Skills: `code` and `code-review` were ~90% restatements of what the new path-scoped rules now deliver — TypeScript standards, React rules, the MVVC layer contracts, accessibility — with the remainder duplicating the `implementer` and `code-reviewer` subagents. Two sources for one convention is the drift risk the docs warn about, and the project `code-review` skill also shadowed Claude Code's bundled skill of the same name. Delete both; the bundled `/code-review` is reachable again and the agents cover the personas. `debug-maidr` stays: it is a procedure nothing else covers. Copilot: the repository configures Copilot under .github/, which has the same four concepts in different file formats. The path-scoped pair matches almost exactly — Claude scopes a rule with a `paths:` YAML list, Copilot with a comma-separated `applyTo:` string — so generate that side rather than hand-maintaining a second copy: CLAUDE.md → .github/copilot-instructions.md .claude/rules/*.md → .github/instructions/*.instructions.md An unscoped rule, which loads every session in Claude Code, becomes `applyTo: "**"`. Brace groups are expanded to comma-separated patterns, since `applyTo` documents commas but not braces. - Add scripts/sync-copilot-instructions.mjs with a --check mode, plus `sync:copilot` and `sync:copilot:check` npm scripts. - Add a CI job running the check. It is pure Node, so it skips npm ci and gates no other job. - Replace style-guide.instructions.md with the generated typescript instructions, folding in its one accurate claim not already covered (barrel index.ts at module boundaries only). Its "80% coverage" and "100 character line length" rules were dropped: neither is configured in jest.config.ts or eslint.config.ts. - Repoint the four files that referenced style-guide.instructions.md, and fix the relative link paths in .github/agents/, which resolved to .github/agents/.github/... and were already broken. Agents and workflow prompts are deliberately not generated: the agent frontmatter schemas genuinely differ (handoffs and a display name on Copilot's side, permissionMode/memory/skills/maxTurns on Claude's), and .github/prompts/ holds GitHub chores that Claude Code does through its GitHub tools instead. .claude/README.md documents the mapping and the asymmetries.
ReviewThis is a docs/config-only change ( What's good
Findings
Nit
No security concerns (no secrets, no code execution changes beyond the new build-time script which only reads/writes within the repo), and no test coverage changes needed given the scope. |
The script lives under scripts/, which eslint.config.ts does not ignore, so `npm run lint` failed on it. Six errors, all mine: - `BANNER` was an arrow constant; antfu/top-level-function wants a declaration. Renamed to `banner` now that it reads as a function. - `process` needs an explicit `node:process` import under node/prefer-global/process. - `/^#\s+(.+)$/m` let `\s+` and `.+` exchange spaces, which regexp/no-super-linear-backtracking flags as polynomial backtracking. Anchoring the capture to `\S` makes the boundary unambiguous. Also filled in the four @returns descriptions that were warnings. Verified locally with dependencies installed: lint, type-check, build, and 1048 unit tests all pass.
ReviewThis is a config/docs-only PR (no Per the PR's own request, I focused most scrutiny on Bug:
|
Addresses review feedback on #652. The sync script listed `.claude/rules/` non-recursively, so a rule in a subdirectory was silently dropped: no output file, no orphan warning, and `--check` still reported "in sync". Confirmed by adding `.claude/rules/frontend/react.md` and watching the check pass while nothing was generated. That is the exact silent-drift failure the script exists to prevent, and `.claude/README.md` actively invites it by documenting that nested rules work — which they do in Claude Code, and now in the mirror too. Both tools support nesting, so the structure is mirrored rather than flattened: `.claude/rules/frontend/react.md` generates `.github/instructions/frontend/react.instructions.md`. Orphan detection walks recursively as well, and output directories are created as needed. Also repoint cross-references. Rule bodies were copied verbatim, so a rule saying "See `rules/model.md`" left a Copilot-only reader chasing a path that does not exist from `.github/instructions/`; those now rewrite to `model.instructions.md`. The same problem in CLAUDE.md is fixed at the source by naming `.claude/skills/debug-maidr/SKILL.md` before the `/debug-maidr` shortcut, and by describing `.claude/` and `.github/` together. Docs: add the missing `debugger` row to the agents table in `.claude/README.md`, and document the subdirectory mirroring, the cross-reference rewriting, and that a renamed rule leaves an orphan the plain sync only warns about. Verified: 8-case regression pass over the script (clean, edited rule, nested rule, nested output path, nested orphan, flat orphan, malformed frontmatter, restored), plus lint clean.
|
Thanks — the subdirectory finding was a real bug and I've fixed it in 1. Nested rules silently dropped — confirmed and fixedYou were right, and it was worse than a missing feature: it was silent. I reproduced it before fixing: A rule that Claude Code loads, that the mirror never generates, and a check that passes anyway — exactly the drift this script exists to catch, made worse by I went with mirroring the structure rather than flattening, since Copilot supports subdirectories of 2. Claude-only references leaking into Copilot's copy — fixed, at two different levelsSplit this by cause:
I deliberately didn't build a general templating/conditional-block mechanism. Two dead pointers didn't justify a second syntax in these files, and rewording is harder to get subtly wrong. 3. Missing
|
ReviewThis is a docs/config-only PR (no Verified correct
Minor / non-blocking findings
None of these are functional blockers — the author's own verification (27 |
Addresses the second round of review feedback on #652. `readPaths` only recognised block-list syntax, but the guard checked for the `paths:` key rather than the parsed result. A rule written as a YAML flow sequence — `paths: ["src/model/**"]` — passed the key check, parsed to zero globs, and fell through to `applyTo: "**"`. Confirmed before fixing: that input mirrored a model-layer rule as applying to every file in the repository. Silently widening scope is the wrong failure direction, so a `paths:` key that yields no globs is now an error naming the supported syntax. A rule with no frontmatter at all still maps to `applyTo: "**"`, which is intentional — that is what an unscoped rule means in Claude Code. Also reword the `/debug-maidr` reference in rules/architecture.md, which is mirrored verbatim into Copilot's copy where slash-command skills do not exist. Same treatment already applied to CLAUDE.md last commit; this was the remaining instance inside a mirrored body. Note the Node 18.17.0 floor for `fs.readdirSync`'s `recursive` option in the script header. Not adding an `engines.node` field: this package is published, so that would change install behaviour for every consumer, which is a project-wide call rather than something to slip into a config change. Regression suite now 11 cases, including the flow-sequence and empty-`paths:` inputs and a check that an unscoped rule still maps to "**". Lint clean.
|
All three addressed in 1.
|
ReviewThis PR touches no application code — it's a restructure of Findings1. const globs = frontmatter
.split('\n')
.filter(line => line.trim().startsWith('- '))
...This grabs every line starting with 2. No automated test for the sync script (test coverage) 3. Nice-to-have: no pre-commit hook wired to Verified as correct
OverallWell-scoped, well-documented tooling change with a sensible design (generate the Copilot side instead of hand-maintaining two copies). The two follow-up commits show the author caught and fixed real bugs in their own script through iteration, which is a good sign. The one gap worth addressing before/after merge is locking in that regression coverage with an actual test file rather than a manually-run suite, plus the minor |
…ping Addresses the third round of review feedback on #652. The glob scan read every `- ` line in a rule's frontmatter rather than only the block under `paths:`, so a sibling list folded into the mirror. Confirmed before fixing: a rule with `paths: [src/model/**]` plus a `tags:` list generated `applyTo: "src/model/**,leaked-value"`. The empty-result guard added last commit could not catch this, since the result was not empty. The scan now stops at the next top-level key. Check in the regression suite that had been run by hand. Three genuine bugs reached review in this script — nested rules dropped silently, a flow-sequence `paths:` widening scope to the whole repo, and now this one — and `.claude/rules/testing.md`, added by this same PR, says new behaviour ships with a test that would have failed before it. The script did not meet that bar. `test/scripts/syncCopilotInstructions.test.ts` drives the CLI end to end: each case builds a fixture tree, points the generator at it, and asserts on the exit code and generated files. `SYNC_COPILOT_ROOT` exists for that; it defaults to the repository root, so nothing changes for normal use. The 18 cases were verified to fail against the code they describe: 1 fails against the previous commit (key scoping), 7 against the commit before it (nested rules, nested orphans, flow sequence, empty `paths:`, cross-references). Verified: lint clean, 1066 unit tests across 88 suites pass, mirror in sync.
|
Findings 1 and 2 are addressed in 1.
|
| Script version | Result |
|---|---|
5533222 (this commit) |
18 passed |
8e917c2 (before key scoping) |
1 failed — should ignore block lists belonging to other frontmatter keys |
009c958 (before recursive + flow-seq) |
7 failed — nested rule, nested orphan, flow sequence, empty paths:, cross-references |
A suite that passes against the buggy code would have been worse than none, so that seemed worth confirming rather than asserting.
Full run: lint clean, 1066 tests across 88 suites pass.
3. Pre-commit hook — declining, but happy to be overruled
Skipping this one deliberately rather than by oversight. Husky currently runs only commit-msg, which is a validation on text the contributor just typed. A pre-commit hook is a different thing: it adds latency to every commit for every contributor, including ones who never touch .claude/. The copilot-sync CI job already catches this, and it's non-gating and finishes in ~5 seconds, so the feedback loop is short.
Changing everyone's commit flow feels like a maintainer call rather than something to fold into a config PR. If you'd like it, a pre-commit that early-exits unless CLAUDE.md or .claude/rules/ is staged would be the cheap version — say the word and I'll add it as its own change.
Generated by Claude Code
ReviewThis PR touches no application code — it's a restructuring of Code quality / correctness
Test coverage
Minor suggestions (non-blocking)
OverallNo security concerns (pure Node built-ins, no dependency additions, operates only on repo-local paths). No performance concerns (small file set, runs once in CI). This is a well-scoped, well-tested infra change with good self-verification already documented in the PR description — I independently confirmed the specific claims (glob coverage, link resolution, generated-output correctness) rather than taking them at face value, and they hold up. 🤖 Generated with Claude Code |
Orphan handling was documented, but under the Copilot section, ~80 lines from the "Adding a rule" heading someone reads when changing rules. It came up in two consecutive review rounds, which is a reasonable signal the placement was the problem rather than the wording. Retitle that section to cover removal and state the extra step there: the generator deliberately does not delete the instruction file a removed rule leaves behind, so `sync:copilot:check` keeps failing in CI until you delete it yourself. Docs only; no generated files change.
|
Thanks. CI is green across all 13 checks on 2. Orphan callout — acted on (
|
|
Review in progress — will follow up with full comments shortly. |
|
Review This PR touches no application source ( What was verified
Minor / non-blocking
Both are small enough to not block on — the core mechanism (single authored source, generated mirror, CI drift check) is sound and well-tested for the cases it does cover. Everything else No security concerns — the script only reads and writes files under a |
…files Addresses the fourth round of review feedback on #652. Stripping quotes with `replace(/^["']|["']$/g, '')` only works when the quote is the last character. A YAML inline comment breaks that: confirmed before fixing, `- "src/model/**" # only the model layer` generated `applyTo: "src/model/**\" # only the model layer"` — a pattern matching nothing, written without complaint. `readGlob` now reads a quoted scalar to its closing quote, allows a trailing comment after it, and strips ` #` comments from unquoted values. Input it cannot read — an unterminated quote, stray text after the closing quote — is an error rather than a guess, consistent with how the parser already treats a flow-sequence `paths:`. Separately, nothing checked that a glob matches anything. A typo parses fine, generates a plausible `applyTo`, and passes the sync check while the rule is simply never loaded by either tool. That was verified by hand for this PR; `test/scripts/claudeRuleGlobs.test.ts` keeps it verified, asserting every generated `applyTo` pattern matches at least one tracked file and carries no unexpanded brace group. It reads the patterns from the generated files rather than re-parsing the rules, so the generator's parser is not duplicated. `git ls-files` does the matching — it is the authority on what is tracked and handles the glob styles in use, so no dependency is added. Both additions were verified to fail against the code they describe: the four comment cases fail on the previous commit, and the glob check fails when `src/model/**` is changed to `src/modle/**`. Verified: lint clean, type-check clean, 1127 tests across 89 suites pass.
|
Both acted on in 1. Inline YAML comments corrupting a glob — fixedReal, and it produced exactly the silent-wrong output you described: Your diagnosis of the mechanism is precise — I went with supporting comments rather than documenting them as unsupported. They're ordinary YAML, a contributor writing one is doing nothing wrong, and "documented footgun" still leaves a rule silently dead. 2. No check that a glob matches anything — fixedThis was the better of the two findings, and it's the same shape as everything else that's gone wrong in this file: parses fine, generates plausible output, exits 0, rule silently never loads. Manual verification doesn't survive contact with future edits — that's precisely how the last three bugs got in.
Two design notes:
I put it in the test suite rather than in the sync script, since the script has to stay runnable against the fixture trees its own tests build, which aren't git repositories. VerificationBoth additions were checked against the code they describe, since a test that passes on the buggy version is worse than none:
Full run: lint clean, type-check clean, 1127 tests across 89 suites. Worth noting where this landed: four review rounds, four bugs in this one script, all the same shape — wrong output, exit 0. The mechanism is now guarded by tests at both levels: the generator's behaviour, and the rules' own globs. Generated by Claude Code |
ReviewThis is a config/tooling PR — no application source under Code quality / best practices
Potential bugs
Performance
Security
Test coverage
Nice work — the generator + tests are the kind of thing that's easy to under-invest in for a "just docs" PR, and this doesn't. |
Addresses the fifth round of review feedback on #652. `readTitle` matched the first `# ` line anywhere in the body, with no idea what a code fence is. Confirmed before fixing: a rule opening with a shell example donated its comment as the description — ```bash # install deps first ``` # Model layer generated `description: "install deps first"`. Low impact, since the description is only hover text on Copilot's side, but it is the same silent substitution of wrong-for-right as the earlier parser bugs, and skipping fences costs about as much as the comment explaining the hazard would have. Also make the orphaned-file note in the non-check path a WARNING that says what to do about it. A contributor could run the generator, see it exit 0, and still get a red CI job for a file they were never told to delete. Not adding a `--prune` flag: deleting files the caller did not ask about is the surprising behaviour, and the note plus the README section now cover it. The fence case fails against the previous commit and passes here. Verified: lint clean, 1129 tests across 89 suites pass, mirror in sync.
|
Both addressed in
|
ReviewThis PR touches no Code quality — generator script
Tests
All CI checks are green, including Consistency checkI diffed a sample of the generated Minor, non-blocking
Security / performanceNo security concerns — no new dependencies, no shell interpolation, script only reads/writes within the repo tree it's pointed at ( Test coverageStrong for the new script; not applicable elsewhere since no application code changed. Overall: a well-scoped, well-tested infra change that removes a real drift risk (two hand-maintained copies of the same conventions) and, as a side effect, fixes some stale documentation. Nothing here blocks merging. |
Follow-up to review feedback on #652: a reviewer asked whether the deleted `code-review` skill's security checklist survived anywhere. Checked, and it only partly did. Grepping `.claude/` for innerHTML/XSS/sanitize turns up exactly one hit, in `agents/code-reviewer.md`. So the guidance reached someone *reviewing* a change and nobody *writing* one — the rules load at write time, and none of them said anything about it. That is the wrong way round for a preventable class of bug. Scoped each part to where it can actually go wrong rather than adding a broad security rule: - `rules/ui.md` — no `dangerouslySetInnerHTML` or `innerHTML` with data arriving from chart input, an LLM response, or a live feed. Nothing in `src/` uses it today, so this is preventive. - `rules/service.md` — no API keys in source, and treat chart input, LLM responses, and live data as untrusted at the boundary. Names `chat.ts` and the LLM services, where it would actually slip. Verified: lint clean, 1129 tests across 89 suites, mirror in sync with the cross-reference correctly rewritten to `ui.instructions.md` on Copilot's side.
|
Item 2 was a real gap — fixed in 2. Did the deleted skills' security content survive? Partly, in the wrong placeGood question to ask, and worth checking rather than assuming. Grepping So it survived only in the reviewer agent. That means the guidance reached someone reviewing a change and nobody writing one — rules load at write time, and none of them said anything about it. Backwards for a preventable bug class, and a real regression from deleting the skill even though the words existed somewhere. Rather than add a broad
This is arguably a better home than the skill had: it now loads automatically when someone opens a UI or service file, instead of only when a reviewer remembered to invoke a checklist. 1.
|
ReviewReviewed the final merged state of this PR ( Strengths
Minor / non-blocking
Not reviewed hereThis PR touches no Nice work on the test-first approach to a generator script — the fixture-driven CLI tests are a good pattern to reuse for future repo-tooling changes. |
…r agent Follow-up to review feedback on #652, which is fair against this PR's own premise: `agents/debugger.md` preloads the `debug-maidr` skill via `skills:` and then restated it, carrying its own layer-by-layer guide and symptom table alongside the skill's. Since preloading injects the skill in full, that agent started every run with two copies of the same guidance — the drift risk this PR exists to remove, and the opposite of what `.claude/README.md` says agent bodies are for. Two details in the agent were genuinely absent from the skill, so they moved into it rather than being deleted with the rest: - a chat row for the symptom table (`chat.ts`, `llmValidation.ts` — unset or rejected API key, or a promise that never settles) - the `Movable` strategies in the model checklist (`MovableGrid`, `MovableGraph`, `MovablePlane`), since a trace moving in the wrong direction is often wired to the wrong one Both verified against `src/` before being propagated. The agent is now role-only: how to work, what to report, how to finish. 79 lines to 59, with the reference material in one place. Verified: lint clean, 1129 tests across 89 suites, mirror in sync.
|
Item 1 fixed in 1.
|
ReviewThis PR restructures What's solid
Minor items worth a look
Other review categories
Nice, well-verified cleanup overall — nothing here blocks merging as far as I can tell. |
…ecks Addresses review feedback on #652. The `react-ts-engineer` merge quietly narrowed the implementer's tools. That agent had no `tools:` key, so it inherited everything including WebFetch and WebSearch, and its body told it to research when unsure. The merge kept `implementer`'s existing allowlist and dropped both. That was not a decision, it was an oversight — for a library built on Web Audio, ARIA, and SVG, looking up the specification beats guessing at it. Restored, with a guardrail: the codebase is the authority on convention, the specification on the platform, and a pattern found online that contradicts the project rules does not get imported. Add `test/scripts/agentParity.test.ts`. Agents cannot be generated the way rules are — the two tools genuinely disagree on frontmatter, and a mapping table would be its own bug source — but the realistic drift is cheap to catch: an agent added, removed, or renamed on one side only. It also checks that `name` matches the filename, that descriptions stay short since they load on every request, and that a `skills:` entry names a skill that exists. Both drift cases were simulated to confirm the test fails on them. That test caught a claim of my own: descriptions were asserted to be "under 250 characters" while `feature-builder` sat at exactly 250. Reworded to 201. Also note in `claudeRuleGlobs.test.ts` that `git ls-files` pathspec matching is close to, but not identical with, the runtime matchers — it catches a typo'd path, it does not prove parity with either tool. Verified: lint clean, 1133 tests across 90 suites, mirror in sync.
|
Items 1 and 3 acted on in 1.
|
ReviewThis PR restructures Code qualityThe sync script is genuinely careful work. Highlights:
Potential issues (minor, non-blocking)
BugsNone found in the current state. The PR history shows five real bugs in the sync script (non-recursive directory listing, flow-sequence Test coverageStrong. One gap: I didn't see a test asserting the content mirroring end-to-end against the real Performance / securityNo concerns. The script only touches Node built-ins, runs against small markdown files, and doesn't execute or eval any file content. Frontmatter parsing is hand-rolled but scoped (fails closed on ambiguity rather than guessing), which is the right tradeoff for a security-adjacent parsing task like this. OverallThis is a well-executed, thoroughly self-reviewed change — the commit history shows a real debug-first process (confirm the bug before fixing it, add a regression test that fails on the old code) that matches the very principles this PR is trying to encode into the rules. The two non-blocking nits above ( |
Addresses review feedback on #652. `--check` is documented as read-only but called `mkdirSync` on the output directory before branching, so a check run created a directory. The call was also redundant: `listMarkdown` already tolerates a missing directory, and each write creates its own parent. Removed, with a test asserting a check run leaves no directory behind. `expandBraces` handles nested groups correctly — `{ts,{jsx,tsx}}` does expand — but repeated a pattern, since both branches of the outer group produce `ts`. Harmless in `applyTo`, and no rule uses a nested group today, but the output is tidier deduplicated and the behaviour is now pinned by a test. Both cases fail against the previous commit. Also make the `no-console` claim precise. A reviewer could not verify it, so I checked by linting a file with a `console.log`: the rule is real, and it allows `console.warn` and `console.error`. The skill and the debugger agent said only that it "is enforced", which risks reading as a ban on deliberate error reporting — they now name the exemptions. Finally, assert the committed mirror is in sync from the test suite as well as the CI job, so a rule edited without regenerating fails on `npm test` rather than after a push. Verified: lint clean, type-check clean, 1136 tests across 90 suites.
|
All four addressed in 1.
|
ReviewThis is a well-executed restructuring of Verified consistent (no action needed)
Minor observations
Test coverageStrong — No security concerns — the generator only reads/writes within the repo tree it's pointed at ( |
|
🎉 This PR is included in version 3.75.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
Reorganises
.claude/to follow Anthropic's documented.claude/layout, then makes the repository's GitHub Copilot configuration a generated mirror of it so the two toolchains cannot drift.1. Restructure
.claude/into path-scoped rulesThe three reference docs under
.claude/were pulled in by@-imports fromCLAUDE.md. Imports are expanded at launch, soARCHITECTURE.md,PATTERNS.md, andDEBUGGING.mdloaded in full on every session — roughly 60KB (~15k tokens) regardless of the task. The docs are explicit that this doesn't help: "Splitting into@pathimports helps organization but doesn't reduce context, since imported files load at launch.".claude/rules/is the mechanism designed for this. Rules withpaths:frontmatter load only when Claude opens a matching file.93% smaller baseline.
Do
agents/andrules/both belong here?Yes — this was the open question behind the change. They're orthogonal, not alternatives, and Anthropic's
.claude/reference lists both as first-class directories. Rules are instructions that shape how work is done; agents are workers with their own context window. They compose: a subagent inherits the CLAUDE.md hierarchy including project rules, so a rule written once applies in the main conversation and inside every subagent. That's what lets the agent definitions get shorter here — they now describe a role, not the codebase.Rules added
architecture.mdsrc/**— MVVC layer contracts, canonical data flowmodel.mdsrc/model/**— traces,notifyStateUpdate(), highlight ownershipservice.mdsrc/service/**,controller.ts— Observer, Emitter, Disposable, secretsviewmodel.mdsrc/state/**— the Redux bridgeui.mdsrc/ui/**— React rendering rules, no untrusted markupcommand.mdsrc/command/**, keybinding services — commands and scopesaccessibility.mdtypescript.md.ts/.tsx— types, style, error handlingtesting.mdtest/**,e2e_tests/**— Jest and Playwrightgit-workflow.mdgit-workflow.mdis deliberately unscoped: commit conventions are needed even in sessions that never open a source file.Also here
.claude/skills/debug-maidr/). Per the docs, a multi-step procedure belongs in a skill rather than always-on context. Nameddebug-maidrso it doesn't shadow the bundled/debug.CLAUDE.mdtrimmed to ~95 lines, no heavy imports, and corrected commands that don't exist — there is nonpm run devornpm run preview; the real ones aredev:recharts/dev:victoryandbuild:preview.react-ts-code-reviewer→code-reviewerandreact-ts-engineer→implementer; overlapping descriptions make delegation ambiguous, and both were ~1500 chars of inline examples loaded on every request. The surviving eight match.github/agents/exactly, which already had the clean set.feature-builderused the legacyTask(...)alias, whose agent-type allowlist is ignored inside a subagent definition, so the restriction was a no-op → switched toAgent, with the delegation list stated in the body where it can actually be read.debuggerpreloads the skill viaskills:;test-runnergained theSkilltool. Neither could reach a skill before.2. Drop redundant skills, mirror rules into Copilot
Skills: two of three removed
codeandcode-reviewwere ~90% restatements of what the new rules now deliver — TypeScript standards, React rules, MVVC layer contracts, accessibility — with the remainder duplicating theimplementerandcode-reviewersubagents. Two sources for one convention is exactly the drift risk the docs warn about, and the projectcode-reviewskill shadowed Claude Code's bundled skill of the same name.Deleting both means the bundled
/code-reviewis reachable again. Their one piece of unique content — the XSS/secrets checklist — moved intorules/ui.mdandrules/service.md, where it now loads at write time rather than only when a reviewer invoked a checklist.Copilot: generated, not duplicated
The repo configures Copilot under
.github/, and the two tools have the same four concepts:CLAUDE.md.github/copilot-instructions.md.claude/rules/*.md(paths:).github/instructions/*.instructions.md(applyTo:).claude/agents/*.md.github/agents/*.agent.md.claude/skills/*/SKILL.md.github/prompts/*.prompt.mdThe path-scoped pair matches almost exactly, so rather than hand-maintain a second copy that's guaranteed to drift,
scripts/sync-copilot-instructions.mjsgenerates that side:Transformations:
paths:list → comma-separatedapplyTo:; unscoped rule →applyTo: "**"; brace groups expanded and deduplicated (sinceapplyTodocuments commas but not braces); cross-references repointed (rules/model.md→model.instructions.md) so a Copilot-only reader never follows a pointer to a file that doesn't exist on their side. Subdirectories are mirrored.npm run sync:copilotregenerates;npm run sync:copilot:checkfails if stale.copilot-syncCI job runs the check. Pure Node with no dependencies, so it skipsnpm ci; no job hasneeds:on it, so it gates nothing. Suggest adding it as a required check — drift nobody notices defeats the purpose, and it's invisible in review because generated files look plausible. That's branch-protection config, so it's a maintainer call rather than something this PR changes.style-guide.instructions.mdreplaced by the generatedtypescript.instructions.md. Its one accurate claim not already covered — barrelindex.tsat module boundaries only — was folded into the rule first. Its "80% coverage" and "100 character line length" rules were dropped as false: neither is configured injest.config.tsoreslint.config.ts..github/agents/, which resolved to.github/agents/.github/...and were already broken before this PR.Deliberately not generated: agents and prompts. The agent frontmatter schemas genuinely differ (
handoffsand a displaynameon Copilot's side;permissionMode,memory,skills,maxTurnson Claude's), and.github/prompts/holds GitHub chores — filing an issue from a template, replying to a PR review — that Claude Code does through its GitHub tools instead.agentParity.test.tsguards the drift that is cheap to catch: a role added, removed, or renamed on one side only.Related Issues
None.
Screenshots (if applicable)
N/A — configuration and tooling only.
Checklist
Additional Notes
On testing. No application source is touched, so there is no runtime behaviour to test manually — and there is no
ManualTestingProcess.mdin the repo to follow, despite the template referencing it. Three test files cover the tooling:test/scripts/syncCopilotInstructions.test.tsSYNC_COPILOT_ROOT, asserting exit codes and generated outputtest/scripts/claudeRuleGlobs.test.tsapplyTopattern matches ≥1 tracked file viagit ls-filesand has no unexpanded brace; also asserts the committed mirror is in synctest/scripts/agentParity.test.tsskills:entries existRun locally with dependencies installed: lint clean, type-check clean, build succeeds, 1136 tests across 90 suites pass.
Seven defects were found in the sync script during review. Five were the same shape — wrong output, exit code 0:
readdirSync--checkstill said "in sync"paths: [...]flow sequenceapplyTo: "**"tags:list folded intoapplyToreadTitlenot fence-aware# commentas the descriptionPlus two lesser ones:
--checkcreated the output directory despite being documented read-only, and nested brace groups expanded correctly but emitted a duplicate pattern.Each is now pinned by a regression case, and the cases were verified to fail against the commit that had the bug — a suite that passes against buggy code would be worse than none.
Reviewer note:
scripts/sync-copilot-instructions.mjsand its three test files are the real code here. The files under.github/instructions/are mechanical output — reviewing.claude/rules/*.mdcovers their content.Sources: CLAUDE.md and
.claude/rules/· Subagents · Copilot custom instructions · Copilot custom agents · VS Code custom instructions🤖 Generated with Claude Code