Skip to content

Add AGENTS.md + 27 OpenMage Agent Skills for Claude Code (and other agents)#5555

Open
colinmollenhour wants to merge 11 commits into
OpenMage:mainfrom
colinmollenhour:fix-bugs
Open

Add AGENTS.md + 27 OpenMage Agent Skills for Claude Code (and other agents)#5555
colinmollenhour wants to merge 11 commits into
OpenMage:mainfrom
colinmollenhour:fix-bugs

Conversation

@colinmollenhour
Copy link
Copy Markdown
Member

Summary

Adds two pieces of agent-facing tooling to the repo:

  1. AGENTS.md at the repo root — an always-loaded north-star document for any code-writing agent (Claude Code, Cursor, Copilot Workspace, Codex, etc.) describing repo conventions, BC rules, where the source of truth lives, and the project's stance on observers vs. rewrites vs. core edits.
  2. .claude/skills/ — 27 OpenMage Agent Skills organized in three tiers:
    • Tier 1 — Framework mechanics (12): m1-module-structure, m1-events-observers, m1-layout-blocks, m1-controllers-routing, m1-system-config, m1-acl-adminhtml, m1-eav, m1-db-setup-scripts, m1-api-soap-rest, m1-indexers-cron, m1-caching, m1-translations.
    • Tier 2 — Domain modules (11): mage-catalog, mage-sales, mage-checkout, mage-customer, mage-adminhtml, mage-promotions, mage-payment-methods, mage-shipping-carriers, mage-tax, mage-cms, mage-product-types.
    • Tier 3 — Tooling and quality (4): phpunit-openmage-tests, phpstan-magento1, rector-openmage, vendor-patches.

These follow the Anthropic Agent Skills format (a SKILL.md per skill with YAML frontmatter declaring name, description, and trigger conditions, plus dense reference content). They live under .claude/skills/ so Claude Code picks them up automatically; nothing else in the repo loads or depends on them, so non-Claude agents and humans can ignore them. Skills are layered (each cross-references neighbors with a See also block) so loading e.g. mage-promotions brings the right adjacent context (mage-sales, m1-events-observers, m1-indexers-cron) into scope by name.

How this was generated

Authored interactively with Claude (Sonnet/Opus 4.x) over several sessions, following a repo-tailored authoring loop:

  1. Plan first. A SKILLS-WISHLIST.md (kept locally, not in this PR) enumerated the candidate skills, grouped by tier, with one-line scope notes per skill — to keep boundaries clean before any prose was written.
  2. Read core, then write. Each skill started by Claude reading the relevant app/code/core/Mage/<Module>/ files (and the shared lib code where relevant), then summarising the non-obvious parts: tables, model lifecycle, area scoping, observer event names, indexer dependencies, gotchas. Things derivable from a single grep were left out; the goal is to stash the knowledge that's expensive to re-derive, not duplicate the codebase.
  3. Calibrate against AGENTS.md. AGENTS.md is the always-loaded summary; skills are the deeper, lazily-loaded layer for when a task touches their area. There's intentional overlap on the load-bearing rules (BC, where to fix, area scoping) and intentional non-overlap on module specifics.
  4. Cross-reference, then verify triggers. Each skill ends with a See also / Cross-refs block. Skill descriptions were tuned so Claude reliably loads the right one for the task — verified by spot-checking on a couple of issue-tracker bugs (see below).

The total cost (Anthropic API spend) to generate the skill set was modest — well under what a single review cycle by an experienced contributor would cost — and the corpus is now reusable for any future agent run on this repo.

Why this is useful

  • Better-grounded agent edits. Without these, an agent confronted with a salesrule/ bug has to spend a chunk of context window doing exploratory grep and reading several files just to recall how Mage_SalesRule_Model_Validator interacts with Quote_Discount and the rule DSL. With the skill loaded, that context is one tool call instead of fifteen — and the agent reads the correct mental model up front rather than reverse-engineering it from a partial sample of the source.
  • Smaller blast radius for AI-driven changes. Skills explicitly call out the BC / area-scoping / event-name conventions that are otherwise easy to violate without realising — e.g. observer area selection, _eventPrefix semantics, indexer reindex-vs-event timing, dynamicCallOnStaticMethod PHPStan trap. An agent that has read these is much less likely to ship the kind of change that technically works but creates a maintenance hazard.
  • Reusable across agent platforms. The format is just markdown with frontmatter; Cursor / Codex / OpenAI agents / etc. can either ingest these directly or with a thin adapter. AGENTS.md follows the same logic — a single, vendor-neutral document.
  • Living documentation. The skills sit next to the code they describe. When Mage_Sales_Model_Order changes, the matching skill (mage-sales) is right there to be updated as part of the same PR — no separate wiki to drift out of sync.
  • No runtime impact. Pure documentation under .claude/; not autoloaded, not deployed, not parsed by Magento.

