Repository navigation
Codify MXC agent and code-review guidance #1519
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
Merged
Branden Bonaby (bbonaby)
merged 1 commit into
main
from
user/bbonaby/codify-repo-instructions
Oct 19, 2026
Merged
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
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,167 @@ | ||
| --- | ||
| name: mxc-code-review | ||
| description: Review MXC pull requests and diffs for SDK breaking changes, CI gate regressions, useful SDK documentation, maintainable design, and containment. | ||
| --- | ||
|
|
||
| # MXC code review | ||
|
|
||
| Read `AGENTS.md`, `.github/copilot-instructions.md`, matching language | ||
| instructions, and the affected subsystem docs. Follow the changed flow through | ||
| its callers, shared code, backends, and failure paths. Do not make edits during | ||
| review or stop after finding one working path. | ||
|
|
||
| ## Check SDK compatibility first | ||
|
|
||
| - Compare changed public APIs with the existing supported version in Rust, | ||
| .NET, and Node. Check removals, renames, signatures, types, import paths, | ||
| serialization, defaults, errors, cancellation, and resource ownership. | ||
| Look for source, binary, and behavioral breaks, not only compile failures. | ||
|
|
||
| - Report every confirmed breaking change to an existing supported SDK API in | ||
| the **highest review category: P0 / critical**. Put these findings first and | ||
| label them **SDK breaking change**. This is a maintainer compatibility alert; | ||
| it does not imply a security vulnerability or universal runtime failure. | ||
|
|
||
| - Identify the affected SDK and version, the old and new behavior, and a | ||
| concrete caller that breaks. Call it out even when the PR describes the | ||
| change as intentional. Maintainers decide whether to accept it; the reviewer | ||
| must not silently waive it or assume acceptance. | ||
|
|
||
| - Published SDK APIs require a new versioned surface for breaking changes. | ||
| Check that the existing version and its references remain intact. Adding | ||
| a new version while preserving the old one is not itself a breaking change. | ||
|
|
||
| ## Check the design in context | ||
|
|
||
| - Does the change fit the existing design and remain understandable to the | ||
| next maintainer, or does it merely get the requested feature working? | ||
|
|
||
| - Look for duplicated paths, feature-specific helper variants, narrow | ||
| exceptions, and unnecessary abstractions. Prefer fixing the responsible | ||
| shared component over adding another special case. | ||
|
|
||
| - Check responsibility boundaries. Parsing should decode contracts and retain | ||
| field presence; backend-specific defaults, policy checks, and enforcement | ||
| belong in the backend, not the shared parser. | ||
|
|
||
| - If a feature requires a broad rewrite or large deletion, call out the | ||
| maintenance risk and recommend smaller, reviewable vertical slices with | ||
| behavior coverage. Do not demand a rewrite for a local problem. | ||
|
|
||
| ## Check MXC rules | ||
|
|
||
| - **Rust package boundaries:** Inspect `src/Cargo.toml` workspace members and | ||
| changed Cargo dependencies. Internal engine, contract, shared-runtime, | ||
| backend, build, and schema implementations belong as modules in `mxc-sdk`, | ||
| not new workspace crates or publishable path dependencies. Required process | ||
| boundaries use existing `[[bin]]` patterns; distinguish legitimate tools and | ||
| FFI packages from unnecessary SDK implementation crates. | ||
|
|
||
| - **Dispatch and build:** `mxc_engine` owns backend routing. No dispatch or | ||
| backend implementation dependencies in `mxc_common`, backend-selection | ||
| matches in thin executors, or recursive Cargo invocation in `build.rs`. | ||
| Keep platform/feature gates and build-time staging intact. | ||
|
|
||
| - **Containment and errors:** Unsupported policy fails closed before container | ||
| creation. Preserve failure propagation, cancellation, cleanup, handle | ||
| ownership, and panic containment across the C ABI. Check each changed | ||
| execution surface, not only create-and-run. | ||
|
|
||
| - **Contracts:** Preserve optional-field presence and version-specific parsing. | ||
| Do not edit released schemas or hand-edit generated artifacts. Development | ||
| fields need the proper contract, adapter, regenerated artifacts, and tests; | ||
| experimental authorization does not replace contract or host validation. | ||
|
|
||
| - **SDKs:** Compare affected Rust, .NET, and Node versioned references. Preserve | ||
| field meanings, optional-field presence, and language-appropriate naming. | ||
| Confirm .NET JSON source-generation registration and AOT smoke coverage. | ||
|
|
||
| - **Host compatibility:** New Windows backend features need concrete host | ||
| prerequisites, runtime capability checks, actionable unsupported-feature | ||
| errors, and a brief update to existing user guidance when requirements or | ||
| behavior change. Document supported builds, limits, and remedies, not a | ||
| backend implementation walkthrough. SDK/schema versions alone cannot prove | ||
| host support. Telemetry requires MXC-owned consent and restrictive admin policy. | ||
|
|
||
| ## Keep SDK documentation useful | ||
|
|
||
| SDK READMEs and versioned API references are consumer-facing entry points. | ||
| Review additions and removals against what a caller needs to install, choose, | ||
| use, and safely clean up an operation. | ||
|
|
||
| - Keep signatures, examples, defaults, prerequisites, limitations, and error | ||
| remedies accurate and consistent with the supported API. Check affected | ||
| links and examples when content is moved or removed. | ||
|
|
||
| - Flag changes that hide supported behavior, remove necessary usage guidance, | ||
| contradict the API, or bury instructions under irrelevant implementation | ||
| detail. Keep backend internals and contributor workflows in developer docs; | ||
| link them only when they help the reader's task. | ||
|
|
||
| - Make additions and removals leave a coherent final document. Avoid agent | ||
| reasoning, change diaries, discarded designs, and repetition. Do not demand | ||
| more documentation merely because code changed, or report wording nits as | ||
| defects. | ||
|
|
||
| ## Check for weakened CI gates | ||
|
|
||
| Inspect removed or changed checks, including workflows, reusable jobs, build | ||
| scripts, test commands, and matrix configuration. Do not assume a green check | ||
| means the same validation still runs. | ||
|
|
||
| - Check trigger and path filters, job conditions and dependencies, platform | ||
| and feature coverage, test discovery, exclusions, and skipped tests. | ||
| Look for checks that disappear or stop covering the changed behavior. | ||
|
|
||
| - Check failure propagation. `continue-on-error`, ignored exit codes, removed | ||
| assertions, and success-shaped fallbacks can make a failing build or test | ||
| appear green. Trace the relevant command through its callers to the final | ||
| check result. | ||
|
|
||
| - Flag removed or weakened validation that lets a concrete regression pass. | ||
| Name the lost check, the affected path or platform, and what CI no longer | ||
| detects. Do not accept disabling the failing check as a fix for the failure. | ||
|
|
||
| - Legitimate replacement or retirement is allowed. Verify equivalent coverage | ||
| still runs and gates the result, or that the covered behavior is actually | ||
| removed. An explanation alone does not establish equivalent coverage. | ||
|
|
||
| ## Require behavior coverage | ||
|
bbonaby marked this conversation as resolved.
|
||
|
|
||
| Feature changes need functional validation in the applicable CI suite. | ||
| Backend behavior needs coverage in the scheduled/nightly validation runs on | ||
| suitable hosts. Bug fixes need a regression scenario for the reported failure, | ||
| preferably shown to fail before the fix and pass afterward. Verify the scenario | ||
| is wired into CI and checks observable behavior through the relevant API or | ||
| backend, including affected interactions and failures. | ||
|
|
||
| Do not accept a collection of tiny unit tests as a substitute for that coverage. | ||
| Unit tests, including those at the bottom of `.rs` files, can support isolated | ||
| logic; passing tests that mirror new helpers do not establish that a feature | ||
| works in MXC. Prefer extending existing functional scenarios over test-count | ||
|
bbonaby marked this conversation as resolved.
|
||
| growth. Follow the validation runbook and backend guide; report specific gaps, | ||
| not demands for unrelated tests. Documentation-only edits need no runtime tests. | ||
|
|
||
| ## Write useful findings | ||
|
|
||
| Focus on high- and medium-severity defects; critical failures always qualify. | ||
| Leave out low-severity issues, style nits, speculative scenarios, and optional | ||
| refactoring. Apply the highest-priority SDK breaking-change rule above; | ||
| otherwise, do not inflate severity just to include a finding. | ||
|
bbonaby marked this conversation as resolved.
|
||
|
|
||
| Review all affected paths before submitting, combine related findings, and | ||
| post one consolidated review. Do not drip-feed findings across rounds or | ||
| reopen resolved points without new evidence. New edits still need review for | ||
| material regressions, not unrelated cleanup. | ||
|
|
||
| Use plain language an everyday developer can act on. Give the severity, | ||
| file/line, triggering situation, incorrect behavior, and consequence. Include | ||
| enough detail to explain the issue without jargon, long essays, or wording so | ||
| short that the reader must guess. Do not include an agent's internal reasoning. | ||
|
bbonaby marked this conversation as resolved.
|
||
|
|
||
| Keep comments and requested documentation focused on non-obvious reasons and | ||
| useful behavior, usage, and limits, not implementation diaries. Separate | ||
| paragraph-length list items with a blank line; keep short lists compact. | ||
| Report concrete defects and material maintenance risks, not speculative bugs | ||
| or style preferences. Distinguish missing test evidence from a confirmed | ||
| runtime failure. | ||
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,81 @@ | ||
| # MXC agent instructions | ||
|
|
||
| Read and follow [.github/copilot-instructions.md](.github/copilot-instructions.md) | ||
|
bbonaby marked this conversation as resolved.
|
||
| before writing or reviewing code. It contains MXC's architecture, compatibility, | ||
| security, build, and SDK rules. The expectations below apply in addition. | ||
|
|
||
| ## Design for the codebase, not just the feature | ||
|
|
||
| - Understand the existing flow before editing: callers, shared helpers, | ||
| backend ownership, errors, cleanup, and affected SDK entry points. A working | ||
| happy path is not enough; check how the change composes with existing behavior. | ||
|
|
||
| - Put behavior where it belongs. Parsers decode contracts and preserve field | ||
| presence; backend-specific defaults, policy checks, and enforcement belong | ||
|
bbonaby marked this conversation as resolved.
|
||
| in the backend, not shared parsing code. | ||
|
|
||
| - Reuse the normal implementation path. Do not add a parallel helper, a | ||
| feature-specific variant, or a narrow exception just to make a feature work. | ||
| If the design cannot express the feature cleanly, refactor the responsible | ||
| component while preserving existing callers and behavior. | ||
|
|
||
| - Choose the simplest design that fits the current requirements. Add an | ||
| abstraction only when it solves a real problem; do not build for imagined | ||
| reuse. Name functions for their responsibility, not a growing list of cases. | ||
|
|
||
| - Consider whether a maintainer can understand and safely extend the result. | ||
| Completing the requested feature does not excuse duplication, misplaced | ||
| responsibilities, weakened checks, or hidden failure paths. | ||
|
|
||
| - If adding a feature requires replacing or deleting a large part of a | ||
| subsystem, split the work into reviewable vertical slices: a small part of | ||
| the feature working through the relevant layers, with its tests. Separate | ||
| prerequisite refactoring from behavior changes; keep each step buildable | ||
| and preserve existing behavior. Do not attempt the whole rewrite at once. | ||
|
|
||
| ## Write for everyday developers | ||
|
|
||
| Use plain, precise language in code reviews, comments, documentation, and | ||
| handoffs. Explain the concrete behavior and consequence; avoid invented jargon | ||
| and research-paper language. Be concise without leaving out the detail needed | ||
| to understand or act. Neither verbosity nor oversimplification is helpful. | ||
|
|
||
| Keep comments sparse and short: explain a non-obvious reason or constraint, | ||
| not what the next line does. Prefer clear names and straightforward code. | ||
|
|
||
| Documentation should describe the finished design, behavior, and usage, not | ||
| an agent's thought process, implementation diary, or discarded approaches. | ||
| Update the relevant canonical document rather than adding overlapping Markdown. | ||
| Document only what the reader needs: supported behavior, usage, prerequisites, | ||
| limitations, and useful error remedies. Include implementation detail only when | ||
| it explains a necessary maintenance or design decision, in developer docs. | ||
|
|
||
| Separate paragraph-length Markdown list items with a blank line; keep short | ||
| lists compact. | ||
|
|
||
| ## Require meaningful behavior coverage | ||
|
|
||
| - For a feature, add or extend a functional validation test in the applicable | ||
| CI suite. Backend behavior must run in scheduled/nightly validation on | ||
| the relevant hosts; adding a test file without wiring it into CI is not enough. | ||
|
|
||
| - For a bug fix, add a regression scenario that exercises the reported failure | ||
| through the relevant public API or backend flow and checks the correct result. | ||
| Where practical, show that it fails before the fix and passes afterward. | ||
|
|
||
| - Prefer tests of useful behavior, feature interactions, and failure handling | ||
| over many tiny checks of implementation details. Extend existing scenarios | ||
| rather than duplicating them. | ||
|
|
||
| - Unit tests, including tests at the bottom of `.rs` files, are useful for | ||
| isolated logic but do not replace functional validation. Do not judge | ||
| coverage by test count or tests that merely repeat the new implementation. | ||
|
|
||
| - Follow the [validation runbook](docs/development/build-and-test/ci-validation-infrastructure.md) | ||
| and backend guide. Check that the intended scenario actually runs and asserts | ||
| behavior on a suitable host. State what was not run or cannot be covered; | ||
| do not invent evidence. Documentation-only changes do not need runtime tests. | ||
|
|
||
| For reviews, use the [MXC review skill](.github/skills/mxc-code-review/SKILL.md). | ||
| Focus on high- and medium-severity defects and consolidate findings into one | ||
| review, rather than creating extra review rounds for nits or optional cleanup. | ||
|
bbonaby marked this conversation as resolved.
|
||
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,5 @@ | ||
| # MXC Claude instructions | ||
|
|
||
| @AGENTS.md | ||
|
|
||
| For code review, follow [.github/skills/mxc-code-review/SKILL.md](.github/skills/mxc-code-review/SKILL.md). |
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.