fix(db): ignore subdirectories and non-SQL files in migrations/

[agent-zhang-beihai]

What was broken

listLocalMigrationFilenames in src/lib/migrations.ts returned every entry of migrations/ via readdirSync, including subdirectories. Because db migrations new and db migrations up --all pass that list straight to parseStrictLocalMigrations, a subdirectory was validated as if it were a migration file.

With a migrations/_archive/ folder present (a common convention for holding superseded reference SQL), both commands failed:

Error: Invalid migration filename: _archive. Expected <migration_version>_<migration-name>.sql.

The only workaround was moving the archive folder out of migrations/.

What changed

listLocalMigrationFilenames now reads with withFileTypes: true and keeps only non-directory entries ending in .sql before sorting, so subdirectories and stray non-SQL files (e.g. README.md) never reach parseStrictLocalMigrations.

The filter uses !entry.isDirectory() rather than entry.isFile() deliberately: isFile() is false for symlinks, which would silently skip a symlinked migration file — a worse failure than the error being fixed. Misnamed *.sql files are still rejected exactly as before.

Scope is one function; no command surface, flags, or output shapes changed, so no InsForge/agent-skills update is needed.

How it was verified

New listLocalMigrationFilenames tests in src/lib/migrations.test.ts build a temp migrations/ containing _archive/ (with a .sql file inside), a valid 20260418091500_create-users.sql, and a README.md, and assert:

• only the valid .sql file is listed;
• parseStrictLocalMigrations succeeds and getNextLocalMigrationVersion picks the right next version (the db migrations new path);
• a misnamed *.sql file still throws Invalid migration filename;
• a missing migrations/ still returns [].

Confirmed the two new assertions fail on main and pass with the fix. Full suite green: npm run lint → 785 passed / 13 skipped, eslint clean.

Addresses user feedback 016dd35e-d192-40f2-bc7b-da45899e24ce (cli): db migrations new/up validate subdirectories in migrations/ as migration filenames

---

Summary by cubic

Fixes migration discovery by ignoring subdirectories and non-SQL files in migrations/. Previously we returned every entry, so a folder like migrations/_archive/ was validated as a migration filename and made db migrations new and db migrations up --all fail. Now we return only non-directory .sql entries; symlinked .sql files are still included. Addresses Linear feedback 016dd35e.

• Change is isolated to listLocalMigrationFilenames (uses readdirSync(..., { withFileTypes: true }) and !entry.isDirectory() + .sql suffix).
• Command surface and error shapes are unchanged; misnamed .sql files still error; subdirectories and files like README.md are ignored.
• Tests added in src/lib/migrations.test.ts cover subdirectories, non-SQL files, misnamed .sql, and missing migrations/. No rollout or migration actions required.

^{Written for commit 60ddbda. Summary will update on new commits.}

Note

Fix listLocalMigrationFilenames to skip subdirectories and non-SQL files in migrations/

Updates listLocalMigrationFilenames in migrations.ts to filter directory entries using withFileTypes: true, keeping only non-directory entries with .sql extensions. Symlinked .sql files are included because the filter uses !isDirectory() rather than isFile(). New tests in migrations.test.ts cover missing directories, mixed file types, and interaction with parseStrictLocalMigrations.

^{Macroscope summarized 60ddbda.}

[jwfing]

Review: fix(db): ignore subdirectories and non-SQL files in migrations/

Summary: Tightly-scoped, well-tested bug fix that makes listLocalMigrationFilenames skip subdirectories and non-.sql entries so a migrations/_archive/ folder (or a stray README.md) no longer aborts db migrations new / up --all.

Requirements context

Consulted docs/specs/2026-04-17-db-migrations-command-design.md (the db-migrations command design; InsForge/CLI keeps specs under docs/specs/, not /docs/superpowers/). The spec defines the migration filename contract as ^(\d{14})_([a-z0-9-]+)\.sql$ — lowercase .sql only. The fix is consistent with that contract, and the PR body ties the change to feedback item 016dd35e. No behavioral requirement in the spec is violated.

Findings

Critical

(none)

Suggestion

(none)

Information

• Functionality — silent skip of mis-cased .SQL files (src/lib/migrations.ts:126-129). The filter entry.name.endsWith('.sql') is case-sensitive, matching the lowercase-only MIGRATION_FILENAME_REGEX, so no valid migration can be dropped — good. One subtle behavior change worth noting: previously a file like 20260418091500_x.SQL reached parseStrictLocalMigrations and threw a loud Invalid migration filename error; now it is silently ignored. This is the same "silent skip vs. loud error" tradeoff the PR body calls out for symlinks — and here it lands on the silent side. It's a genuinely marginal case (uppercase extensions were never valid migrations), so this is informational only, not a request to change. If you wanted symmetry with the symlink reasoning you could match .sql case-insensitively and let the strict parser reject the casing, but the current behavior is defensible.

Notes on the four review dimensions

• Software engineering — Strong. Four new tests in src/lib/migrations.test.ts cover the fix (_archive/ + README.md skipped), the db migrations new integration path (parseStrictLocalMigrations + getNextLocalMigrationVersion), the regression that misnamed *.sql still throws, and the missing-dir [] case. Follows existing test conventions (temp dirs via mkdtempSync, node:fs/node:path imports). The !isDirectory()-over-isFile() choice for symlink safety is correct and well-commented. Verified locally: vitest run src/lib/migrations.test.ts → 38 passed; eslint on both changed files → clean.
• Functionality — Fix is complete. listLocalMigrationFilenames is the single source for all three callers (fetch dedup, new strict-parse, up), so filtering at the source covers every affected path. No command surface, flag, or output shape changed.
• Security — No security-relevant changes. Reads local directory entries only; no new user input reaches SQL/shell/HTTP, nothing new logged.
• Performance — No concern. Same single readdirSync with an added filter/map on a normally small directory; no extra I/O, no N+1.

Verdict

approved (informational; a human still gives the explicit GitHub approval). Zero Critical findings — clean, verified, in-scope fix with solid regression coverage.

[jwfing]

LGTM - approved.

[greptile-apps]

Greptile Summary

This PR narrows local migration discovery to SQL entries so archive directories and unrelated files no longer break migration commands.

• Uses Dirent metadata to exclude direct subdirectories and filters filenames by the .sql suffix.
• Adds temporary-directory tests covering archive folders, non-SQL files, strict validation, version generation, and missing migration directories.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking edge case remaining for validly named SQL symlinks that target directories.

Direct archive directories and non-SQL files are now excluded as intended, but the broad symbolic-link predicate can still pass a directory target into migration file reads.

Files Needing Attention: src/lib/migrations.ts

Important Files Changed

Filename	Overview
src/lib/migrations.ts	Filters migration directory entries before parsing, with a non-blocking edge case for symbolic links whose targets are directories.
src/lib/migrations.test.ts	Adds focused filesystem tests for directory and extension filtering while preserving strict SQL filename validation.

_{Reviews (1): Last reviewed commit: "fix(db): ignore subdirectories and non-S..." | Re-trigger Greptile}

[greptile-apps]

Directory-targeting symlinks pass filtering

A validly named .sql symlink to a directory passes !entry.isDirectory() because the Dirent describes the link itself; db migrations up then follows the link in readFileSync, fails with EISDIR, and can interrupt a batch after earlier migrations were applied.

Knowledge Base Used: Config Management