Repository navigation
test(dir-sync-frontmatter-refresh): integration + reproducer + gitignore + changelog (wish group 7) - #1201
Conversation
…ore + changelog (wish group 7)
Wave 4 / Group 7. Ships the end-to-end validation layer for the
dir-sync-frontmatter-refresh wish and completes it.
Deliverables:
1. **`src/__tests__/agent-yaml-migration.integration.test.ts`** — 7
cases composing G1-G6 into full-flow assertions:
- Today's reproducer (edit agent.yaml → re-sync → DB updates in one
breath, "Unchanged" literal asserted absent from stdout).
- Migration preserves body byte-for-byte in .bak (plain, CRLF,
Unicode).
- `permissions.bashAllowPatterns` round-trip (using the exact TS
field name, not the CLI flag name).
- Idempotent sync: second run skips migration, still upserts, still
no "Unchanged".
- Post-migration drift: re-adding frontmatter to AGENTS.md → doctor
warns via `checkLegacyAgentFrontmatter`.
- Fresh agent with no frontmatter syncs with defaults.
2. **`scripts/tests/repro-yaml-config.sh`** — shell-level reproducer
mirroring the integration test against the live pgserve. Uses an
ephemeral test schema (same isolation pattern as
`repro-canonical-uuid.sh`). Exits 0 when every invariant holds.
3. **`.gitignore`** — adds `agents/*/AGENTS.md.bak` and
`**/agents/*/AGENTS.md.bak` so the migration-safety backup files
never leak into source control.
4. **`CHANGELOG.md`** — new Unreleased entries covering: the file-
shape change (AGENTS.md body-only + agent.yaml canonical), the
unconditional upsert in dir sync, the yaml-first edit/add flow,
the doctor drift warning, zero-touch migration on first sync post-
upgrade, and downgrade safety via .bak restore.
Full suite pre-push green. The wish is shippable after this.
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 migrates agent configuration from AGENTS.md frontmatter to dedicated agent.yaml files, establishing disk files as the source of truth for runtime fields. The changes include automatic migration logic, updated CLI commands for synchronization and editing, and the addition of comprehensive integration tests and a shell reproducer. Review feedback suggests improving test helpers by addressing a potential directory leak in the createAgent function and ensuring the captureStdout utility also intercepts console.error to better capture failure details.
| function createAgent(name: string, agentsMdContent: string): string { | ||
| workspaceRoot = join(tmpdir(), `integ-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| const agentDir = join(workspaceRoot, 'agents', name); | ||
| mkdirSync(agentDir, { recursive: true }); | ||
| writeFileSync(join(agentDir, 'AGENTS.md'), agentsMdContent); | ||
| return agentDir; | ||
| } |
There was a problem hiding this comment.
The createAgent helper re-assigns the shared workspaceRoot variable on every call. This design leads to directory leaks in /tmp if a test case calls createAgent multiple times (as only the last directory is tracked for cleanup in afterEach). It also prevents testing multiple agents within the same workspace. Reusing the same workspaceRoot if it is already initialized for the current test solves both issues.
References
- Ensure that temporary test resources are properly tracked and cleaned up to avoid resource leaks.
| async function captureStdout<T>(fn: () => Promise<T>): Promise<{ value: T; output: string }> { | ||
| const chunks: string[] = []; | ||
| const origLog = console.log; | ||
| const origWarn = console.warn; | ||
| console.log = (...args: unknown[]) => { | ||
| chunks.push(args.map(String).join(' ')); | ||
| }; | ||
| console.warn = (...args: unknown[]) => { | ||
| chunks.push(args.map(String).join(' ')); | ||
| }; | ||
| try { | ||
| const value = await fn(); | ||
| return { value, output: chunks.join('\n') }; | ||
| } finally { | ||
| console.log = origLog; | ||
| console.warn = origWarn; | ||
| } | ||
| } |
There was a problem hiding this comment.
The captureStdout helper intercepts console.log and console.warn but misses console.error. Since the agent sync logic and printSyncResult use console.error to report failures, tests using this helper might fail to capture or display critical error information during debugging or assertion failures. It is safer to capture all standard output and error streams.
![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: f1569e3f1a
ℹ️ 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".
| const { syncAgentDirectory, printSyncResult } = await import('./src/lib/agent-sync.js'); | ||
| const postgres = (await import('postgres')).default; | ||
| // Use the schema-scoped connection for the sync path. | ||
| process.env.GENIE_PG_SCHEMA = process.env.TEST_SCHEMA; |
There was a problem hiding this comment.
The script exports GENIE_PG_SCHEMA, but the DB layer only honors GENIE_TEST_SCHEMA when setting search_path (src/lib/db.ts). As a result, both sync runs execute against the default schema instead of the ephemeral test schema, so this reproducer can mutate real public data and produce non-hermetic results despite the isolation claim.
Useful? React with 👍 / 👎.
| bun --cwd "$REPO_ROOT" -e " | ||
| const postgres = (await import('postgres')).default; | ||
| const port = Number.parseInt(process.env.GENIE_PG_PORT || '19642', 10); | ||
| const sql = postgres({ host: '127.0.0.1', port, database: 'postgres', user: 'postgres' }); |
There was a problem hiding this comment.
The test schema is created (and later migrated/dropped) in database: 'postgres', but the application code under test connects to the genie database (DB_NAME in src/lib/db.ts). This means the script provisions isolation in a different database than the one syncAgentDirectory actually uses, so schema setup/cleanup does not protect the data path being exercised.
Useful? React with 👍 / 👎.
Wave 4 / Group 7 of wish `dir-sync-frontmatter-refresh`. Depends on G1-G6 (all on dev). Completes the wish.
What ships
Integration test coverage
Reproducer script
`scripts/tests/repro-yaml-config.sh` mirrors the integration test against the live pgserve using an ephemeral test schema. Exits 0 when every invariant holds. Usable from CI or by users wanting to smoke-test their install.
Test plan
Wish completion
After #1200 (this PR) merges, the wish `dir-sync-frontmatter-refresh` is fully shipped:
🤖 Generated with Claude Code