ci: add dynamic import verification to catch stale paths#540
ci: add dynamic import verification to catch stale paths#540carlos-alm merged 5 commits intomainfrom
Conversation
Add scripts/verify-imports.js that scans all await import() calls in src/ and verifies each relative path resolves to an existing file. Runs as a new CI job (verify-imports) in the pipeline gate. Also fixes two stale imports found by the script: - src/mcp/tools/semantic-search.js: ../../embeddings/index.js → ../../domain/search/index.js (embeddings was moved to domain/search) - src/cli/commands/info.js: ../../db.js → ../../db/index.js (db.js was split into db/ directory) Closes roadmap item 10.3.
|
Claude finished @carlos-alm's task —— View job PR Review Complete ⭐⭐⭐☆☆ (3/5)Reviewing PR #540: ci: add dynamic import verification to catch stale paths Root Cause AnalysisProblem Being Solved: Runtime crashes caused by stale dynamic Backlog Compliance
Code Quality Issues Found1. Regex Limitation
|
Greptile SummaryThis PR adds a zero-dependency static analysis script ( The core scanner logic is solid — it correctly handles mid-line
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([node scripts/verify-imports.js]) --> B[walk src/\ncollect .js/.ts files]
B --> C{for each file}
C --> D[extractDynamicImports\nline-by-line scan]
D --> E{line in\nblock comment?}
E -- yes, no close --> F[skip line]
E -- yes, close found --> G[slice after */\nfall through]
E -- no --> H{line starts\nwith // ?}
H -- yes --> F
H -- no --> I[strip inline\n/* ... */ blocks]
I --> J{unclosed /* ?}
J -- yes --> K[truncate line\nenter block mode]
J -- no --> L[run DYNAMIC_IMPORT_RE\nagainst scanLine]
G --> L
K --> F
L --> M{isInsideLineComment\nbefore match?}
M -- yes --> N[skip import]
M -- no --> O[collect specifier]
O --> P[resolveSpecifier]
P --> Q{starts with\n. or / ?}
Q -- no --> R[skip bare specifier\nnull = OK]
Q -- yes --> S{exact file\nexists?}
S -- yes --> R
S -- no --> T{no extension?\ntry .js/.ts/.mjs/.cjs}
T -- found --> R
T -- not found --> U{is directory?\ntry index files}
U -- found --> R
U -- not found --> V[broken import]
V --> W[broken list]
R --> C
N --> C
F --> C
W --> X{any broken?}
X -- no --> Y([exit 0 ✓])
X -- yes --> Z([exit 1 ✗ print paths])
Last reviewed commit: "fix: scan content af..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
scripts/verify-imports.js
Outdated
| const before = line.slice(0, match.index); | ||
| if (before.includes('//') || before.includes('/*')) continue; |
There was a problem hiding this comment.
// inside string literals silently skips real imports
The check before.includes('//') is meant to detect trailing // comments, but it matches any occurrence of // in the preceding text — including URLs embedded in string literals. For example:
const base = 'https://example.com'; const { mod } = await import('./missing-path.js');The before slice here contains //, so the import is silently skipped and the broken path goes undetected — exactly the opposite of what this script is meant to do.
A safer heuristic is to check whether // appears outside quoted strings, or at minimum check that it is preceded only by whitespace or non-string characters. A simple approach is to scan for the first unquoted //:
| const before = line.slice(0, match.index); | |
| if (before.includes('//') || before.includes('/*')) continue; | |
| // Skip if the match is inside a trailing comment (simple heuristic: look for // outside quotes) | |
| const before = line.slice(0, match.index); | |
| const trailingCommentIdx = before.search(/(?<!['"]\S*)\/\//); | |
| if (trailingCommentIdx !== -1 || before.includes('/*')) continue; |
Alternatively, use a stricter check that only skips when // or /* is not preceded by a quote character on the same line.
There was a problem hiding this comment.
Fixed — replaced naive \ with a new \ helper that walks character-by-character tracking quote state (single, double, backtick). Only flags \ that appears outside string literals, so URLs like \ no longer cause false negatives.
scripts/verify-imports.js
Outdated
| if (/^\s*\/\*/.test(line)) { | ||
| if (!line.includes('*/')) inBlockComment = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
/* openings
inBlockComment is only set when a line matches /^\s*\/\*/ — i.e., /* at the beginning of the line. A /* that appears after real code is ignored:
const x = callSomething(); /* begin multi-line comment
await import('./stale-path.js') ← this line IS inside a comment but WILL be scanned
*/On the line containing await import('./stale-path.js') the state inBlockComment is still false, so it gets scanned and could produce either a false positive (valid path inside comment flagged) or miss a stale path in code.
Consider tracking the opening position more broadly:
| if (/^\s*\/\*/.test(line)) { | |
| if (!line.includes('*/')) inBlockComment = true; | |
| continue; | |
| } | |
| if (!inBlockComment && line.includes('/*')) { | |
| if (!line.includes('*/')) inBlockComment = true; | |
| // Still check content before the /* | |
| } |
Or strip block comments from the line before applying the regex, rather than using a line-level guard.
There was a problem hiding this comment.
Fixed — removed the line-start-only ^\s*\/\* check. Block comments are now stripped from each line before regex scanning: inline /* ... */ pairs are removed with a greedy replace, and if an unclosed /* remains, we truncate the line at that point and enter inBlockComment mode. This correctly handles mid-line openings like const x = foo(); /* begin comment.
scripts/verify-imports.js
Outdated
|
|
||
| // ── extract dynamic import specifiers ─────────────────────────────────── | ||
| // Matches: await import('...') and await import("...") | ||
| const DYNAMIC_IMPORT_RE = /await\s+import\(\s*(['"])(.+?)\1\s*\)/g; |
There was a problem hiding this comment.
import() calls are invisible to this script
The regex only matches await import(...). A dynamic import that is returned, .then()-chained, or assigned without await will be entirely skipped:
return import('./stale-path.js'); // not caught
import('./stale-path.js').then(doSomething); // not caughtIf the codebase ever uses these patterns, a stale path would bypass this check entirely. Consider broadening the regex to match bare import(...) calls as well:
| const DYNAMIC_IMPORT_RE = /await\s+import\(\s*(['"])(.+?)\1\s*\)/g; | |
| const DYNAMIC_IMPORT_RE = /(?:await\s+)?import\(\s*(['"])(.+?)\1\s*\)/g; |
There was a problem hiding this comment.
Fixed — broadened the regex from /await\s+import\(...\)/ to /(?:await\s+)?import\(...\)/ so it now catches bare import() calls: return import(...), import(...).then(...), and variable assignments without await.
- Broaden regex to catch non-awaited import() calls (return, .then())
- Fix block-comment tracking to handle mid-line /* openings
- Replace naive before.includes('//') with quote-aware isInsideLineComment
to avoid false negatives on URLs in string literals
|
All three Greptile review items addressed in ab315f7:
Script still passes: all 262 src/ files scanned, 0 broken imports. |
| if (inBlockComment) { | ||
| if (line.includes('*/')) inBlockComment = false; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
*/ on same line is never scanned
When the closing */ of a block comment appears mid-line (e.g. */ await import('./stale.js');), inBlockComment is set to false but the continue unconditionally skips the rest of the current line. Any real import() call after the */ is invisible to the scanner — exactly the kind of false negative this tool is meant to prevent.
// current (buggy):
if (inBlockComment) {
if (line.includes('*/')) inBlockComment = false;
continue; // ← skips ALL of the current line, including code after */
}A minimal fix is to skip the continue when */ is found and instead pass the post-close slice into scanLine:
if (inBlockComment) {
const closeIdx = line.indexOf('*/');
if (closeIdx === -1) continue; // still fully inside a block comment
inBlockComment = false;
// fall through with only the part of the line that follows '*/'
scanLine = line.slice(closeIdx + 2);
// skip re-assigning scanLine = line below, then proceed to import scanning
}Because scanLine is currently declared a few lines later (let scanLine = line;), the cleanest approach is to hoist the declaration and branch early, similar to how the block-comment stripping already works for mid-line /* openings.
There was a problem hiding this comment.
Fixed in 3ee1b89 — when */ is found mid-line, the scanner now extracts the content after the close (scanLine = line.slice(closeIdx + 2)) and falls through to the import scanning logic instead of unconditionally skipping the line with continue.
When */ appeared mid-line, the continue statement skipped the entire line including any real code after the comment close. Now extracts and scans the post-close portion of the line.
1 participant
Summary
scripts/verify-imports.js— scans allawait import()calls insrc/, resolves relative paths, and fails if any target file doesn't existverify-importsCI job to.github/workflows/ci.yml(no npm install needed — zero-dependency script)src/mcp/tools/semantic-search.js:../../embeddings/index.js→../../domain/search/index.js(3 occurrences)src/cli/commands/info.js:../../db.js→../../db/index.jsCloses roadmap item 10.3 — the ast command crash was caused by a stale import path in compiled output; this CI step prevents that class of bug.
Test plan
node scripts/verify-imports.jsexits 0 on clean codebaseverify-importsjob passes in pipelinecodegraph inforuns without crash (stale db.js import fixed)