diff --git a/docs/design/rea-limit-inventory.md b/docs/design/rea-limit-inventory.md index 5dfbf83150..f85e378e24 100644 --- a/docs/design/rea-limit-inventory.md +++ b/docs/design/rea-limit-inventory.md @@ -156,7 +156,6 @@ or logger. | `REA_KUNA_CACHE_DIR` | `src/kuna/KunaConfiguration.ts` | Filesystem root. | | `REA_KUNA_COMMAND` | `src/kuna/KunaConfiguration.ts` | Caller-selected executable path. | | `REA_LOG_LEVEL` | `src/config/environment.ts` | Logger configuration for the process. | -| `REA_MANAGED_LINUX_HOPPER_LAUNCHER` | `src/application/HopperLauncherDiscovery.ts` | Caller-selected launcher path. | | `REA_MCP_INPUT_SCHEMA_PROFILE` | `src/config/environment.ts` | MCP server schema shape. | | `REA_MCP_OUTPUT_SCHEMA_PROFILE` | `src/config/environment.ts` | MCP server schema shape. | | `REA_MCP_TOOL_CATALOG_PROFILE` | `src/config/environment.ts` | MCP server catalog shape. | @@ -176,10 +175,12 @@ or logger. | `REA_UNBLOB_COMMAND` | `src/application/SetupRegistrationEnvironment.ts` | Caller-selected command. | | `REA_WABT_BIN_DIRECTORY` | `src/wasm/WabtArtifactProvider.ts` | Filesystem root of the WABT tools. | | `REA_WAKARU_COMMAND` | `src/application/SetupRegistrationEnvironment.ts` | Caller-selected command. | -| `REA_WORKFLOW_PROVIDER` | `src/application/DirectAnalysis.ts` | Provider choice. | | `REA_XPROP_COMMAND` | `src/application/desktop/DesktopCaptureFailure.ts` | Caller-selected command. | -Names that look like `REA_` but are process protocol, not caller settings: +`REA_MANAGED_LINUX_HOPPER_LAUNCHER` is a fixed path constant, and +`REA_WORKFLOW_PROVIDER` is a provider identity object. Neither is an +environment setting. Names that look like `REA_` but are process protocol, not +caller settings: `REA_BUILD_COMMIT`, `REA_BUILD_COMPUTER_REA_ARCHIVE`, `REA_INSTALL_SCRIPT`, `REA_OWNS_PROCESS_LIFETIME`, `REA_PAYLOAD`, `REA_PROCESS_RUN_ID`, `REA_RUN_ID`, `REA_SOCKET`, `REA_STATE_FORMAT`, `REA_TARGET_PATH`, `REA_TOKEN`, bridge markers diff --git a/scripts/verify/rea-numeric-limits.mjs b/scripts/verify/rea-numeric-limits.mjs index 0fc3632b25..e0137e05b5 100644 --- a/scripts/verify/rea-numeric-limits.mjs +++ b/scripts/verify/rea-numeric-limits.mjs @@ -30,14 +30,6 @@ const listFiles = async (dir) => { return output; }; -/** - * Count-shaped limits whose names do not use a byte, time, or maximum suffix. - * `REA_WEB_BUNDLE_ENDPOINT_LITERAL_CANDIDATES` is the runtime setting on #9836. - */ -const DOCUMENTED_COUNT_LIMITS = new Set([ - "REA_WEB_BUNDLE_ENDPOINT_LITERAL_CANDIDATES", -]); - /** Registry text is a declaration, not a producer that uses the limit. */ export const NUMERIC_LIMIT_DECLARATION = "src/domain/numericLimitRegistry.ts"; @@ -118,7 +110,7 @@ const boundLimitName = (node, bindings) => { }; export const isNumericLimit = (name) => - DOCUMENTED_COUNT_LIMITS.has(name) || + REQUIRED_RUNTIME_LIMITS.includes(name) || /(?:_BYTES|_MS|_MIB|_SECONDS|_POLLS|_JOBS|_ELEMENTS|_COUNT|_SESSIONS|_COMMANDS)$/u.test( name, ) || @@ -126,15 +118,54 @@ export const isNumericLimit = (name) => /_(?:TIMEOUT|DEADLINE|BUDGET)(?:_|$)/u.test(name) || /_HEAP(?:_|$)/u.test(name); -const limitName = (text) => - text !== undefined && /^REA_[A-Z0-9_]+$/u.test(text) && isNumericLimit(text) - ? text - : undefined; +const exactLimit = (text) => + text !== undefined && /^REA_[A-Z0-9_]+$/u.test(text) ? text : undefined; -const remember = (discovered, path, name) => { - const limit = limitName(name); - if (limit === undefined || discovered.has(limit)) return; - discovered.set(limit, path); +const limitName = (text) => { + const name = exactLimit(text); + return name !== undefined && isNumericLimit(name) ? name : undefined; +}; + +const numericSchemaText = (node) => + /z\.(?:number|int|coerce\.number)|(?:Bytes|Timeout|Deadline|Budget|Heap|Count|Integer|MiB)Schema\b/u.test( + node.getText(), + ); + +const numericParse = (node) => { + if (!ts.isCallExpression(node)) return false; + const expression = unwrapExpression(node.expression); + if ( + ts.isIdentifier(expression) && + (expression.text === "Number" || + expression.text === "parseInt" || + expression.text === "parseFloat") + ) + return true; + return ( + ts.isPropertyAccessExpression(expression) && + (expression.name.text === "safeParse" || expression.name.text === "parse") + ); +}; + +const readLifecycle = (node) => { + let current = node.parent; + while (current !== undefined) { + if (ts.isConstructorDeclaration(current)) return "construction"; + current = current.parent; + } + return "call"; +}; + +const remember = (discovered, path, name, node, force = false) => { + const limit = force ? exactLimit(name) : limitName(name); + if (limit === undefined) return; + const read = readLifecycle(node); + const current = discovered.get(limit); + if (current === undefined) { + discovered.set(limit, { path, read }); + return; + } + if (current.read === "construction" && read === "call") current.read = "call"; }; /** @@ -161,26 +192,35 @@ export const discoverNumericLimitUses = (files) => { ts.isPropertyAccessExpression(node) || ts.isPropertyAccessChain?.(node) ) - remember(discovered, relativePath, node.name.text); + remember(discovered, relativePath, node.name.text, node); if (ts.isElementAccessExpression(node)) remember( discovered, relativePath, boundLimitName(node.argumentExpression, bindings), + node, ); if (ts.isCallExpression(node) || ts.isNewExpression(node)) { + const force = numericParse(node); for (const argument of node.arguments ?? []) remember( discovered, relativePath, boundLimitName(argument, bindings), + node, + force, ); } if ( ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.InKeyword ) - remember(discovered, relativePath, boundLimitName(node.left, bindings)); + remember( + discovered, + relativePath, + boundLimitName(node.left, bindings), + node, + ); if ( ts.isBindingElement(node) && node.dotDotDotToken === undefined && @@ -192,7 +232,7 @@ export const discoverNumericLimitUses = (files) => { ts.isStringLiteral(key) || ts.isNumericLiteral(key) ) - remember(discovered, relativePath, key.text); + remember(discovered, relativePath, key.text, node); } if ( (ts.isPropertyAssignment(node) || @@ -201,18 +241,26 @@ export const discoverNumericLimitUses = (files) => { ts.isStringLiteral(node.name) || ts.isNumericLiteral(node.name)) ) - remember(discovered, relativePath, node.name.text); + remember( + discovered, + relativePath, + node.name.text, + node, + ts.isPropertyAssignment(node) && numericSchemaText(node.initializer), + ); if (ts.isVariableDeclaration(node) && node.initializer !== undefined) remember( discovered, relativePath, literalText(unwrapExpression(node.initializer)), + node, ); if (ts.isPropertyAssignment(node)) remember( discovered, relativePath, literalText(unwrapExpression(node.initializer)), + node, ); if ( ts.isTemplateExpression(node) || @@ -226,7 +274,7 @@ export const discoverNumericLimitUses = (files) => { ]; for (const chunk of chunks) for (const token of chunk.match(/REA_[A-Z0-9_]+/gu) ?? []) - remember(discovered, relativePath, token); + remember(discovered, relativePath, token, node); } ts.forEachChild(node, visit); }; @@ -258,7 +306,7 @@ if (direct) { const runtime = namesIn(definitions, /environment:\s*"(REA_[A-Z0-9_]+)"/gu); const entries = [ ...registry.matchAll( - /name:\s*"(REA_[A-Z0-9_]+)"\s*,\s*policy:\s*"(restart-only|per-call)"\s*,\s*read:\s*"(startup|per-call)"\s*,\s*reason:\s*"((?:\\.|[^"\\])*)"/gu, + /name:\s*"(REA_[A-Z0-9_]+)"\s*,\s*policy:\s*"(restart-only|per-call)"\s*,\s*read:\s*"(startup|per-call|construction)"\s*,\s*reason:\s*"((?:\\.|[^"\\])*)"/gu, ), ]; const listed = new Map(); @@ -272,9 +320,15 @@ if (direct) { failures.push( `${name} is both a runtime setting and on the exception list`, ); - if (policy === "per-call" && read !== "per-call") - failures.push(`${name} is per-call policy but is not read per call`); - if (policy === "restart-only" && read !== "startup") + if (policy === "restart-only" && read === "per-call") + failures.push( + `${name} is restart-only but is recorded as a per-call read`, + ); + if ( + policy === "restart-only" && + read !== "startup" && + read !== "construction" + ) failures.push(`${name} is restart-only but is not read at startup`); } @@ -292,12 +346,18 @@ if (direct) { )) { if (!runtime.has(name) && !listed.has(name)) failures.push( - `${name} in ${file} is neither an analysis setting nor docs/design/rea-limit-inventory.md via src/domain/numericLimitRegistry.ts`, + `${name} in ${file.path} is neither an analysis setting nor docs/design/rea-limit-inventory.md via src/domain/numericLimitRegistry.ts`, ); } for (const name of listed.keys()) { if (!discovered.has(name)) failures.push(`${name} is registered but no longer appears in src`); + const site = discovered.get(name); + const declared = listed.get(name); + if (site?.read === "construction" && declared?.read === "per-call") + failures.push( + `${name} is copied when its owner is constructed, so read cannot be per-call`, + ); } const present = new Set([...discovered.keys(), ...listed.keys(), ...runtime]); for (const name of REQUIRED_RUNTIME_LIMITS) { diff --git a/src/domain/numericLimitRegistry.test.ts b/src/domain/numericLimitRegistry.test.ts index 5ce47508bc..79abfd8a77 100644 --- a/src/domain/numericLimitRegistry.test.ts +++ b/src/domain/numericLimitRegistry.test.ts @@ -21,8 +21,10 @@ it("gives every numeric exception a reason and a matching inventory row", () => ); for (const entry of NUMERIC_LIMIT_REGISTRY) { expect(entry.reason.trim().length).toBeGreaterThanOrEqual(40); - expect(entry.read).toBe( - entry.policy === "per-call" ? "per-call" : "startup", + if (entry.policy === "restart-only") + expect(["startup", "construction"]).toContain(entry.read); + expect(entry.read).not.toBe( + entry.policy === "per-call" ? "startup" : "per-call", ); expect(runtimeEnvironments.has(entry.name)).toBe(false); expect(inventory).toContain(`\`${entry.name}\``); @@ -33,4 +35,12 @@ it("gives every numeric exception a reason and a matching inventory row", () => expect( NUMERIC_LIMIT_REGISTRY.filter((entry) => entry.policy === "per-call"), ).toHaveLength(11); + for (const definition of Object.values(ANALYSIS_SETTINGS)) { + if (definition.environment === undefined) continue; + expect(inventory, definition.environment).toContain( + `\`${definition.environment}\``, + ); + } + expect(inventory).not.toMatch(/^\| `REA_MANAGED_LINUX_HOPPER_LAUNCHER`/mu); + expect(inventory).not.toMatch(/^\| `REA_WORKFLOW_PROVIDER`/mu); }); diff --git a/src/domain/numericLimitRegistry.ts b/src/domain/numericLimitRegistry.ts index 0f137a587c..9730fcfeab 100644 --- a/src/domain/numericLimitRegistry.ts +++ b/src/domain/numericLimitRegistry.ts @@ -13,7 +13,7 @@ export interface NumericLimitRegistryEntry { readonly name: string; readonly policy: "restart-only" | "per-call"; - readonly read: "startup" | "per-call"; + readonly read: "startup" | "per-call" | "construction"; readonly reason: string; } @@ -28,14 +28,14 @@ export const NUMERIC_LIMIT_REGISTRY = [ { name: "REA_DEBUGGER_COMMAND_DEADLINE_MS", policy: "per-call", - read: "per-call", + read: "construction", reason: "GDB and Rizin apply this deadline to each debugger command. The session manager currently copies it at construction from the launch environment; the runtime migration reads the accepted operation snapshot instead.", }, { name: "REA_DEBUGGER_MAX_COMMAND_OUTPUT_BYTES", policy: "per-call", - read: "per-call", + read: "construction", reason: "Each debugger command retains output up to this ceiling. The session manager currently copies it at construction from the launch environment; the runtime migration reads the accepted operation snapshot instead.", }, @@ -84,7 +84,7 @@ export const NUMERIC_LIMIT_REGISTRY = [ { name: "REA_JEB_MCP_TIMEOUT_MS", policy: "per-call", - read: "per-call", + read: "construction", reason: "Bounds each JEB tools/call and catalog discovery. The connection currently captures the launch value when it is created; the runtime migration reads the accepted snapshot for each request.", }, diff --git a/tests/evaluation/numericLimitCheck.test.ts b/tests/evaluation/numericLimitCheck.test.ts index 358bcbbb24..d133d0f07a 100644 --- a/tests/evaluation/numericLimitCheck.test.ts +++ b/tests/evaluation/numericLimitCheck.test.ts @@ -16,7 +16,7 @@ it("recognizes the documented endpoint-candidate limit as numeric", () => { source: `const candidates = process.env.${ENDPOINT_CANDIDATES};`, }, ]); - expect(discovered.get(ENDPOINT_CANDIDATES)).toBe( + expect(discovered.get(ENDPOINT_CANDIDATES)?.path).toBe( "src/domain/webBundleAnalysis.ts", ); }); @@ -33,7 +33,7 @@ it("does not treat a registry declaration as a producer", () => { source: `const value = process.env.${name};`, }, ]); - expect(discovered.get(name)).toBe("src/config/removedLimit.ts"); + expect(discovered.get(name)?.path).toBe("src/config/removedLimit.ts"); expect( discoverNumericLimitUses([ { path: NUMERIC_LIMIT_DECLARATION, source: `name: "${name}",` }, @@ -53,7 +53,7 @@ it("keeps a helper-call argument that is not the first argument", () => { source: `const value = read(environment, "${name}");`, }, ]); - expect(discovered.get(name)).toBe("src/config/newLimit.ts"); + expect(discovered.get(name)?.path).toBe("src/config/newLimit.ts"); }); it("does not treat prose that mentions a limit name as a producer", () => { @@ -75,7 +75,33 @@ it("keeps a destructured environment key, including a rename and a default", () source: `const { ${name}: renamed = 1 } = process.env;\nconst present = "${name}" in environment;\nconst owned = Object.hasOwn(environment, "${name}");\nconst source = { ${name}: renamed };\nconst copy = { ...source };`, }, ]); - expect(discovered.get(name)).toBe("src/config/newLimit.ts"); + expect(discovered.get(name)?.path).toBe("src/config/newLimit.ts"); +}); + +it("records a constructor read separately from a per-call read", () => { + const name = "REA_NEW_MAX_BYTES"; + const discovered = discoverNumericLimitUses([ + { + path: "src/config/constructedLimit.ts", + source: `class Session {\n constructor(environment) {\n this.limit = environment.${name};\n }\n}`, + }, + ]); + expect(discovered.get(name)).toEqual({ + path: "src/config/constructedLimit.ts", + read: "construction", + }); +}); + +it("treats a count parsed from the environment as numeric without a suffix", () => { + const name = "REA_ARCHIVE_ENTRY_LIMIT"; + expect(isNumericLimit(name)).toBe(false); + const discovered = discoverNumericLimitUses([ + { + path: "src/config/archiveLimit.ts", + source: `const limit = Number(process.env.${name});\nconst schema = { ${name}: z.number().int() };`, + }, + ]); + expect(discovered.get(name)?.path).toBe("src/config/archiveLimit.ts"); }); it("does not invent a limit from a spread of the environment object", () => { @@ -96,7 +122,7 @@ it("keeps a bracket-access read as a producer", () => { source: `const selected = environment["${name}"] ?? environment?.[ "${name}" ];`, }, ]); - expect(discovered.get(name)).toBe("src/config/newLimit.ts"); + expect(discovered.get(name)?.path).toBe("src/config/newLimit.ts"); }); it("does not treat a registration allowlist or a comment as a producer", () => {