fix(javascript): preserve path reasons, module coverage, and property provenance (#5651)
* fix(javascript): preserve selected path host resolution reasons Classify ELOOP and ENAMETOOLONG while resolving the caller-selected input so the path and host code stay on the invalid-request issue. Refs #5623 Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(javascript): count summary coverage in module units Use the module population for both examined and total counts. Non-module graph nodes remain in the application coverage section. Refs #5464 Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(javascript): connect direct property reads to return and argument flow Record the unwrapped value expression and alias the object projection's read occurrence so a direct member read stays in backward provenance. Refs #5585, #5648 Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(javascript): count return wrappers against expression depth Evaluate the original return expression so erased TypeScript and Flow wrappers stay inside the expression-depth budget. Keep the unwrapped node for identity and value ranges. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(javascript): describe ELOOP as unresolved symlink traversal A host can return ELOOP for a cycle or for a chain past its traversal limit. Keep the selected path and code without asserting that a cycle exists. Co-authored-by: morluto <morluto@users.noreply.github.com> * test(javascript): skip symlink traversal fixtures on Windows Keep the overlong-name case portable. Windows does not produce ELOOP for these fixtures, matching the other path-failure tests. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(javascript): describe ENAMETOOLONG as a path or component limit realpath reports that code for a long component and for a long full path. Keep the selected path and host code without claiming only a filename was too long. Co-authored-by: morluto <morluto@users.noreply.github.com> * style(javascript): format return-depth and object-read changes oxfmt rejected the return-depth test and the object-read index. No behavior change. Co-authored-by: morluto <morluto@users.noreply.github.com> * test(ida): expect unchanged invalid requests to be nonretryable The shared projector no longer marks invalid_request retryable. This lookup assertion was not updated with that contract. Co-authored-by: morluto <morluto@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: morluto <morluto@users.noreply.github.com>
This commit is contained in:
12 files changed
+315
-7
No files matched your search
@@ -343,8 +343,9 @@ export const reconstructJavaScriptArtifact = async (
|
||||
const OPERATION = "analyze_javascript_application";
|
||||
|
||||
/**
|
||||
* Resolve the caller-selected input. A missing or unreadable selection is a
|
||||
* caller or host-permission failure, not a damaged artifact.
|
||||
* Resolve the caller-selected input. A missing, looping, overlong, or
|
||||
* unreadable selection is a caller or host-resolution failure, not a damaged
|
||||
* artifact.
|
||||
*/
|
||||
const resolveSelectedInput = async (inputPath: string): Promise<string> => {
|
||||
try {
|
||||
@@ -366,10 +367,26 @@ const resolveSelectedInput = async (inputPath: string): Promise<string> => {
|
||||
message: `No file or directory exists at the selected input path (${code}): ${inputPath}`,
|
||||
},
|
||||
]);
|
||||
if (code === "ELOOP" || code === "ENAMETOOLONG")
|
||||
throw new AnalysisInputError(OPERATION, { cause }, [
|
||||
{
|
||||
path: ["input_path"],
|
||||
reason: "invalid_value",
|
||||
message: selectedInputResolutionMessage(code, inputPath),
|
||||
},
|
||||
]);
|
||||
throw cause;
|
||||
}
|
||||
};
|
||||
|
||||
const selectedInputResolutionMessage = (
|
||||
code: "ELOOP" | "ENAMETOOLONG",
|
||||
inputPath: string,
|
||||
): string =>
|
||||
code === "ENAMETOOLONG"
|
||||
? `Selected input path exceeds the host path or component length limit (ENAMETOOLONG): ${inputPath}`
|
||||
: `Selected input path could not be resolved because symbolic-link traversal looped or exceeded the host limit (ELOOP): ${inputPath}`;
|
||||
|
||||
/** The selected input's kind, checked against the caller's requested format. */
|
||||
const resolveFormat = async (
|
||||
path: string,
|
||||
|
||||
@@ -172,6 +172,56 @@ it("continues to project an ordinary member access as a property read", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps a directly returned or passed property in backward provenance", () => {
|
||||
const source = `
|
||||
const settings = { index: "products" };
|
||||
function viaLocal() {
|
||||
const value = settings.index;
|
||||
return value;
|
||||
}
|
||||
function viaMember() {
|
||||
return settings.index;
|
||||
}
|
||||
function viaParen() {
|
||||
return (settings.index);
|
||||
}
|
||||
const arrow = () => settings.index;
|
||||
function take(value) { return value; }
|
||||
function added() { return settings.index + "!"; }
|
||||
const a = viaLocal();
|
||||
const b = viaMember();
|
||||
const direct = settings.index;
|
||||
const c = viaParen();
|
||||
const d = arrow();
|
||||
const e = take(settings.index);
|
||||
const g = take((settings.index));
|
||||
const kept = settings.index;
|
||||
const local = take(kept);
|
||||
const concatenated = added();
|
||||
`;
|
||||
const graph = graphFor(source);
|
||||
const provenance = (binding: string, candidates: boolean) => {
|
||||
const node = graph.nodes.find(
|
||||
(item) => item.kind === "binding" && item.label === binding,
|
||||
);
|
||||
if (node === undefined) throw new Error(`missing ${binding}`);
|
||||
const trace = queryJavaScriptSemanticGraph(graph, {
|
||||
seed: { kind: "semantic-node", node_id: node.node_id },
|
||||
direction: "backward-provenance",
|
||||
include_ambiguous_dynamic_edges: candidates,
|
||||
});
|
||||
return trace.nodes.some(
|
||||
(item) => item.kind === "literal" && item.properties.value === "products",
|
||||
);
|
||||
};
|
||||
for (const binding of ["a", "b", "direct", "c", "d", "e", "g", "local"]) {
|
||||
expect(provenance(binding, false), binding).toBe(true);
|
||||
expect(provenance(binding, true), `${binding} candidates`).toBe(true);
|
||||
}
|
||||
expect(provenance("concatenated", false)).toBe(false);
|
||||
expect(provenance("concatenated", true)).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps dynamic calls explicit and returns complete deterministic results", () => {
|
||||
const graph = graphFor(`
|
||||
const handlers = { ready() { return 1; } };
|
||||
|
||||
@@ -60,6 +60,7 @@ import {
|
||||
projectSemanticFrontiers,
|
||||
projectSemanticPromises,
|
||||
projectSemanticReturnValues,
|
||||
semanticObjectReadOccurrence,
|
||||
type SemanticFlowProjectionContext,
|
||||
} from "./JavaScriptSemanticGraphFlowProjection.js";
|
||||
import {
|
||||
@@ -828,6 +829,15 @@ const projectCalls = (context: FileContext): void => {
|
||||
? "resolved"
|
||||
: "candidate",
|
||||
});
|
||||
addRelation(context.state, {
|
||||
source: semanticObjectReadOccurrence(
|
||||
context,
|
||||
argument.valueExpressionLocation,
|
||||
),
|
||||
target: argumentNode,
|
||||
relation: "aliases",
|
||||
resolution: "resolved",
|
||||
});
|
||||
}
|
||||
}
|
||||
for (const flow of context.ir.argumentFlows) {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { JavaScriptSemanticGraphNode } from "../../domain/javascript/javascriptSemanticGraphSchemas.js";
|
||||
import { javaScriptSemanticNodeId } from "../../domain/javascript/javascriptSemanticGraph.js";
|
||||
import type { JavaScriptSemanticIr } from "../../domain/javascript/javascriptSemanticIr.js";
|
||||
import { sourceRangesEqual } from "../../domain/javascript/javascriptStaticAnalysisHelpers.js";
|
||||
import type { JavaScriptSourceRange } from "../../domain/javascript/javascriptStaticAnalysisTypes.js";
|
||||
@@ -190,6 +191,57 @@ const addPromiseUnknown = (
|
||||
});
|
||||
};
|
||||
|
||||
interface ObjectReadIndex {
|
||||
readonly state: SemanticGraphProjectionState;
|
||||
readonly nodes: ReadonlyMap<string, JavaScriptSemanticGraphNode>;
|
||||
}
|
||||
|
||||
const objectReadIndexes = new WeakMap<JavaScriptSemanticIr, ObjectReadIndex>();
|
||||
|
||||
const objectReadRangeKey = (location: JavaScriptSourceRange): string =>
|
||||
`${String(location.start.line)}:${String(location.start.column)}:${String(location.end.line)}:${String(location.end.column)}`;
|
||||
|
||||
/**
|
||||
* The object projection owns a member read's occurrence. Index it once per
|
||||
* file so return and argument flow can connect that expression without
|
||||
* scanning every property read again.
|
||||
*/
|
||||
const objectReadOccurrences = (
|
||||
context: SemanticFlowProjectionContext,
|
||||
): ReadonlyMap<string, JavaScriptSemanticGraphNode> => {
|
||||
const cached = objectReadIndexes.get(context.ir);
|
||||
if (cached !== undefined && cached.state === context.state)
|
||||
return cached.nodes;
|
||||
const nodes = new Map<string, JavaScriptSemanticGraphNode>();
|
||||
for (const operation of context.ir.objectOperations) {
|
||||
if (operation.kind !== "read") continue;
|
||||
const node = context.state.nodes.get(
|
||||
javaScriptSemanticNodeId({
|
||||
kind: "expression",
|
||||
identity: {
|
||||
artifact_sha256: context.file.sha256,
|
||||
module_path: context.file.path,
|
||||
source_range: operation.location,
|
||||
role_key: operation.objectOperationId,
|
||||
},
|
||||
}),
|
||||
);
|
||||
if (node !== undefined)
|
||||
nodes.set(objectReadRangeKey(operation.location), node);
|
||||
}
|
||||
objectReadIndexes.set(context.ir, { state: context.state, nodes });
|
||||
return nodes;
|
||||
};
|
||||
|
||||
/** Return the retained read occurrence for one unwrapped member expression. */
|
||||
export const semanticObjectReadOccurrence = (
|
||||
context: SemanticFlowProjectionContext,
|
||||
location: JavaScriptSourceRange | null,
|
||||
): JavaScriptSemanticGraphNode | undefined =>
|
||||
location === null
|
||||
? undefined
|
||||
: objectReadOccurrences(context).get(objectReadRangeKey(location));
|
||||
|
||||
/** Link direct return expressions to their retained semantic references. */
|
||||
export const projectSemanticReturnValues = (
|
||||
context: SemanticFlowProjectionContext,
|
||||
@@ -212,6 +264,15 @@ export const projectSemanticReturnValues = (
|
||||
? "resolved"
|
||||
: "candidate",
|
||||
});
|
||||
addSemanticGraphRelation(context.state, {
|
||||
source: semanticObjectReadOccurrence(
|
||||
context,
|
||||
site.valueExpressionLocation,
|
||||
),
|
||||
target: returnNode,
|
||||
relation: "aliases",
|
||||
resolution: "resolved",
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -429,6 +429,54 @@ it("rejects ambiguous layout names with candidate indexes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("counts JavaScript summary coverage in module units", () => {
|
||||
const analysis = analysisViewJavaScriptAnalysisWithSource();
|
||||
const moduleNode = analysis.graph.nodes[0];
|
||||
const observation = moduleNode?.observations[0];
|
||||
if (moduleNode === undefined || observation === undefined)
|
||||
throw new Error("missing module observation");
|
||||
const packageNode = createJavaScriptApplicationNode({
|
||||
kind: "artifact",
|
||||
identity: moduleNode.identity,
|
||||
observations: moduleNode.observations.map((item) => ({
|
||||
label: item.label,
|
||||
properties: item.properties,
|
||||
evidence: item.evidence,
|
||||
...(item.source_map_reference === null
|
||||
? {}
|
||||
: { source_map_reference: item.source_map_reference }),
|
||||
})),
|
||||
});
|
||||
const graph = createJavaScriptApplicationGraph({
|
||||
schema: "JavaScriptApplicationGraph",
|
||||
root_node_ids: [moduleNode.node_id],
|
||||
nodes: [moduleNode, packageNode],
|
||||
edges: [],
|
||||
coverage: analysis.graph.coverage,
|
||||
limitations: analysis.graph.limitations,
|
||||
});
|
||||
const parent = javascriptParent(
|
||||
analysisViewBindJavaScriptGraphs(analysis, graph),
|
||||
);
|
||||
const summary = projectAnalysisView(parent, { kind: "summary" });
|
||||
if (!summary.ok) throw summary.error;
|
||||
expect(summary.value.coverage).toEqual({
|
||||
status: "complete-within-view",
|
||||
examined: 1,
|
||||
total: 1,
|
||||
next_offset: null,
|
||||
exhausted: true,
|
||||
});
|
||||
const page = projectAnalysisView(parent, {
|
||||
kind: "page",
|
||||
collection: "modules",
|
||||
offset: 0,
|
||||
limit: 8,
|
||||
});
|
||||
if (!page.ok) throw page.error;
|
||||
expect(page.value.coverage).toMatchObject({ examined: 1, total: 1 });
|
||||
});
|
||||
|
||||
it("projects JavaScript summary and preserves selected module observations", () => {
|
||||
const analysis = analysisViewJavaScriptAnalysisWithSource();
|
||||
const parent = javascriptParent(analysis);
|
||||
|
||||
@@ -350,6 +350,8 @@ export const projectJavaScriptApplicationView = (
|
||||
});
|
||||
}
|
||||
if (view.kind === "summary") {
|
||||
// Summary coverage counts modules. Graph nodes that are not modules stay
|
||||
// in summary.coverage.application rather than inflating this total.
|
||||
const moduleCount = analysis.graph.nodes.reduce(
|
||||
(count, node) => count + Number(isModuleNode(node)),
|
||||
0,
|
||||
@@ -384,7 +386,7 @@ export const projectJavaScriptApplicationView = (
|
||||
}),
|
||||
limitation_count: analysis.limitations.length,
|
||||
},
|
||||
coverage: completeCoverage(moduleCount, analysis.graph.nodes.length),
|
||||
coverage: completeCoverage(moduleCount, moduleCount),
|
||||
...shared,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -503,6 +503,37 @@ describe("semantic expression depth bounds", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("counts erased TypeScript wrappers on a return against expression depth", () => {
|
||||
const source = "function answer() { return true; }";
|
||||
const parsed = parseJavaScriptSource(source);
|
||||
const declaration = parsed?.program.body[0];
|
||||
const statement = t.isFunctionDeclaration(declaration)
|
||||
? declaration.body.body[0]
|
||||
: undefined;
|
||||
const argument = t.isReturnStatement(statement)
|
||||
? statement.argument
|
||||
: undefined;
|
||||
if (parsed === null || !t.isReturnStatement(statement) || argument == null)
|
||||
throw new Error("Expected a returned value");
|
||||
let expression = argument;
|
||||
for (let index = 0; index < 300; index += 1)
|
||||
expression = t.tsAsExpression(expression, t.tsBooleanKeyword());
|
||||
statement.argument = expression;
|
||||
|
||||
const ir = analyzeParsedJavaScriptSemantics(parsed);
|
||||
const site = ir.callables.find((item) => item.name === "answer")
|
||||
?.returnSites[0];
|
||||
expect(site?.value).toMatchObject({
|
||||
status: "unknown",
|
||||
resourceLimit: "expression-depth",
|
||||
});
|
||||
const literalStart = source.indexOf("true");
|
||||
expect(site?.valueExpressionLocation).toEqual({
|
||||
start: { line: 1, column: literalStart },
|
||||
end: { line: 1, column: literalStart + "true".length },
|
||||
});
|
||||
});
|
||||
|
||||
it("counts erased TypeScript wrappers against expression depth", () => {
|
||||
const parsed = parseJavaScriptSource("const answer = true;");
|
||||
const declaration = parsed?.program.body[0];
|
||||
|
||||
@@ -11,7 +11,10 @@ import type {
|
||||
} from "./javascriptSemanticIr.js";
|
||||
import { semanticReferenceRole } from "./javascriptSemanticProjection.js";
|
||||
import { createSemanticCallableStack } from "./javascriptSemanticCallableStack.js";
|
||||
import { semanticStaticPropertyKey } from "./javascriptAstValues.js";
|
||||
import {
|
||||
semanticStaticPropertyKey,
|
||||
unwrapJavaScriptExpression,
|
||||
} from "./javascriptAstValues.js";
|
||||
import {
|
||||
resolveSemanticBindingState,
|
||||
isUnshadowedGlobal,
|
||||
@@ -210,9 +213,14 @@ const retainedArguments = (
|
||||
for (const [index, node] of nodes.entries()) {
|
||||
// Babel recovery can leave null slots; retain the other argument positions.
|
||||
if (node === null) continue;
|
||||
const valueNode =
|
||||
t.isExpression(node) && !t.isSpreadElement(node)
|
||||
? unwrapJavaScriptExpression(node).node
|
||||
: null;
|
||||
retained.push({
|
||||
index,
|
||||
location: range(node),
|
||||
valueExpressionLocation: valueNode === null ? null : range(valueNode),
|
||||
spread: t.isSpreadElement(node),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -109,6 +109,11 @@ export interface JavaScriptSemanticReturnSite {
|
||||
readonly returnSiteId: string;
|
||||
readonly location: JavaScriptSourceRange;
|
||||
readonly identityReferenceLocation: JavaScriptSourceRange | null;
|
||||
/**
|
||||
* Range of the unwrapped returned expression. Null when the return has no
|
||||
* value. This is the value's syntax, which may be wider than an identifier.
|
||||
*/
|
||||
readonly valueExpressionLocation: JavaScriptSourceRange | null;
|
||||
readonly value: JavaScriptSemanticValue;
|
||||
}
|
||||
|
||||
@@ -128,6 +133,8 @@ export interface JavaScriptSemanticCallSite {
|
||||
export interface JavaScriptSemanticCallArgument {
|
||||
readonly index: number;
|
||||
readonly location: JavaScriptSourceRange;
|
||||
/** Unwrapped value expression; null for a spread or non-expression slot. */
|
||||
readonly valueExpressionLocation: JavaScriptSourceRange | null;
|
||||
readonly spread: boolean;
|
||||
}
|
||||
|
||||
|
||||
@@ -54,6 +54,10 @@ export const collectSemanticReturns = (
|
||||
const expressions = directReturnExpressions(node);
|
||||
const sites: JavaScriptSemanticReturnSite[] = [];
|
||||
for (const expression of expressions) {
|
||||
const returned =
|
||||
expression.node === null
|
||||
? null
|
||||
: unwrapJavaScriptExpression(expression.node).node;
|
||||
sites.push({
|
||||
returnSiteId: semanticReturnSiteId(
|
||||
callable.callableId,
|
||||
@@ -61,9 +65,10 @@ export const collectSemanticReturns = (
|
||||
),
|
||||
location: expression.location,
|
||||
identityReferenceLocation:
|
||||
expression.node !== null && t.isIdentifier(expression.node)
|
||||
? range(expression.node)
|
||||
returned !== null && t.isIdentifier(returned)
|
||||
? range(returned)
|
||||
: null,
|
||||
valueExpressionLocation: returned === null ? null : range(returned),
|
||||
value:
|
||||
expression.node === null
|
||||
? { status: "unknown", reason: "Return has no value." }
|
||||
|
||||
@@ -630,6 +630,7 @@ const callable = z.strictObject({
|
||||
returnSiteId: z.string(),
|
||||
location: range,
|
||||
identityReferenceLocation: range.nullable(),
|
||||
valueExpressionLocation: range.nullable(),
|
||||
value: semanticValue,
|
||||
}),
|
||||
),
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { chmod, mkdir, writeFile } from "node:fs/promises";
|
||||
import { chmod, mkdir, symlink, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { describe, expect, it, onTestFinished } from "vitest";
|
||||
@@ -23,6 +23,74 @@ const application = async () => {
|
||||
};
|
||||
|
||||
describe("JavaScript application input selection", () => {
|
||||
it("preserves the selected path and host reason for an overlong name", async () => {
|
||||
const { root } = await application();
|
||||
const longPath = join(root, "x".repeat(300));
|
||||
|
||||
expect(await analyzeFailure({ input_path: longPath })).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: ["input_path"],
|
||||
reason: "invalid_value",
|
||||
message: expect.stringContaining(
|
||||
`Selected input path exceeds the host path or component length limit (ENAMETOOLONG): ${longPath}`,
|
||||
),
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"preserves the selected path when symlink traversal loops or exceeds the host limit",
|
||||
async () => {
|
||||
const { root } = await application();
|
||||
const loopPath = join(root, "loop-a");
|
||||
await symlink("loop-b", loopPath);
|
||||
await symlink("loop-a", join(root, "loop-b"));
|
||||
await writeFile(join(root, "chain-end"), "");
|
||||
let target = "chain-end";
|
||||
for (let index = 59; index >= 0; index -= 1) {
|
||||
const name = `chain-${String(index)}`;
|
||||
await symlink(target, join(root, name));
|
||||
target = name;
|
||||
}
|
||||
const chainPath = join(root, "chain-0");
|
||||
|
||||
const loopError = await analyzeFailure({ input_path: loopPath });
|
||||
const chainError = await analyzeFailure({ input_path: chainPath });
|
||||
const eloopMessage = (path: string) =>
|
||||
`Selected input path could not be resolved because symbolic-link traversal looped or exceeded the host limit (ELOOP): ${path}`;
|
||||
expect(loopError).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: ["input_path"],
|
||||
reason: "invalid_value",
|
||||
message: expect.stringContaining(eloopMessage(loopPath)),
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(chainError).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: ["input_path"],
|
||||
reason: "invalid_value",
|
||||
message: expect.stringContaining(eloopMessage(chainPath)),
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(loopError).not.toEqual(chainError);
|
||||
},
|
||||
);
|
||||
|
||||
it("reports a missing input path as invalid input, not an artifact read failure", async () => {
|
||||
const { root } = await application();
|
||||
const missing = join(root, "missing");
|
||||
|
||||
Reference in new issue
Block a user