Repository navigation
feat(doctor): warn on legacy frontmatter drift (wish dir-sync-frontmatter-refresh group 6) - #1199
Conversation
…tter-refresh group 6)
Wave 3 / Group 6. Adds a new `checkLegacyAgentFrontmatter` check to
`genie doctor` that flags any agent directory where AGENTS.md starts
with a `---` fence AND `agent.yaml` is also present — the drift
scenario where someone pastes config back into the prompt file after
migration and sync silently ignores it.
Check semantics:
- Walks `<workspace>/agents/*`.
- Warn (not fail) when both files exist and AGENTS.md.slice(0, 3) === '---'.
- Pass when agent.yaml absent (pre-migration) OR no fence in AGENTS.md.
- Silent (empty results) when agents/ dir doesn't exist.
- Doctor exit code unchanged — warnings ≠ failures.
Exported for unit tests and for callers that want to run this check
outside `genie doctor` (e.g. a pre-sync lint).
Surface:
- `src/genie-commands/doctor.ts` — `checkLegacyAgentFrontmatter(workspaceRoot?)`
and a `runCheckSection('Agent Config', ...)` wired into `doctorCommand`.
- `src/genie-commands/doctor.test.ts` — 7 cases: drift warn, pre-migration
silent, clean post-migration silent, missing agents/ dir silent, multiple
agents flagged independently, non-dir entries skipped, yaml-only dirs pass.
Validation:
- `bun test src/genie-commands/doctor.test.ts` → 7 pass
- `bun run typecheck` clean
- `bunx biome check` clean modulo a mild complexity warning (17 vs 15)
Note: the wish spec said `src/term-commands/doctor.ts` but the actual
file lives at `src/genie-commands/doctor.ts`. Scope unchanged; file
location only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new diagnostic check, checkLegacyAgentFrontmatter, to the genie doctor command, along with comprehensive unit tests. The check identifies agent directories where legacy frontmatter in AGENTS.md might conflict with an existing agent.yaml. Review feedback suggests optimizing directory traversal by using readdirSync with withFileTypes: true to eliminate redundant statSync calls. Additionally, it was recommended to refine the frontmatter detection logic to include newline checks and to use a more memory-efficient approach than reading the entire file before slicing.
| */ | ||
|
|
||
| import { existsSync, unlinkSync } from 'node:fs'; | ||
| import { existsSync, readFileSync, readdirSync, statSync, unlinkSync } from 'node:fs'; |
There was a problem hiding this comment.
Since statSync is only used within checkLegacyAgentFrontmatter to check if an entry is a directory, and we can optimize that using readdirSync({ withFileTypes: true }), this import can be removed.
| import { existsSync, readFileSync, readdirSync, statSync, unlinkSync } from 'node:fs'; | |
| import { existsSync, readFileSync, readdirSync, unlinkSync } from 'node:fs'; |
| let entries: string[]; | ||
| try { | ||
| entries = readdirSync(agentsDir); | ||
| } catch { | ||
| return []; | ||
| } | ||
|
|
||
| for (const name of entries) { | ||
| const agentDir = join(agentsDir, name); | ||
| try { | ||
| if (!statSync(agentDir).isDirectory()) continue; | ||
| } catch { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
You can optimize this loop by using readdirSync(agentsDir, { withFileTypes: true }). This avoids a separate statSync call for every entry in the agents/ directory, which can be a significant performance improvement if there are many agents.
let entries;
try {
entries = readdirSync(agentsDir, { withFileTypes: true });
} catch {
return [];
}
for (const entry of entries) {
if (!entry.isDirectory()) continue;
const name = entry.name;
const agentDir = join(agentsDir, name);| let headContent: string; | ||
| try { | ||
| // Read only the first handful of bytes to cheaply detect the fence. | ||
| headContent = readFileSync(agentsMdPath, 'utf-8').slice(0, 4); | ||
| } catch { | ||
| continue; | ||
| } | ||
|
|
||
| if (!headContent.startsWith('---')) continue; |
There was a problem hiding this comment.
While using a hardcoded limit (magic number) to cap I/O is acceptable in non-critical logic, readFileSync(path, 'utf-8') reads the entire file into memory before slicing, which is inefficient for large files. Consider using a stream or a buffer-limited read to truly optimize performance. Additionally, ensure the fence check includes a newline to avoid false positives.
| let headContent: string; | |
| try { | |
| // Read only the first handful of bytes to cheaply detect the fence. | |
| headContent = readFileSync(agentsMdPath, 'utf-8').slice(0, 4); | |
| } catch { | |
| continue; | |
| } | |
| if (!headContent.startsWith('---')) continue; | |
| try { | |
| // Read only the first handful of bytes to cheaply detect the fence. | |
| const headContent = readFileSync(agentsMdPath, 'utf-8').slice(0, 4); | |
| if (!headContent.startsWith('---\n') && !headContent.startsWith('---\r')) continue; | |
| } catch { | |
| continue; | |
| } |
References
- It is acceptable to use hardcoded numeric limits (magic numbers) in non-critical fallback logic, especially when they serve as intentional caps to prevent performance issues like excessive I/O.
![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: 3f0f3efec1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| continue; | ||
| } | ||
|
|
||
| if (!headContent.startsWith('---')) continue; |
There was a problem hiding this comment.
This check flags any AGENTS.md whose first bytes start with ---, but the codebase’s frontmatter parser treats frontmatter as valid only when there is a proper opening line (---\n) and a closing fence. With agent.yaml present, files that start with a Markdown horizontal rule or an unclosed --- block will be reported as “legacy frontmatter” even though sync does not consider them frontmatter, producing persistent false warnings in genie doctor.
Useful? React with 👍 / 👎.
Wave 3 / Group 6 of wish `dir-sync-frontmatter-refresh`. Depends only on Group 1 (on dev).
What
`genie doctor` gains an "Agent Config" section that walks `agents/*` and warns when `AGENTS.md` starts with a `---` fence while `agent.yaml` is also present. Signals that someone pasted config back into the prompt file post-migration — sync silently ignores it, and without this check the drift goes unseen.
Check semantics
Warnings do NOT change exit code. Doctor's overall pass/warn/fail logic unchanged.
Code
Note on file location
Wish spec said `src/term-commands/doctor.ts` but the real file is at `src/genie-commands/doctor.ts`. Scope unchanged, only location.
Test plan
🤖 Generated with Claude Code