Repository navigation
feat(docs): Add skill for PR review, AGENTS.md, and contributing guide - #318
Conversation
Signed-off-by: Andrey Velichkevich <andrey.velichkevich@gmail.com>
|
/hold for review |
| @@ -0,0 +1,323 @@ | |||
| # AGENTS.md | |||
There was a problem hiding this comment.
@modelcontextprotocol/swift-sdk Please review this and let me know if you think it would be better to shorten it.
akhilcb
left a comment
There was a problem hiding this comment.
Thanks for putting this together, @andreyvelich. Having AGENTS.md and CONTRIBUTING.md will help a lot and I checked most of the paths, commands and conventions against main. They're accurate. A few suggestions:
AGENTS.md / CONTRIBUTING.md
- The Agent Behavior Policy links the
2026-07-28spec, while Principle 2 says the SDK implements up to2025-11-25. Since 2026-07-28 support is still in progress, a short note on which revision to follow (current behavior vs. new-spec work) would help agents avoid mixing the two. .vscode/settings.jsonhasformatOnSaveturned on, so the "no formatter configuration" line could say something like "don't commit unrelated formatting changes" instead.NetworkTransportuses[weak self]in a few NWConnection callbacks. Narrowing that rule to actor-owned Tasks might stop agents from "fixing" working code.- The
Base/imports rule could be "match the file you're editing", since someBase/files useimport Foundation. - A one-line
CLAUDE.mdwith@AGENTS.mdwould let Claude Code pick this up automatically.
review-pr skill
- For
/review-pr <PR_LINK>, it might help to fetch the PR diff (gh pr diff) explicitly. Right now the agents default to the localgit diff. - Could code-simplifier be advisory-only in this flow, so it doesn't edit the local checkout during a review?
- Some of the agent prompts still have references from other languages and projects (Go/Rust/Python,
errorIds.ts/Sentry,pkg/foo/bar.go). Swapping those for ourMCPErrorand swift-log conventions would make the prompts sharper. - The publish-guard hook could also catch
gh apiwith-f/-F/--input, since those default to POST. If the hook is meant as a personal opt-in,.claude/settings.local.jsonmight be a better place to register it than the shared settings file.
Really like the pending-review and suggestion-block approach. Thanks again!
|
Thanks for review @akhilcb, I addressed the comments.
I think, this is not needed since Claude can read AGENTS.md now: |
Agreed. My bad. That’s a valid point. I missed it. |
|
Looks like the same Ubuntu setup failure we hit across our PRs: the Swift 6.1 job stops at |
Yeah, that makes sense. Could you help us to open PR to fix the CI @EhsanAzish80 ? |
|
Will go ahead with this PR, feel free to enhance contributing guides in the followup PRs! |
|
Thanks Andrey. I opened #320 for the Ubuntu CI fix. It pins the Swift 6.1 test job to |
modelcontextprotocol#318) * feat(docs): Add skill for PR review, AGENTS.md, and contributing guide Signed-off-by: Andrey Velichkevich <andrey.velichkevich@gmail.com> * Address review comments --------- Signed-off-by: Andrey Velichkevich <andrey.velichkevich@gmail.com>
I added initial contributing guide, AGENTS.md, and skill for PR reviews.
Motivation and Context
This should enhance how AI agents interact with our code base. Please review the contributing and agents guide for correctness.
/assign @varun06 @jeanmatthieu @akhilcb
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context
This PR was assisted with AI tools.