mirror of
https://github.com/giancarloerra/socraticode.git
synced 2026-07-03 14:05:21 +02:00
fix(graph): pre-validate ast-grep grammar libraryPath to survive missing prebuilds
Resolves #43. On Linux/Node combinations where one ast-grep grammar package's prebuilt parser binary is missing for the host architecture (reporter: Ubuntu 24.04 + WSL2 + Node 24, missing the cpp prebuild), the existing loader silently failed to register every dynamic grammar, not just the broken one. Mechanism: registerDynamicLanguage iterates the modules it receives and accesses the lazy libraryPath getter on each. If that getter throws (because the prebuild lookup fails), the entire batch call aborts atomically — none of the 13 dynamic grammars get registered. The empty inner `catch {}` in ensureDynamicLanguages discarded the underlying reason, and the failure surfaced only at WARN as a generic "Failed to register dynamic ast-grep languages", with no indication of which grammar was the culprit. Empirically verified against @ast-grep/napi@0.40.5 in a clean Node environment: - Sequential register({A}); register({B}) calls REPLACE rather than accumulate; only the last call's grammar survives. So the obvious alternative of "register one at a time" is actively broken — it would silently leave only the last grammar in langPackages registered (php, since it's last in our list). On a polyglot codebase that's a regression. - Batch register({A, B, C}) with one entry whose libraryPath getter throws aborts atomically — none of A/B/C end up registered. The fix is to pre-validate each grammar's libraryPath getter inside our own per-grammar try/catch, exclude any that throw, then make ONE batch registerDynamicLanguage call with only the survivors: - src/services/code-graph.ts: ensureDynamicLanguages now touches mod.libraryPath inside the inner try/catch so a missing-prebuild failure is contained to that one grammar. Adds module-level loadedDynamicLanguages and failedDynamicLanguages state plus a public getDynamicLanguageStatus() introspection API. The empty `catch {}` becomes `catch (err) { ... }` capturing the actual reason. Per-grammar failures and the (now-rare) catch-all log at warn level with the underlying message. - src/tools/graph-tools.ts: codebase_graph_status now appends an "AST grammars: Loaded / Failed" block listing both sets, with the failure reason for each unloaded grammar. Users see the loader state without needing to enable debug logging. - src/services/graph-symbols.ts: extractSymbolsAndCalls bumps its failure log from DEBUG to WARN with one-shot dedupe per language — one warn per language for the lifetime of the process, not one warn per file (potentially hundreds). - src/services/graph-imports.ts: extractImports gets the same one-shot dedupe per language treatment for consistency. - tests/unit/dynamic-language-loader.test.ts: nine new tests covering the sync contract, idempotence, loaded/failed sort order, disjoint sets, and a sanity check that at least one expected grammar registers in the test environment. The loader stays sync. createRequire stays. No call-site signature changes, no test fixture rewrites. Existing 721 tests pass; with the new file, 730 tests pass. typecheck and biome clean. CodeRabbit review of the diff returned no findings. Co-authored-by: X-Adam <X-Adam@users.noreply.github.com>
This commit is contained in:
@@ -445,13 +445,38 @@ export async function getGraphStatus(projectPath: string): Promise<{
|
||||
// ── Register dynamic language grammars ───────────────────────────────────
|
||||
|
||||
let dynamicLangsRegistered = false;
|
||||
const loadedDynamicLanguages = new Set<string>();
|
||||
const failedDynamicLanguages = new Map<string, string>();
|
||||
|
||||
/** Module export shape exposed by `@ast-grep/lang-*` packages. */
|
||||
interface AstGrepLangModule {
|
||||
libraryPath: string;
|
||||
extensions: string[];
|
||||
languageSymbol?: string;
|
||||
}
|
||||
|
||||
/** Snapshot of dynamic-language registration state, for diagnostics. */
|
||||
export interface DynamicLanguageStatus {
|
||||
loaded: string[];
|
||||
failed: Array<{ name: string; error: string }>;
|
||||
}
|
||||
|
||||
/** Returns which dynamic ast-grep grammars registered successfully and which failed. */
|
||||
export function getDynamicLanguageStatus(): DynamicLanguageStatus {
|
||||
return {
|
||||
loaded: [...loadedDynamicLanguages].sort(),
|
||||
failed: [...failedDynamicLanguages.entries()]
|
||||
.sort(([a], [b]) => a.localeCompare(b))
|
||||
.map(([name, error]) => ({ name, error })),
|
||||
};
|
||||
}
|
||||
|
||||
export function ensureDynamicLanguages(): void {
|
||||
if (dynamicLangsRegistered) return;
|
||||
dynamicLangsRegistered = true;
|
||||
|
||||
try {
|
||||
const langModules: Record<string, { libraryPath: string; extensions: string[]; languageSymbol?: string }> = {};
|
||||
const survivors: Record<string, AstGrepLangModule> = {};
|
||||
|
||||
const langPackages: Array<[string, string]> = [
|
||||
["python", "@ast-grep/lang-python"],
|
||||
@@ -471,21 +496,45 @@ export function ensureDynamicLanguages(): void {
|
||||
|
||||
for (const [name, pkg] of langPackages) {
|
||||
try {
|
||||
langModules[name] = esmRequire(pkg);
|
||||
} catch {
|
||||
// Language grammar not installed — skip silently
|
||||
logger.debug(`ast-grep language not available: ${name}`);
|
||||
const mod = esmRequire(pkg) as AstGrepLangModule;
|
||||
// Pre-validate the lazy `libraryPath` getter. `registerDynamicLanguage`
|
||||
// accesses this property for every entry it receives, and a single
|
||||
// throwing getter aborts the entire batch atomically (issue #43).
|
||||
// Touching the getter here, inside the per-grammar try/catch, isolates
|
||||
// a missing-prebuild failure to that one grammar so the rest can still
|
||||
// be registered. The getter caches its result inside the package, so
|
||||
// this is not duplicated work.
|
||||
void mod.libraryPath;
|
||||
survivors[name] = mod;
|
||||
loadedDynamicLanguages.add(name);
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
failedDynamicLanguages.set(name, message);
|
||||
logger.warn("ast-grep grammar failed to load", { name, error: message });
|
||||
}
|
||||
}
|
||||
|
||||
if (Object.keys(langModules).length > 0) {
|
||||
registerDynamicLanguage(langModules);
|
||||
if (Object.keys(survivors).length > 0) {
|
||||
registerDynamicLanguage(survivors);
|
||||
logger.info("Registered dynamic ast-grep languages", {
|
||||
languages: Object.keys(langModules),
|
||||
languages: [...loadedDynamicLanguages].sort(),
|
||||
});
|
||||
} else {
|
||||
logger.warn(
|
||||
"No dynamic ast-grep grammars loaded; PHP, Python, JVM and other dynamic languages will fall through to <module>-only extraction",
|
||||
);
|
||||
}
|
||||
if (failedDynamicLanguages.size > 0) {
|
||||
logger.warn(
|
||||
"Some dynamic ast-grep grammars failed to load; affected languages will produce only <module>-level symbols",
|
||||
{ failed: [...failedDynamicLanguages.keys()].sort() },
|
||||
);
|
||||
}
|
||||
} catch (err) {
|
||||
logger.warn("Failed to register dynamic ast-grep languages", { error: String(err) });
|
||||
// Should be unreachable now that each grammar is validated independently,
|
||||
// but keep the outer guard so an unexpected throw cannot take the indexer
|
||||
// process down.
|
||||
logger.warn("Unexpected error in ensureDynamicLanguages", { error: String(err) });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,22 @@ export interface ImportInfo {
|
||||
isCssImport?: boolean; // True when extracted from a CSS/style context
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-language dedupe set for import-extraction failures. Without this, a
|
||||
* missing PHP grammar would emit one warn per file (potentially hundreds).
|
||||
* We log the first failure per language at warn level (with the underlying
|
||||
* error attached) and silently skip subsequent failures.
|
||||
*/
|
||||
const importExtractionWarned = new Set<string>();
|
||||
|
||||
/**
|
||||
* Reset the per-language dedupe set. Intended for tests that want to assert
|
||||
* deterministically on extraction warnings.
|
||||
*/
|
||||
export function resetImportExtractionWarnings(): void {
|
||||
importExtractionWarned.clear();
|
||||
}
|
||||
|
||||
/** Extract CSS/SCSS/Stylus @import statements from raw style source text. */
|
||||
function extractCssImports(source: string): ImportInfo[] {
|
||||
const imports: ImportInfo[] = [];
|
||||
@@ -349,7 +365,17 @@ export function extractImports(source: string, lang: Lang | string, _ext: string
|
||||
break;
|
||||
}
|
||||
} catch (err) {
|
||||
logger.warn("Failed to parse file for imports", { lang: String(lang), error: String(err) });
|
||||
const langKey = String(lang);
|
||||
if (!importExtractionWarned.has(langKey)) {
|
||||
importExtractionWarned.add(langKey);
|
||||
logger.warn(
|
||||
"Failed to parse file for imports; subsequent failures will be suppressed for this language",
|
||||
{
|
||||
lang: langKey,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return imports;
|
||||
|
||||
@@ -52,6 +52,22 @@ interface ScopeFrame {
|
||||
symbolId: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-language dedupe set for symbol-extraction failures. Without this, a
|
||||
* missing PHP grammar would emit one warn per file (potentially hundreds).
|
||||
* We log the first failure per language at warn level (with the underlying
|
||||
* error attached) and silently skip subsequent failures.
|
||||
*/
|
||||
const symbolExtractionWarned = new Set<string>();
|
||||
|
||||
/**
|
||||
* Reset the per-language dedupe set. Intended for tests that want to assert
|
||||
* deterministically on extraction warnings.
|
||||
*/
|
||||
export function resetSymbolExtractionWarnings(): void {
|
||||
symbolExtractionWarned.clear();
|
||||
}
|
||||
|
||||
/** Find the deepest scope frame covering a line. */
|
||||
function findCallerId(scopes: ScopeFrame[], line: number, fallback: string): string {
|
||||
let best: ScopeFrame | null = null;
|
||||
@@ -129,11 +145,17 @@ export function extractSymbolsAndCalls(
|
||||
// Dart, Lua, Svelte, Vue and others fall through to the regex fallback.
|
||||
return extractFromRegex(source, relativePath, language, moduleSymbol);
|
||||
} catch (err) {
|
||||
logger.debug("extractSymbolsAndCalls failed", {
|
||||
file: relativePath,
|
||||
lang: langKey,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
if (!symbolExtractionWarned.has(langKey)) {
|
||||
symbolExtractionWarned.add(langKey);
|
||||
logger.warn(
|
||||
"Symbol extraction failed for language; subsequent failures will be suppressed for this language",
|
||||
{
|
||||
lang: langKey,
|
||||
file: relativePath,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
},
|
||||
);
|
||||
}
|
||||
return { symbols: [moduleSymbol], rawCalls: [] };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
import path from "node:path";
|
||||
import { projectIdFromPath } from "../config.js";
|
||||
import { mergeExtraExtensions } from "../constants.js";
|
||||
import { awaitGraphBuild, findCircularDependencies, generateMermaidDiagram, getFileDependencies, getGraphBuildProgress, getGraphStats, getGraphStatus, getLastGraphBuildCompleted, getOrBuildGraph, isGraphBuildInProgress, rebuildGraph, removeGraph } from "../services/code-graph.js";
|
||||
import { awaitGraphBuild, ensureDynamicLanguages, findCircularDependencies, generateMermaidDiagram, getDynamicLanguageStatus, getFileDependencies, getGraphBuildProgress, getGraphStats, getGraphStatus, getLastGraphBuildCompleted, getOrBuildGraph, isGraphBuildInProgress, rebuildGraph, removeGraph } from "../services/code-graph.js";
|
||||
import { detectEntryPoints } from "../services/graph-entrypoints.js";
|
||||
import {
|
||||
type FlowNode,
|
||||
@@ -234,6 +234,30 @@ export async function handleGraphTool(
|
||||
case "codebase_graph_status": {
|
||||
const resolved = path.resolve(projectPath);
|
||||
|
||||
// Trigger grammar registration so the diagnostic block below reflects
|
||||
// the real loader state. Idempotent and cheap after the first call.
|
||||
ensureDynamicLanguages();
|
||||
const grammarStatus = getDynamicLanguageStatus();
|
||||
const renderGrammarBlock = (): string[] => {
|
||||
if (grammarStatus.loaded.length === 0 && grammarStatus.failed.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const block: string[] = ["", "AST grammars:"];
|
||||
if (grammarStatus.loaded.length > 0) {
|
||||
block.push(` Loaded (${grammarStatus.loaded.length}): ${grammarStatus.loaded.join(", ")}`);
|
||||
}
|
||||
if (grammarStatus.failed.length > 0) {
|
||||
block.push(` Failed (${grammarStatus.failed.length}):`);
|
||||
for (const f of grammarStatus.failed) {
|
||||
block.push(` - ${f.name}: ${f.error}`);
|
||||
}
|
||||
block.push(
|
||||
" Symbols and imports for failed languages will be empty until the underlying load error is resolved.",
|
||||
);
|
||||
}
|
||||
return block;
|
||||
};
|
||||
|
||||
// Show in-flight build progress if building
|
||||
if (isGraphBuildInProgress(resolved)) {
|
||||
const progress = getGraphBuildProgress(resolved);
|
||||
@@ -243,17 +267,19 @@ export async function handleGraphTool(
|
||||
? Math.round((progress.filesProcessed / progress.filesTotal) * 100)
|
||||
: 0;
|
||||
|
||||
return [
|
||||
const buildingLines = [
|
||||
`Code Graph Status for: ${resolved}`,
|
||||
"",
|
||||
`Status: BUILDING`,
|
||||
`Phase: ${progress.phase}`,
|
||||
`Progress: ${progress.filesProcessed}/${progress.filesTotal} files (${pct}%)`,
|
||||
`Elapsed: ${elapsed}s`,
|
||||
...renderGrammarBlock(),
|
||||
"",
|
||||
"The graph is being built in the background.",
|
||||
"Call codebase_graph_status again to check progress.",
|
||||
].join("\n");
|
||||
];
|
||||
return buildingLines.join("\n");
|
||||
}
|
||||
|
||||
// Show last completed build info if available
|
||||
@@ -266,6 +292,7 @@ export async function handleGraphTool(
|
||||
lines.push(`Last build failed: ${lastBuild.error}`);
|
||||
}
|
||||
lines.push("Run codebase_graph_build or codebase_index to create one.");
|
||||
lines.push(...renderGrammarBlock());
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
@@ -294,6 +321,8 @@ export async function handleGraphTool(
|
||||
lines.push(` Unresolved: ${sm.unresolvedEdgePct.toFixed(1)}%`);
|
||||
}
|
||||
|
||||
lines.push(...renderGrammarBlock());
|
||||
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
||||
//
|
||||
// Tests for the dynamic ast-grep grammar loader.
|
||||
//
|
||||
// Background: in 1.8.3 the loader called `registerDynamicLanguage` once with
|
||||
// every loaded grammar in a single batch. Per `@ast-grep/napi`, that call is
|
||||
// atomic: a single throwing `libraryPath` getter aborts the whole batch and
|
||||
// no grammar gets registered. On environments where one prebuilt binary is
|
||||
// missing for the host architecture, this manifested as all dynamic
|
||||
// languages being silently broken (issue #43).
|
||||
//
|
||||
// 1.8.4 fixes this by pre-validating each grammar's `libraryPath` getter in
|
||||
// the inner per-grammar try/catch so a missing prebuild is contained to that
|
||||
// one grammar. The `getDynamicLanguageStatus` API exposes which grammars
|
||||
// registered successfully and which failed (with the underlying reason),
|
||||
// surfaced in `codebase_graph_status`.
|
||||
|
||||
import { beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
ensureDynamicLanguages,
|
||||
getDynamicLanguageStatus,
|
||||
} from "../../src/services/code-graph.js";
|
||||
|
||||
beforeAll(() => {
|
||||
ensureDynamicLanguages();
|
||||
});
|
||||
|
||||
describe("dynamic-language-loader", () => {
|
||||
describe("ensureDynamicLanguages", () => {
|
||||
it("is synchronous (returns void, not a Promise)", () => {
|
||||
const result = ensureDynamicLanguages();
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("is idempotent — calling repeatedly does not change registration state", () => {
|
||||
const before = getDynamicLanguageStatus();
|
||||
ensureDynamicLanguages();
|
||||
ensureDynamicLanguages();
|
||||
const after = getDynamicLanguageStatus();
|
||||
|
||||
expect(after.loaded).toEqual(before.loaded);
|
||||
expect(after.failed.map((f) => f.name)).toEqual(before.failed.map((f) => f.name));
|
||||
});
|
||||
});
|
||||
|
||||
describe("getDynamicLanguageStatus", () => {
|
||||
it("returns loaded and failed arrays", () => {
|
||||
const status = getDynamicLanguageStatus();
|
||||
expect(status).toHaveProperty("loaded");
|
||||
expect(status).toHaveProperty("failed");
|
||||
expect(Array.isArray(status.loaded)).toBe(true);
|
||||
expect(Array.isArray(status.failed)).toBe(true);
|
||||
});
|
||||
|
||||
it("loaded entries are unique strings", () => {
|
||||
const status = getDynamicLanguageStatus();
|
||||
const seen = new Set<string>();
|
||||
for (const name of status.loaded) {
|
||||
expect(typeof name).toBe("string");
|
||||
expect(seen.has(name)).toBe(false);
|
||||
seen.add(name);
|
||||
}
|
||||
});
|
||||
|
||||
it("failed entries each include name and error", () => {
|
||||
const status = getDynamicLanguageStatus();
|
||||
for (const f of status.failed) {
|
||||
expect(typeof f.name).toBe("string");
|
||||
expect(typeof f.error).toBe("string");
|
||||
expect(f.error.length).toBeGreaterThan(0);
|
||||
}
|
||||
});
|
||||
|
||||
it("loaded list is sorted alphabetically", () => {
|
||||
const status = getDynamicLanguageStatus();
|
||||
const sorted = [...status.loaded].sort();
|
||||
expect(status.loaded).toEqual(sorted);
|
||||
});
|
||||
|
||||
it("failed list is sorted alphabetically by name", () => {
|
||||
const status = getDynamicLanguageStatus();
|
||||
const names = status.failed.map((f) => f.name);
|
||||
const sorted = [...names].sort();
|
||||
expect(names).toEqual(sorted);
|
||||
});
|
||||
|
||||
it("loaded and failed sets are disjoint", () => {
|
||||
const status = getDynamicLanguageStatus();
|
||||
const failedNames = new Set(status.failed.map((f) => f.name));
|
||||
for (const loaded of status.loaded) {
|
||||
expect(failedNames.has(loaded)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("at least one expected dynamic grammar registers in this environment", () => {
|
||||
// On every supported dev environment for this repo (macOS, common
|
||||
// Linux distros), the pre-validation step should let at least a few
|
||||
// of these load successfully. If this fails, either the environment
|
||||
// is unusual (and we want to know) or there is a real regression in
|
||||
// the loader.
|
||||
const status = getDynamicLanguageStatus();
|
||||
const loaded = new Set(status.loaded);
|
||||
const expected = ["python", "go", "java", "php", "ruby"];
|
||||
const found = expected.filter((n) => loaded.has(n));
|
||||
expect(found.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user