How well does it actually work?

Three small bug-fix PRs were authored end-to-end by Claude using only these skills (no human pre-review of the diffs, just verification afterwards). Each PR's description lists exactly which skills the agent loaded for the task — useful as a calibration signal for skill triggering:

Each fix is small, targeted, and lands in the right model with the right idioms. Reviewers may want to look at those PRs alongside this one.

Compatibility / opt-out

  • Anyone not using Claude Code can ignore .claude/ entirely — it's gitignored by most non-Claude workflows already, and there's no build step or runtime hook.
  • Anyone using Claude Code automatically gets the skills with no additional config.
  • Other agent platforms can either symlink / copy the SKILL.md files into their own format or read them as-is — they're plain Markdown.

Test plan

This PR is documentation only. There are no source / config / test changes that affect runtime behavior.

  • Sanity-check the skill index loads correctly under Claude Code (auto, via .claude/skills/<name>/SKILL.md discovery).
  • Spot-check at least one Tier-1, Tier-2, and Tier-3 skill description for accuracy against current main (the three reference PRs above effectively did this for mage-promotions, m1-events-observers, and mage-product-types).
  • Confirm git ls-files .claude/ lists the 27 SKILL.md files.

🤖 Authored with Claude Code

Copilot AI review requested due to automatic review settings April 29, 2026 18:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds agent-facing documentation to help coding agents (Claude Code, Cursor, Copilot Workspace, etc.) work safely and consistently in the OpenMage / Magento LTS codebase.

Changes:

  • Adds a repo-root AGENTS.md as the always-loaded “north star” for agent workflows and project conventions.
  • Adds 27 Claude Agent Skills under .claude/skills/**/SKILL.md, organized by framework mechanics, domain modules, and tooling/quality topics.
  • Documents operational workflows for tooling (PHPStan, PHPUnit, Rector) and vendor patching.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
AGENTS.md Agent guide: project overview, BC rules, commands, architecture notes, and conventions.
.claude/skills/m1-acl-adminhtml/SKILL.md Agent skill: Magento 1 admin ACL concepts and wiring.
.claude/skills/m1-api-soap-rest/SKILL.md Agent skill: Magento 1 SOAP/XML-RPC and REST (Api2/OAuth) concepts.
.claude/skills/m1-caching/SKILL.md Agent skill: cache types, tagging, invalidation, and block caching.
.claude/skills/m1-controllers-routing/SKILL.md Agent skill: controller structure, routing, lifecycle, and form-key patterns.
.claude/skills/m1-db-setup-scripts/SKILL.md Agent skill: install/upgrade scripts, version bumps, and EAV setup.
.claude/skills/m1-eav/SKILL.md Agent skill: Magento 1 EAV mechanics and common patterns.
.claude/skills/m1-events-observers/SKILL.md Agent skill: event dispatch/observer registration and area scoping.
.claude/skills/m1-indexers-cron/SKILL.md Agent skill: indexer registration/modes and cron scheduling lifecycle.
.claude/skills/m1-layout-blocks/SKILL.md Agent skill: layout XML, blocks, templates, fallback, and template PHPStan implications.
.claude/skills/m1-module-structure/SKILL.md Agent skill: module activation, config.xml, aliases/rewrites, and area scoping.
.claude/skills/m1-system-config/SKILL.md Agent skill: system.xml, scopes, backend/source models, encrypted config.
.claude/skills/m1-translations/SKILL.md Agent skill: CSV translations, __() binding, and JS translator config.
.claude/skills/mage-adminhtml/SKILL.md Agent skill: adminhtml module concepts and common extension points.
.claude/skills/mage-catalog/SKILL.md Agent skill: catalog models, URL rewrites, indexing, and flat/EAV pitfalls.
.claude/skills/mage-checkout/SKILL.md Agent skill: checkout flows, quote session behavior, and conversion handoff.
.claude/skills/mage-cms/SKILL.md Agent skill: CMS pages/blocks/widgets and directive rendering.
.claude/skills/mage-customer/SKILL.md Agent skill: customer EAV, sessions, login flow, hashing, groups.
.claude/skills/mage-payment-methods/SKILL.md Agent skill: payment method capabilities, info storage, IPN/webhook patterns.
.claude/skills/mage-product-types/SKILL.md Agent skill: configurable/grouped/bundle/downloadable internals and cart payloads.
.claude/skills/mage-promotions/SKILL.md Agent skill: catalog vs sales rules, rule DSL, coupon generation, reindex needs.
.claude/skills/mage-sales/SKILL.md Agent skill: quote→order lifecycle, totals collectors, order states/statuses.
.claude/skills/mage-shipping-carriers/SKILL.md Agent skill: carrier registration, rate pipeline, free-shipping interactions.
.claude/skills/mage-tax/SKILL.md Agent skill: tax calculation pipeline, algorithms, display matrix, Weee/FPT.
.claude/skills/phpstan-magento1/SKILL.md Agent skill: PHPStan posture, plugin behavior, baselines, common pitfalls.
.claude/skills/phpunit-openmage-tests/SKILL.md Agent skill: OpenMage PHPUnit conventions, base class, providers, isolation.
.claude/skills/rector-openmage/SKILL.md Agent skill: Rector config/presets, custom migration rules, skip list.
.claude/skills/vendor-patches/SKILL.md Agent skill: vendor patch workflow via patches.json/.vendor-patches.

