fix(agentsmd): import nested AGENTS.md files and stop competing for .agents/skills/ - #2436
Merged
dyoshikawa merged 6 commits intoJul 27, 2026
Conversation
…agents/skills/ Two pre-existing divergences from the AGENTS.md standard. Nested AGENTS.md files were silently dropped on import. They are the standard's only scoping mechanism — agents read the nearest file in the directory tree, so the closest one wins — and export already honored them via `agentsmd.subprojectPath`, but import enumerated only the root file and `.agents/memories/**`. A project with `packages/api/AGENTS.md` imported one rule and lost the other without a word. `AgentsMdRule` now exposes `getNestedFileGlobs`, a new optional hook the RulesProcessor uses to enumerate rule files by pattern rather than at a fixed path. The scan skips hidden directories (other tools' generated output, including rulesync's own) and `node_modules/`, and is import-only: a nested file rulesync did not write must never be swept up by `--delete`. Each match imports to `.rulesync/rules/<directory-with-hyphens>.md` carrying `subprojectPath`, so the next generate puts it back where it came from. `fromFile` also stops treating a modular file literally named `AGENTS.md` under `.agents/memories/` as the project root file, which made it read the wrong file entirely. `.agents/skills/` was written by both `agentsmd` and `agentsskills`. That path is not an AGENTS.md convention at all — the standard defines only `AGENTS.md` — it is the Agent Skills project location. Both targets resolved to the same file, so `--targets agentsmd,agentsskills` and `--targets agentsskills,agentsmd` produced different content: the simulated writer kept only name/description and dropped `license`, `compatibility`, `metadata` and `allowed-tools`. The simulated writer now emits exactly what the native one emits, through the same `toSpecConformantAgentSkillFields` helper, so enabling both writes one identical file instead of two competing ones. Removing the convention outright was the other option, but `agentsmd` is the only simulated-skills target, so that would have left `--simulate-skills` with nothing to do. Closes #2428 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Security review of the nested-file scan.
The scan walks the whole project tree, and `findFilesByGlobs` follows symbolic
links by default while returning the unresolved path — so `checkPathTraversal`,
which is purely lexical, saw an in-project path and let it through. A repository
could commit `docs/AGENTS.md` as a symlink to `../../../.ssh/id_rsa`; importing
it copied the key into `.rulesync/rules/docs.md`, which this tool's whole design
expects the user to commit. Confirmed by reproducing it with the repo's own
helpers. The same default also made two directory symlinks pointing at a shared
parent explode the traversal until the process ran out of heap.
The scan now passes `followSymbolicLinks: false`, which drops both the symlinked
file and the symlinked directory (verified). Fixed-path scans are unaffected —
they only ever look inside rulesync-owned directories.
Also from the review:
- The root file's exclusion never worked. globby rewrites a negative pattern
containing no glob metacharacter as cwd-relative, so `!` + an absolute path
silently matches nothing. The hook now returns `{ include, ignore }` and
`findFilesByGlobs` gained an `ignore` option that goes straight to globby,
which has no such rewriting. The test no longer asserts on the pattern
strings — it runs them against a real tree, which is what would have caught
this.
- The exclusion set covers dependency, vendoring and build trees beyond
`node_modules/` (`vendor`, `third_party`, `dist`, `build`, `out`, `target`,
`coverage`, `tmp`, `temp`, `venv`, `__pycache__`). Those are usually
gitignored, so importing from them moves content the user deliberately kept
out of the repository into a tracked directory.
- Two directories deriving the same rulesync file name (`packages/api` and
`packages-api`) are reported at import time instead of one silently
overwriting the other.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… cover every .agents/skills/ writer Code review round 1. HIGH — an `overview/` subproject claimed the reserved `overview.md` name, so its content overwrote the imported root rule. The root rule then no longer existed, and the next `generate --delete` removed the project's `AGENTS.md` outright. A subproject whose derived name would be `overview.md` now gets an `-agents` suffix, and the duplicate check moved from "nested versus nested" to every rule being written, so a collision with a `.agents/memories/` file or a hand-written rule is reported too. MID — the exclusion list dropped real subprojects. `build`, `dist`, `vendor`, `tmp` and friends were excluded at any depth, so `packages/build/AGENTS.md` — a package, not a build directory — vanished from the import without a word. Those names are now excluded at the project root only; `node_modules` and `__pycache__`, which are never package names, stay excluded at any depth. MID — `.agents/skills/` has four writers, not two. `aiassistant` and `codexcli` also target it and also emitted only `name`/`description`, so the same order-dependent frontmatter loss this PR set out to fix was still reachable via `--targets agentsskills,aiassistant`. Both now go through `toSpecConformantAgentSkillFields`; Codex's own `short-description` metadata is merged into the shared map rather than replacing it. MID (recorded, not fixed) — `--delete` leaves nested `AGENTS.md` files behind, so a deleted rulesync rule stops being referenced from the root file while the subproject file keeps being read. Deleting them is not an option: they are the user's own files anywhere in the tree, which is exactly why the scan is import-only. Warning instead would need a full-tree scan on every generate, for a condition that predates this PR (it already applied to hand-written `subprojectPath` rules). Documented as a stated limitation instead. LOW — the nested-file note moved from the skills section to the rules section; the `## Symlinks` section now records that this one scan is the exception; `getNestedFilePatterns` is skipped entirely in global mode, where the output root is the home directory; `fromFile` stores the directory with native separators like every other construction path; and the duplicate check no longer converts each rule twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ls fix to the simulated writer Security re-review: - The nested scan honors `.gitignore` (`findFilesByGlobs` gained `cwd` and `gitignore` options). Narrowing the build/vendor exclusions to the project root reopened a real path: a gitignored `services/api/vendor/` could carry a third-party `AGENTS.md` into version-controlled `.rulesync/rules/`, where a target that concatenates non-root rules into one always-loaded file would then apply it project-wide. Git's own statement of what is not your source is a better rule than any name list, and it keeps `packages/build/` — the case the name list was narrowed for. Ignore rules resolve from the enclosing repository, so the nested tests now create a `.git` directory: without one, this repo's ignored `tmp/` hides the whole test project. Code re-review: - The reserved-name guard and the duplicate check are case-insensitive. On a case-insensitive filesystem an `Overview/` subproject derived `Overview.md`, which is the root rule's `overview.md` — the same silent root-rule loss the previous commit fixed for the lowercase spelling, and the duplicate check did not catch it either. - The shared-frontmatter normalization is reverted for `aiassistant` and `codexcli`. Nine targets write `.agents/skills/`, and eight of them are native with their own documented frontmatter block; feeding them the `agentsskills` block made them write fields their own `toRulesyncSkill` does not read, so generate → import became lossy where it had been symmetric. Which rulesync block should feed a file that many native targets share is a design decision, not a bug fix, and a partial answer is worse than none. `agentsmd` stays fixed, because it is not the same case: its skills support is *simulated* — the AGENTS.md standard defines no skills — so it has no frontmatter of its own to contribute and was purely degrading a native target's output. "A simulated writer must not degrade a path a native target owns" is the rule this PR enforces. The docs now state the general situation. - Tests: the reserved-name guard on a mixed-case directory, the no-collision case (guarding against false-positive warnings), and `logger.warn.mockClear()` so the shared mock cannot make an assertion pass by accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sted scan Review round 3. MID — `gitignore: true` on the glob collided head-on with rulesync's own tooling. `rulesync gitignore` derives `**/AGENTS.md` from the agentsmd root path, and this repository's `.gitignore` has exactly that line; a file-level ignore test therefore excluded every nested match, so this PR's whole feature returned zero results for any project that ran the recommended command — and said nothing but a debug line. The check now runs against the *directories* above each file (`filterOutPathsInGitIgnoredDirectories`), which still skips a vendored tree such as `services/api/vendor/` while surviving a pattern aimed at the generated file name. `findFilesByGlobs` loses the `gitignore`/`cwd` options again; the shared util should not carry a knob only one caller can use safely. LOW — the "no-op outside git" claim was wrong: globby falls back to the directory's own `.gitignore` files when there is no repository, so the docs now say so. The collision warning no longer claims one file always wins, since the comparison is case-insensitive while a case-sensitive filesystem keeps both. The `.git` test setup moved into one `asRepositoryRoot` helper, and the docs paragraph that had escaped its blockquote is back inside it. Tests: the new util is covered directly (ignored directory, a rule matching the files themselves, and a project with no rules), plus a processor-level regression for the `**/AGENTS.md` case that motivated this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…or walk
Review round 4 — no mid-or-above findings; these are the lows.
- globby's `isGitIgnoredSync` reads the `.gitignore` files at and below `cwd`;
it does not search upward for a repository root. The JSDoc and docs said the
opposite, and the `.git` directory the tests created to "become a repository
root" was a no-op — the tests passed either way, which is the proof. Both are
corrected, the `.git` setup is gone, and the docs note the consequence: a
run against a subdirectory only sees that subdirectory's own ignore rules.
- `filterOutPathsInGitIgnoredDirectories` recursed forever on a path outside
`rootDir`, because `dirname("/")` is `"/"` and the cache was written after the
recursive call. The only caller cannot reach it, but the helper is exported
and `resolve()` turns a relative path into one rooted at the process cwd.
Guarded, with a test.
- The helper returns early for an empty input; building the matcher scans the
tree for `.gitignore` files, which is not worth doing with nothing to filter.
- The simulated `agentsmd` writer now reports the same spec violations the
native writer reports. Emitting the same file while staying silent about it
contradicted this PR's own reasoning, and `--targets agentsmd` alone got no
diagnostics at all.
- Documented that testing directories rather than files means ignoring one
individual `AGENTS.md` no longer keeps it out of the import.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dyoshikawa
deleted the
resolve-scrap-issue-2428-agentsmd-nested-and-skills-path
branch
July 27, 2026 09:53
Owner
Author
|
@dyoshikawa Thank you! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Background
The AGENTS.md standard has had no normative change in 4+ months (spec repo
agentsmd/agents.md, no releases or tags, last commit 2026-03-12 and site-only). Re-checked against https://agents.md/ on 2026-07-27: the standard defines a rootAGENTS.md, plain Markdown with no frontmatter, and nestedAGENTS.mdin subdirectories with nearest-file-wins precedence — and defines no.agents/subdirectory conventions and no global scope.Two pre-existing divergences, both reproduced before fixing.
1. Nested
AGENTS.mdfiles were silently dropped on importNested files are the standard's only scoping mechanism: "Agents automatically read the nearest file in the directory tree, so the closest one takes precedence and every subproject can ship tailored instructions."
Export already honored them via
agentsmd.subprojectPath. Import did not — it enumerated the root file plus.agents/memories/**and nothing else:AgentsMdRulenow exposesgetNestedFileGlobs, a new optional hook theRulesProcessoruses to enumerate rule files by pattern rather than at a fixed path (the existinggetExtraFixedFileshook only covers known paths). Each match imports to.rulesync/rules/<directory-with-hyphens>.mdcarryingagentsmd.subprojectPath, so generate puts it back where it came from.Two deliberate constraints:
--deletewould sweep away work rulesync never wrote.node_modules/are skipped. AnAGENTS.mdinside a dot-directory is another tool's generated output (rulesync writes several itself); one undernode_modules/describes somebody else's project.fromFilealso stops treating a modular file literally namedAGENTS.mdunder.agents/memories/as the project root file — it previously read<root>/AGENTS.mdinstead of the memory file.After:
2. The simulated
agentsmdskills writer degraded.agents/skills/.agents/skills/is not an AGENTS.md convention — it is the Agent Skills project location. Nine targets write there, because they all implement that convention; each native one writes its own documented frontmatter, so enabling several and reordering--targetscan change which optional keys survive. That is inherent to several tools sharing one path.agentsmdis the exception, and the one this PR fixes. Its skills support is simulated — the AGENTS.md standard defines no skills at all — so it has no frontmatter model of its own, yet it overwrote the native output with a barename/descriptionpair:...and with the targets reversed, the simulated writer ran last and stripped
license,compatibility,metadataandallowed-tools.It now emits exactly what
agentsskillsemits, through the sametoSpecConformantAgentSkillFieldshelper, so both orders produce one identical file:The rule enforced here is narrow and defensible: a simulated writer must not degrade a path a native target owns. Making the eight native targets share one frontmatter block instead is a design decision about which rulesync block feeds a shared file, not a bug fix — a partial answer would make their
generate→importround trip lossy, so it is deliberately out of scope.Dropping the
agentsmdskills convention outright was the issue's other option.agentsmdis the only simulated-skills target, so removing it would leave--simulate-skillswith nothing to do — a much larger product change than this issue calls for.Tests
src/features/rules/agentsmd-rule.test.ts— the glob set, nested import, the subproject round trip, and that neither the root file nor a.agents/memories/AGENTS.mdis mistaken for a subproject.src/features/skills/agentsmd-skill.test.ts— the simulated writer's frontmatter equals the native writer's for the same input.src/e2e/e2e-rules.spec.ts— nested import → generate round trip, withnode_modules/and.agents/proven excluded.Security review outcomes
The scan walks the whole project, so it was reviewed for that. It does not follow symbolic links — a link committed to a repository could otherwise pull a file from outside the project (a key, a dotfile) into version-controlled
.rulesync/— and it honors.gitignore, so a vendored dependency's rule file is not copied into a tracked directory. A subproject that would claim the reservedoverview.mdname gets an-agentssuffix (compared case-insensitively), because overwriting the root rule would make the next--deleteremove the project'sAGENTS.md.pnpm cicheckpasses.Closes #2428