Use authored root AGENTS.md for shared instructions - #22
Conversation
🦋 Changeset detectedLatest commit: fa90f8d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c110f380c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| await planFileMigration( | ||
| path.join(legacyDir, PROJECT_AGENTS_FILE), | ||
| path.join(canonicalDir, PROJECT_AGENTS_FILE), | ||
| plannedWrites, | ||
| deletePaths, | ||
| conflicts, | ||
| ); | ||
| if (!(await pathExists(canonicalSkillsDir))) { |
There was a problem hiding this comment.
When an existing .claude project is upgraded, this migration still moves skiller.toml into .agents but now leaves .claude/AGENTS.md behind. Subsequent rule discovery uses the new .agents configuration and only reads the repository-root AGENTS.md, so projects without an already-authored root file silently lose all legacy shared instructions after their first apply; migrate the legacy file to the root when that destination is available, and handle conflicts explicitly.
Useful? React with 👍 / 👎.
| .filter((file) => file.path !== path.join(skillerDir, PROJECT_AGENTS_FILE)) | ||
| .sort((a, b) => a.path.localeCompare(b.path)); | ||
|
|
||
| for (const f of processedFiles) { | ||
| if (primaryFile && f.path === primaryFile.path) continue; | ||
| others.push(f); | ||
| if (path.basename(skillerDir) !== CANONICAL_SKILLER_DIR) { | ||
| return supplementalFiles; |
There was a problem hiding this comment.
For configurations created by skiller init --global, skillerDir is $XDG_CONFIG_HOME/skiller, and the initializer writes the instructions to that directory's AGENTS.md. This filter removes that file and the non-.agents early return prevents any replacement from being prepended, so applying a global-only configuration ignores the instructions that init --global just created.
Useful? React with 👍 / 👎.
|
|
||
| async applySkillerConfig( | ||
| concatenatedRules: string, | ||
| _concatenatedRules: string, |
There was a problem hiding this comment.
When an existing Aider configuration has output_path or output_path_instructions, this change stops generating that custom file but the later .aider.conf.yml logic still adds its basename to read. Aider therefore reads a missing or stale legacy file instead of the authored root AGENTS.md; now that instruction output overrides are intentionally ignored, the generated Aider configuration must also ignore those overrides.
Useful? React with 👍 / 👎.
| if (path.relative(presetRoot, sourcePath) === 'AGENTS.md') { | ||
| return 'AGENTS.md'; |
There was a problem hiding this comment.
A preset include such as ../../AGENTS.md resolves successfully but fails this exact relative-path check and then returns no target because AGENTS.md is absent from specialFileNames. This makes the newly documented AGENTS include support useful only for a file already inside the preset root, which is copied without an include anyway; recognize a terminal AGENTS.md like the lock files so shared external instructions can be materialized.
Useful? React with 👍 / 👎.
| const content = await fs.readFile(rootAgentsPath, 'utf8'); | ||
| return [{ path: rootAgentsPath, content }, ...supplementalFiles]; |
There was a problem hiding this comment.
On projects previously applied with Codex, Copilot, or another AGENTS-based adapter, the root AGENTS.md commonly starts with <!-- Generated by Skiller --> and already contains the concatenated .agents rules. Unconditionally treating that output as the new authored source while also appending the current supplemental files duplicates those rules in projected outputs and preserves stale copies after a supplemental rule changes; the upgrade path needs to recognize and migrate or reject the old generated blob before loading it as authored content.
Useful? React with 👍 / 👎.
| * Uses @filename references instead of concatenating content. | ||
| */ | ||
| export class ClaudeAgent extends AbstractAgent { | ||
| export class ClaudeAgent extends AgentsMdAgent { |
There was a problem hiding this comment.
Existing installations may already have a Skiller-generated CLAUDE.md containing @file references, but changing Claude's default output path to the no-op AGENTS adapter means neither a subsequent apply nor revert --agents claude-code knows that file is managed. Claude Code will continue loading the stale file alongside the new root instructions, and references may point at files moved by the .claude migration; retain a migration or revert path for recognizable legacy generated CLAUDE.md files.
Useful? React with 👍 / 👎.
| ): Promise<void> { | ||
| const output = | ||
| agentConfig?.outputPath ?? this.getDefaultOutputPath(projectRoot); | ||
| const absolutePath = path.resolve(projectRoot, output); | ||
| await ensureDirExists(path.dirname(absolutePath)); | ||
|
|
||
| // Add marker comment to the content to identify it as generated | ||
| const contentWithMarker = `<!-- Generated by Skiller -->\n${concatenatedRules}`; | ||
|
|
||
| // Read existing content if present and skip write if identical | ||
| let existing: string | null = null; | ||
| try { | ||
| existing = await fs.readFile(absolutePath, 'utf8'); | ||
| } catch { | ||
| existing = null; | ||
| } | ||
|
|
||
| if (existing !== null && existing === contentWithMarker) { | ||
| // No change; skip backup/write for idempotency | ||
| return; | ||
| } | ||
|
|
||
| // Backup (only if file existed and backup is enabled) then write new content | ||
| if (backup) { | ||
| await backupFile(absolutePath); | ||
| } | ||
| await writeGeneratedFile(absolutePath, contentWithMarker); | ||
| // Shared instructions are authored in place and need no projection. |
There was a problem hiding this comment.
For a new project containing root AGENTS.md plus supplemental .agents/*.md or .mdc rules, this no-op discards the concatenatedRules passed by the apply engine. Adapters inheriting this class—including Claude, Copilot, Cursor, Windsurf, Amp, Jules, and Zed—only consume the root file, so they never receive the supplemental rules that rule discovery and the documentation still advertise; retain an agent-consumable projection for supplemental content without overwriting the authored root.
Useful? React with 👍 / 👎.
| Write shared project instructions in the repository's root `AGENTS.md`. Skiller | ||
| reads that file when producing agent-specific rules and never rewrites it. |
There was a problem hiding this comment.
The newly added root-instructions feature is written as a prose paragraph at the top of the README, but the repository's README fork-feature convention requires additions in that section to be concise bullet points rather than verbose prose. Represent this statement using the required feature/bullet structure.
AGENTS.md reference: AGENTS.md:L7-L15
Useful? React with 👍 / 👎.
1 participant
Summary
AGENTS.mdas the authored shared instruction source for current agents.AGENTS.mdandCLAUDE.md; move project Skiller configuration to.agents/skiller.toml.pnpm-lock.yamlfor frozen installs.Verification
pnpm install --frozen-lockfilenpm run lintnpm test— 128 suites / 808 tests passednpm run buildnpm ciis not applicable because this pnpm repository has nopackage-lock.json.