Comment thread .claude/skills/m1-layout-blocks/SKILL.md Outdated
Comment thread AGENTS.md Outdated
Comment thread AGENTS.md Outdated
Comment thread .claude/skills/phpstan-magento1/SKILL.md
Comment thread .claude/skills/phpstan-magento1/SKILL.md Outdated
@ajbonner
Copy link
Copy Markdown
Contributor

ajbonner commented Apr 29, 2026

I'll have a good play with this, have built out a similar CLAUDE.md / skills layout for our store so will be interesting to get a comparison.

First thing off the bat - Anthropic Agent Skills doesn't exist anymore looks like.

@colinmollenhour
Copy link
Copy Markdown
Member Author

I'll have a good play with this, have built out a similar CLAUDE.md / skills layout for our store so will be interesting to get a comparison.

Great, looking forward to your feedback. Feel free to also push improvements directly here if you like. This is just the first pass, I'll try to get around to making a second pass; I have a "skills audit" that I'll dig up and adapt, it works very well (the first pass of such large skills files often has lots of hallucinations). But in my experience the skills can make a big difference once they are of high quality.

First thing off the bat - Anthropic Agent Skills doesn't exist anymore looks like.

Hah, good catch. This must have come from my skill-writer skill that is out of date.. Seems like nobody cares to do redirects anymore these days.. :/

@colinmollenhour
Copy link
Copy Markdown
Member Author

have built out a similar CLAUDE.md / skills layout for our store

That's one question.. how do we include this in the repo? Should it just be right there in the root, or should it instead be published to a marketplace/plugin?

…ine header

Repo bumped PHPStan to level 8 in OpenMage#5542; AGENTS.md and the affected
skills still referenced level 6. Also drops the stale "23693 errors"
total from the phpstan-magento1 skill — `_loader.php` is just an
include list with no header.

Addresses Copilot review feedback on PR OpenMage#5555.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@colinmollenhour
Copy link
Copy Markdown
Member Author

AI Review Response · Commit: dba9558 · By: Claude Code with Opus 4.7 · Summary

Comments addressed: 5 / 5 (all fixed)

# Path Disposition
3163462338 .claude/skills/m1-layout-blocks/SKILL.md:8 Fixed — dropped "at level 6" in prose; bumped the example .phpstan.dist.neon snippet from level: 6 to level: 8.
3163462381 AGENTS.md:30 Fixed — applied the suggested wording verbatim (PHPStan level 8 with macopedia/phpstan-magento1).
3163462422 AGENTS.md:155 Fixed — heading is now ### PHPStan gotchas (level 8, strict rules on). (Kept the level number for context; happy to drop it entirely if reviewer prefers the suggestion as-is.)
3163462462 .claude/skills/phpstan-magento1/SKILL.md:9 Fixed — frontmatter description, opening paragraph, and the Config posture bullet (- \level: 8``) all now reflect level 8.
3163462492 .claude/skills/phpstan-magento1/SKILL.md:89 Fixed — dropped the stale **23693 errors** claim from the loader description. Replaced with "currently ~110 files" (one per identifier) plus a pointer to composer run phpstan:baseline. Also updated the earlier "(~23k entries total)" mention to a non-stamped, regenerate-to-get-fresh-count phrasing — to avoid baking another stale number into the doc.

