mirror of
https://github.com/morluto/rea.git
synced 2026-11-12 09:52:27 +00:00
fix(settings): classify caller-selected limits as parameter updates
analysis_timeout_ms, max_heap_mb, and command_timeout_ms keep the source chosen by the caller, the runtime override, or the built-in default. A request value reports update_mode parameter because update_analysis_settings cannot change that call.
This commit is contained in:
1 parent
58a1216a57
commit
0ae186e196
14 files changed
+128
-24
No files matched your search
@@ -1,6 +1,7 @@
|
||||
import { discoverJavaScriptPreloadPaths } from "./JavaScriptPreloadDiscovery.js";
|
||||
import { artifactNameForClassification } from "../../domain/artifactNameSelection.js";
|
||||
import type { AnalysisSettingValues } from "../../domain/analysisSettings.js";
|
||||
import { selectedAnalysisSetting } from "../../domain/analysisSettingDefinitions.js";
|
||||
import { absoluteCallerPath } from "../../filesystem/callerPath.js";
|
||||
import {
|
||||
DEFAULT_ASAR_HEADER_ADMISSION,
|
||||
@@ -181,18 +182,24 @@ export const reconstructJavaScriptArtifact = async (
|
||||
? {}
|
||||
: { fileNodes: input.max_semantic_nodes_per_file }),
|
||||
});
|
||||
const heap = selectedAnalysisSetting(
|
||||
input.max_heap_mb,
|
||||
overrides?.["javascript.max_heap_mb"],
|
||||
JAVASCRIPT_ANALYSIS_RESOURCE_DEFAULTS.heapMb,
|
||||
);
|
||||
const timeout = selectedAnalysisSetting(
|
||||
input.analysis_timeout_ms,
|
||||
overrides?.["javascript.analysis_timeout_ms"],
|
||||
JAVASCRIPT_ANALYSIS_RESOURCE_DEFAULTS.timeoutMs,
|
||||
);
|
||||
const sourceLimits = {
|
||||
...(input.semantic_precision === undefined
|
||||
? {}
|
||||
: { semanticPrecision: input.semantic_precision }),
|
||||
heapMb:
|
||||
input.max_heap_mb ??
|
||||
overrides?.["javascript.max_heap_mb"] ??
|
||||
JAVASCRIPT_ANALYSIS_RESOURCE_DEFAULTS.heapMb,
|
||||
timeoutMs:
|
||||
input.analysis_timeout_ms ??
|
||||
overrides?.["javascript.analysis_timeout_ms"] ??
|
||||
JAVASCRIPT_ANALYSIS_RESOURCE_DEFAULTS.timeoutMs,
|
||||
heapMb: heap.value,
|
||||
heapSource: heap.source,
|
||||
timeoutMs: timeout.value,
|
||||
timeoutSource: timeout.source,
|
||||
};
|
||||
const preloads = await discoverJavaScriptPreloadPaths(
|
||||
files,
|
||||
|
||||
@@ -15,6 +15,8 @@ export type WasmArtifactExecution = Omit<AnalysisExecution, "subject"> & {
|
||||
/** Effective operation budget, separate from caller-authored Evidence parameters. */
|
||||
export interface WasmArtifactExecutionOptions extends ExecutionOptions {
|
||||
readonly commandTimeoutMs?: number;
|
||||
/** Selection owner for `commandTimeoutMs`. Absent when the caller did not record it. */
|
||||
readonly commandTimeoutSource?: "default" | "runtime" | "request" | undefined;
|
||||
}
|
||||
|
||||
/** Replaceable offline WASM inspection engine, independent of target execution and network clients. */
|
||||
|
||||
@@ -240,6 +240,7 @@ it("gives cancellation precedence over malformed raw input", async () => {
|
||||
it("uses runtime defaults only for omitted budgets, preserving explicit input and reset", async () => {
|
||||
const settings = new AnalysisSettings({});
|
||||
const accepted: number[] = [];
|
||||
const sources: (string | undefined)[] = [];
|
||||
const authored: (number | undefined)[] = [];
|
||||
const service = new WasmArtifactService(
|
||||
{
|
||||
@@ -248,6 +249,7 @@ it("uses runtime defaults only for omitted budgets, preserving explicit input an
|
||||
const value = wasmArtifactFixture();
|
||||
value.tool_profile.timeout_ms = options!.commandTimeoutMs!;
|
||||
accepted.push(options!.commandTimeoutMs!);
|
||||
sources.push(options!.commandTimeoutSource);
|
||||
authored.push(input.command_timeout_ms);
|
||||
return ok(wasmArtifactExecution(value));
|
||||
},
|
||||
@@ -275,6 +277,7 @@ it("uses runtime defaults only for omitted budgets, preserving explicit input an
|
||||
});
|
||||
expect((await settings.run(() => service.inspect({ path }))).ok).toBe(true);
|
||||
expect(accepted).toEqual([123000, 42000, 30000]);
|
||||
expect(sources).toEqual(["runtime", "request", "default"]);
|
||||
expect(authored).toEqual([undefined, 42000, undefined]);
|
||||
});
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { AnalysisSettingsReader } from "../../domain/analysisSettings.js";
|
||||
import { selectedAnalysisSetting } from "../../domain/analysisSettingDefinitions.js";
|
||||
import { constants as bufferConstants } from "node:buffer";
|
||||
import type { ExecutionOptions } from "../AnalysisProvider.js";
|
||||
import {
|
||||
@@ -78,13 +79,15 @@ export class WasmArtifactService {
|
||||
),
|
||||
);
|
||||
}
|
||||
const timeout =
|
||||
input.value.command_timeout_ms ??
|
||||
this.settings?.().overrides["wasm.command_timeout_ms"] ??
|
||||
DEFAULT_WASM_COMMAND_TIMEOUT_MS;
|
||||
const timeout = selectedAnalysisSetting(
|
||||
input.value.command_timeout_ms,
|
||||
this.settings?.().overrides["wasm.command_timeout_ms"],
|
||||
DEFAULT_WASM_COMMAND_TIMEOUT_MS,
|
||||
);
|
||||
const inspected = await this.provider.inspect(input.value, {
|
||||
...options,
|
||||
commandTimeoutMs: timeout,
|
||||
commandTimeoutMs: timeout.value,
|
||||
commandTimeoutSource: timeout.source,
|
||||
});
|
||||
if (!inspected.ok) return inspected;
|
||||
const execution = inspected.value;
|
||||
@@ -108,7 +111,7 @@ export class WasmArtifactService {
|
||||
!report.success ||
|
||||
report.data.artifact.path !== input.value.path ||
|
||||
execution.subject.path !== input.value.path ||
|
||||
report.data.tool_profile.timeout_ms !== timeout ||
|
||||
report.data.tool_profile.timeout_ms !== timeout.value ||
|
||||
report.data.tool_profile.output_budget_bytes !==
|
||||
wasmResourceLimits(input.value).outputBytes ||
|
||||
!wasmArtifactMatchesSubject(report.data.artifact, execution.subject) ||
|
||||
|
||||
@@ -1530,11 +1530,25 @@ describe("analysis error projection: caller lifetime", () => {
|
||||
expect.objectContaining({
|
||||
setting: "wasm.command_timeout_ms",
|
||||
source: "request",
|
||||
update_mode: "parameter",
|
||||
parameter: "command_timeout_ms",
|
||||
applies_at: "next_invocation",
|
||||
scope: "rea-invocation",
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
analysisSettingLimit("wasm.command_timeout_ms", 100, "runtime"),
|
||||
).toMatchObject({ source: "runtime", update_mode: "runtime" });
|
||||
expect(
|
||||
analysisSettingLimit("javascript.max_heap_mb", 128, "request"),
|
||||
).toMatchObject({
|
||||
source: "request",
|
||||
update_mode: "parameter",
|
||||
parameter: "max_heap_mb",
|
||||
});
|
||||
expect(
|
||||
analysisSettingLimit("javascript.analysis_timeout_ms", 1, "default"),
|
||||
).toMatchObject({ source: "default", update_mode: "runtime" });
|
||||
expect(
|
||||
projectAnalysisError(error, "invocation").details?.limit,
|
||||
).not.toHaveProperty("environment");
|
||||
|
||||
@@ -333,6 +333,23 @@ export const analysisSettingValueSchema = (id: AnalysisSettingId) => {
|
||||
? number.nullable()
|
||||
: number;
|
||||
};
|
||||
/**
|
||||
* Caller argument, then the accepted runtime override, then the built-in
|
||||
* default. The third value is that default, not a launch-environment seed.
|
||||
*/
|
||||
export const selectedAnalysisSetting = (
|
||||
request: number | undefined,
|
||||
override: number | undefined,
|
||||
fallback: number,
|
||||
): {
|
||||
readonly value: number;
|
||||
readonly source: "default" | "runtime" | "request";
|
||||
} => {
|
||||
if (request !== undefined) return { value: request, source: "request" };
|
||||
if (override !== undefined) return { value: override, source: "runtime" };
|
||||
return { value: fallback, source: "default" };
|
||||
};
|
||||
|
||||
/** Project one owner's accepted budget, including the explicit per-call precedence. */
|
||||
export const analysisSettingLimit = (
|
||||
setting: AnalysisSettingId,
|
||||
@@ -350,7 +367,7 @@ export const analysisSettingLimit = (
|
||||
applies_at: definition.appliesAt,
|
||||
agent_editable: true,
|
||||
scope: "rea-runtime",
|
||||
update_mode: definition.updateMode,
|
||||
update_mode: source === "request" ? "parameter" : definition.updateMode,
|
||||
pending_restart: false,
|
||||
...(definition.parameter === undefined
|
||||
? {}
|
||||
|
||||
@@ -29,8 +29,8 @@ export type AnalysisSettingsReader = () => AnalysisSettingsSnapshot;
|
||||
/**
|
||||
* Whether `update_analysis_settings` can change the active value.
|
||||
* `runtime` applies inside this process. `restart` stays at the value this
|
||||
* process already started with. Caller-selected parameters use `parameter`
|
||||
* on their own facts; they are not analysis settings.
|
||||
* process already started with. A limit fact uses `parameter` when this call
|
||||
* supplied the value; the setting definition itself stays `runtime`.
|
||||
*/
|
||||
export type AnalysisSettingUpdateMode = "runtime" | "restart";
|
||||
|
||||
@@ -45,7 +45,7 @@ export interface AnalysisSettingLimit {
|
||||
readonly applies_at: "next_operation" | "next_worker" | "next_startup";
|
||||
readonly agent_editable: true;
|
||||
readonly scope: "rea-runtime";
|
||||
readonly update_mode: AnalysisSettingUpdateMode;
|
||||
readonly update_mode: AnalysisSettingUpdateMode | "parameter";
|
||||
/**
|
||||
* True only when a configured value is not the one this process is using.
|
||||
* The reported effective value is active, so limit errors leave this false.
|
||||
|
||||
@@ -41,7 +41,21 @@ export interface JavaScriptSourceAnalysisFailure {
|
||||
export interface JavaScriptSourceAnalysisLimits {
|
||||
readonly semanticPrecision?: JavaScriptSemanticPrecisionInput;
|
||||
readonly heapMb: number;
|
||||
/** Where `heapMb` was selected. Absent when the caller did not record it. */
|
||||
readonly heapSource?:
|
||||
| "default"
|
||||
| "setting"
|
||||
| "runtime"
|
||||
| "request"
|
||||
| undefined;
|
||||
readonly timeoutMs: number;
|
||||
/** Where `timeoutMs` was selected. Absent when the caller did not record it. */
|
||||
readonly timeoutSource?:
|
||||
| "default"
|
||||
| "setting"
|
||||
| "runtime"
|
||||
| "request"
|
||||
| undefined;
|
||||
}
|
||||
|
||||
/** Sequential source-analysis authority with independently verified cleanup. */
|
||||
|
||||
@@ -951,7 +951,7 @@ export class JavaScriptAnalysisWorker {
|
||||
limit: analysisSettingLimit(
|
||||
"javascript.max_heap_mb",
|
||||
this.limits.heapMb,
|
||||
undefined,
|
||||
this.limits.heapSource,
|
||||
),
|
||||
remediationAction: javaScriptWorkerHeapRemediation(this.limits.heapMb),
|
||||
},
|
||||
@@ -1150,7 +1150,7 @@ export class JavaScriptAnalysisWorker {
|
||||
limit: analysisSettingLimit(
|
||||
"javascript.analysis_timeout_ms",
|
||||
this.limits.timeoutMs,
|
||||
undefined,
|
||||
this.limits.timeoutSource,
|
||||
),
|
||||
capturedOutput: this.#captured(),
|
||||
userMessage: `JavaScript source analysis reached its ${String(this.limits.timeoutMs)}ms per-source server/provider deadline (analysis_timeout_ms), not the MCP client deadline.`,
|
||||
@@ -1185,7 +1185,7 @@ export class JavaScriptAnalysisWorker {
|
||||
limit: analysisSettingLimit(
|
||||
"javascript.max_heap_mb",
|
||||
this.limits.heapMb,
|
||||
undefined,
|
||||
this.limits.heapSource,
|
||||
),
|
||||
}
|
||||
: {}),
|
||||
|
||||
@@ -1223,7 +1223,15 @@ it("enforces a selected command deadline and completes a corrected retry after c
|
||||
if (timed.ok) throw new Error("Selected 20 ms deadline was not enforced");
|
||||
expect(projectAnalysisError(timed.error)).toMatchObject({
|
||||
code: "provider_timeout",
|
||||
details: { timeout_ms: 20 },
|
||||
details: {
|
||||
timeout_ms: 20,
|
||||
limit: {
|
||||
setting: "wasm.command_timeout_ms",
|
||||
effective_value: 20,
|
||||
source: "request",
|
||||
update_mode: "parameter",
|
||||
},
|
||||
},
|
||||
remediation: { action: expect.stringContaining("command_timeout_ms") },
|
||||
});
|
||||
expect(timed.error.cleanupIncomplete).toBe(false);
|
||||
|
||||
@@ -230,6 +230,10 @@ export class WabtArtifactProvider implements WasmArtifactPort {
|
||||
input.command_timeout_ms ??
|
||||
options?.commandTimeoutMs ??
|
||||
DEFAULT_WASM_COMMAND_TIMEOUT_MS;
|
||||
const commandTimeoutSource =
|
||||
input.command_timeout_ms !== undefined
|
||||
? "request"
|
||||
: options?.commandTimeoutSource;
|
||||
try {
|
||||
if (options?.signal?.aborted)
|
||||
throw new AnalysisCancelledError(OPERATION, {
|
||||
@@ -713,6 +717,9 @@ export class WabtArtifactProvider implements WasmArtifactPort {
|
||||
phase === "artifact" ? artifactInput.path : input.path,
|
||||
{
|
||||
commandTimeoutMs,
|
||||
...(commandTimeoutSource === undefined
|
||||
? {}
|
||||
: { commandTimeoutSource }),
|
||||
limits,
|
||||
inputField: artifactInput.field,
|
||||
observedOutput: capturedOutput,
|
||||
|
||||
@@ -122,6 +122,24 @@ it.each([1234, 2_147_483_647])(
|
||||
},
|
||||
recovery: { kind: "inspect_timeout_state", timeout_ms: commandTimeoutMs },
|
||||
});
|
||||
expect(
|
||||
projectAnalysisError(
|
||||
wabtFailure(
|
||||
new OwnedCommandFailure("timeout", "deadline"),
|
||||
"producer",
|
||||
"/module.wasm",
|
||||
{ commandTimeoutMs, commandTimeoutSource: "request" },
|
||||
),
|
||||
),
|
||||
).toMatchObject({
|
||||
details: {
|
||||
limit: {
|
||||
source: "request",
|
||||
update_mode: "parameter",
|
||||
effective_value: commandTimeoutMs,
|
||||
},
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -60,6 +60,12 @@ export type WabtVersionProbe = {
|
||||
|
||||
type WabtFailureContext = {
|
||||
readonly commandTimeoutMs: number;
|
||||
readonly commandTimeoutSource?:
|
||||
| "default"
|
||||
| "setting"
|
||||
| "runtime"
|
||||
| "request"
|
||||
| undefined;
|
||||
readonly limits?: WasmResourceLimits;
|
||||
readonly versionProbe?: WabtVersionProbe;
|
||||
readonly inputField?: readonly (string | number)[];
|
||||
@@ -179,13 +185,17 @@ const classifyWabtFailure = (
|
||||
// Classify once before cleanup takes precedence, so the primary remains actionable.
|
||||
const timeoutMs =
|
||||
execution?.commandTimeoutMs ?? DEFAULT_WASM_COMMAND_TIMEOUT_MS;
|
||||
const timeoutSource =
|
||||
execution?.commandTimeoutMs === undefined
|
||||
? "default"
|
||||
: execution.commandTimeoutSource;
|
||||
const timeout =
|
||||
cause.reason === "timeout"
|
||||
? new AnalysisTimeoutError("inspect_wasm_artifact", timeoutMs, {
|
||||
limit: analysisSettingLimit(
|
||||
"wasm.command_timeout_ms",
|
||||
timeoutMs,
|
||||
undefined,
|
||||
timeoutSource,
|
||||
),
|
||||
...options,
|
||||
// The cleanup wrapper retains output once; its nested primary needs only recovery facts.
|
||||
|
||||
Reference in new issue
Block a user