Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
|
Hi @bindlegirl 👋 |
There was a problem hiding this comment.
Pull request overview
This PR adds Claude AI assistant instruction files to the Connection package to help Claude better understand and work with this codebase. The instructions cover the package's overall structure, JavaScript build system, and Identity Crisis (IDC) subsystem.
Changes:
- Added comprehensive CLAUDE.md file documenting the Connection package architecture, option storage, communication patterns, and key subsystems
- Added js-build.md rule file explaining webpack configuration and build commands
- Added identity-crisis.md rule file detailing IDC detection, resolution paths, and technical implementation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| projects/packages/connection/changelog/add-connection-claude | Changelog entry for the new Claude instruction files (minor formatting issue) |
| projects/packages/connection/.claude/CLAUDE.md | Main Claude instruction file providing comprehensive package overview with accurate technical details |
| projects/packages/connection/.claude/rules/js-build.md | Build system documentation covering webpack entries, commands, and asset output |
| projects/packages/connection/.claude/rules/identity-crisis.md | Detailed IDC subsystem documentation including detection mechanisms, state management, and resolution workflows |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
| - `migrate_for_idc` — Flags that sync actions should be accepted (URL migration path) | ||
| - `identity_crisis_url_secret` — Short-lived secret (5min) for verifying same-site IDC | ||
| - `identity_crisis_ip_requester` — IP addresses making requests (for IP-based site URLs) | ||
|
|
There was a problem hiding this comment.
Maybe add that while in safe mode, only sync requests are blocked. Other requests will go out but without the URLs appended to the request.
There was a problem hiding this comment.
Good point, added a bit more info: 26c870b
bindlegirl
left a comment
There was a problem hiding this comment.
LGTM. This reminded me I need to update the Error Handler doc 😅
jeherve
left a comment
There was a problem hiding this comment.
Instead of adding a new set of claude-specific files, it may be best to create a new AGENTS.md file, since that's where we're headed at a8c:
- PCYsg-1bzA-p2
The main documentation seems to say that nested AGENTS.md files should work:
https://agents.md/
For reference, I'm starting work on migrating things in this repo, in #47190 |
kraftbj
left a comment
There was a problem hiding this comment.
Agreed with @jeherve. Per the field guide, we should be using AGENTS.md file (with a CLAUDE.md file that just points to @AGENTS.md)
We can use .ai instead of .claude for the other files and likely should reference them in the AGENTS.md (if they're not... I skimmed quickly and it didnt' scream out to me that it does).
Proposed changes:
Add Claude instruction files:
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
Take a look and see if the instructional files make sense.