State: branch head is dba955864a, all review threads resolved, no follow-up questions, no skipped threads.

@sreichel
Copy link
Copy Markdown
Contributor

sreichel commented Apr 30, 2026

Should it just be right there in the root, or should it instead be published to a marketplace/plugin?

Any place/idea where it could published to?

imho its fine to have it in root/project.

ideas

  • wording ... replace Magento 1 with OpenMage
  • tell CLAUDE OM relies on Magento 1
  • rename m1-* to openmage-* for general advices
  • rename mage-* to mage-module-* ...

colinmollenhour and others added 2 commits April 30, 2026 03:31
Adds .claude-plugin/marketplace.json + .claude/.claude-plugin/plugin.json
so the existing 27 skills under .claude/skills/ can be installed via
`/plugin marketplace add OpenMage/magento-lts` and the plugin name
`openmage-skills@openmage`. Project-local skill auto-load is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@colinmollenhour
Copy link
Copy Markdown
Member Author

Just pushed 395c708 which exposes the skills as a Claude Code plugin marketplace. The 27 .claude/skills/<name>/SKILL.md files don't move — they continue to auto-load as project skills when contributors open this repo. The new files just let downstream users (and other agents) pull them in without cloning the whole magento-lts tree.

Install in Claude Code

Inside an interactive Claude Code session:

/plugin marketplace add OpenMage/magento-lts
/plugin install openmage-skills@openmage

Or non-interactively from a shell:

claude plugin marketplace add OpenMage/magento-lts
claude plugin install openmage-skills@openmage

Updates land via /plugin marketplace update (each commit on main counts as a new version since no version is pinned in the manifest).

Install in any other agent (Cursor, Codex, OpenCode, …) — and Claude Code without the marketplace

vercel-labs/skills reads the same SKILL.md files and symlinks them into whatever agent layout is on disk (50+ agents detected automatically, including claude-code). No marketplace required:

# preview
bunx skills add OpenMage/magento-lts --list

# install all 27
bunx skills add OpenMage/magento-lts --all

# or pick one
bunx skills add OpenMage/magento-lts --skill openmage-eav

# target a specific agent (otherwise it auto-detects)
bunx skills add OpenMage/magento-lts -a cursor -a claude-code

npx skills add … works identically if you're not on Bun.

@sreichel
Copy link
Copy Markdown
Contributor

name: mage-module-promotions

... does not exist. Its CatalogRule ...

@ajbonner
Copy link
Copy Markdown
Contributor

have built out a similar CLAUDE.md / skills layout for our store

That's one question.. how do we include this in the repo? Should it just be right there in the root, or should it instead be published to a marketplace/plugin?

So the convex project have a great way using their convex cli to manage it and a skills-lock.json so the managing of skills / ai files is outside the main repo and the tooling is part of the main 'binary'. That wouldn't work so cleanly here as we dont have a blessed openmage cli tool (magerun prob will fit the bill as it's adopted to be more openmage specific in 3+ versions). Then you'd have magerun dev:skills:install magerun dev:skills:update and so forth and have it manage the install that way.

Also allows the skills / ai files to update far more regularly than the core project, though you then have versioning issues i suppose.

https://docs.convex.dev/ai/agent-skills

At the bottom of one of our apps' CLAUDE.md, convex skills adds a demarked portion

This project uses Convex as its backend.

When working on Convex code, always read convex/_generated/ai/guidelines.md first for important guidelines on how to correctly use Convex APIs and patterns. The file contains rules that override what you may have learned about Convex from training data.

Convex agent skills for common tasks can be installed by running npx convex ai-files install.

I quite like this approach, though it has to be said there's already one issue in that we mandate pnpm over npx so even that little snippet is subtly wrong.

@colinmollenhour
Copy link
Copy Markdown
Member Author

name: mage-module-promotions

... does not exist. Its CatalogRule ...

I don't think it hallucinated in this case, it just seems to have bundled Mage_CatalogRule, Mage_SalesRule and core Mage_Rule modules into one under that general title. You think it would be better named mage-module-rule?

Verified every factual claim in each SKILL.md against the codebase
and applied surgical fixes for ~157 findings: factually wrong method
names, fabricated APIs, wrong indexer aliases, stale event names,
inverted semantics, and assorted wording precision issues.

Highlights:
- mage-module-cms: corrected router target, dropped non-existent
  _addAfterFilterCallback, fixed widget instance ACL path, real
  page_groups enum, real <tempate_filter> typo
- mage-module-product-types: removed swatch_image/swatch_thumb (M2
  concepts), real canUseAttribute() rules, sales_order_save_commit_after
- mage-module-customer: default_billing/_shipping are EAV attrs not
  columns, group_id (not customer_group_id), corrected SHARE_GLOBAL/
  SHARE_WEBSITE polarity
- mage-module-payment-methods: dropped fabricated Centinel
  MethodInterface, real Centinel auth URLs, payment_transaction
  schema, distinguished _canCreateBillingAgreement vs interface
- mage-module-sales: corrected sales_payment_transaction table name,
  service_order::prepareInvoiceCreditmemo, dropped phantom
  sales_order_state_change_before event, shipment-vs-invoice
  independence
- mage-module-shipping-carriers: rate pipeline order, USPS uses
  <environment> (not <mode>), real getUps* getters
- openmage-eav: composite UNIQUE columns, _isAttributeValueEmpty()
  location, _storeValuesFlags semantics, addFieldToFilter behavior
- openmage-indexers-cron: removed fabricated catalogrule_product
  indexer, real index_event schema columns, --reindexallrequired
  semantics, process state labels
- openmage-layout-blocks: jsQuoteEscape (not escapeJsQuote),
  removed nonexistent escapeUrlAttribute, __() escaping reality
- openmage-module-structure: fixed malformed XML closing tags, real
  community modules list, MAGE_MODULES order, no alphabetical merge
- phpstan-magento1: dynamicCallOnStaticMethod via strict-rules (not
  bleedingEdge), per-identifier baselines (not globally suppressed),
  variable.undefined exemption (not empty.notAllowed), no _init()
  plugin handling
- rector-openmage: PHP set scope, ArrayFirstLastRector behavior,
  Zend_Log → Monolog (not Laminas), Zend_Measure → Mage helpers,
  RenameClassAndConstFetch (not RenameClassConstFetch)

Audited via /openmage-skills:audit; fixes applied via Python
exact-string replacement with manual review per finding.
@sreichel
Copy link
Copy Markdown
Contributor

sreichel commented May 1, 2026

I don't think it hallucinated in this case, [...] You think it would be better named mage-module-rule?

I think mage-module-* skills should follow module-structure, instead of combing them.

(no, its not important)

sreichel
sreichel previously approved these changes May 4, 2026
@sreichel
Copy link
Copy Markdown
Contributor

sreichel commented May 4, 2026

Better dont get lost in details. Approved.

@sreichel
Copy link
Copy Markdown
Contributor

sreichel commented May 4, 2026

@colinmollenhour pls merge your own.

@colinmollenhour
Copy link
Copy Markdown
Member Author

To mitigate concerns of these files interfering with other people's files I've added archive.exclude to the composer.json and also excluded a bunch of other files that don't need to be in the composer package (docs, tests, CNAME, dotfiles).

Also added a docs page with instructions for installing the skills (for people using the composer install since even without excluding them from the package it's likely they won't be useful in the vendor/ directory.

So I think that resolves my concerns with objections people might have had and nobody else has objected. :)

colinmollenhour and others added 3 commits May 5, 2026 01:41
Surgical fixes from a verification pass against the actual codebase:
exception class names, method default values, default-route resolution,
default cache backend, getCheckout() topology, _canReviewPayment behavior,
Zend rename targets (Monolog/Mage_Core_Helper, not laminas/Carbon),
dynamicCallOnStaticMethod attribution (strict-rules, not bleedingEdge),
and ~20 other minor factual corrections. Nine skills passed clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sreichel
Copy link
Copy Markdown
Contributor

sreichel commented May 5, 2026

Custom Migration rules (dev/rector/Migration/)

...
— don't fight the rename, accept it.

Maybe it should be clarfied (not only for AI) that migration rules are here to update 3rd-party-code as well.

  • deprecated / removed methods will be replaced to have no errors on recent code
  • ...

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

composer Relates to composer.json documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants