From 484f15bba90a9461618608a4fa72eac9710c6c97 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Tue, 1 Sep 2026 18:10:27 +0000 Subject: [PATCH] fix(config): review follow-ups from #135/#143 Codex review follow-ups on the static route-config extractor and script route discovery: - reject non-finite numeric literals (`1e999`, `-1e999`) with the AB4806 dynamic-config diagnostic instead of serializing Infinity as null - carry extracted object literals on a null prototype so a literal `__proto__` key stays an own property instead of invoking the legacy prototype setter - discover `.jsx` under src/scripts/ so rendered .jsx scripts reach the AB4807 gate instead of vanishing, and parse .jsx modules as JSX during config extraction --- .../agent-bundle/src/routes/config-extract.ts | 34 ++++++++++++++----- packages/agent-bundle/src/routes/graph.ts | 4 ++- .../agent-bundle/tests/normalization.test.ts | 8 +++++ .../tests/route-config-extract.test.ts | 27 +++++++++++++++ .../agent-bundle/tests/route-graph.test.ts | 19 +++++++++++ 5 files changed, 83 insertions(+), 9 deletions(-) diff --git a/packages/agent-bundle/src/routes/config-extract.ts b/packages/agent-bundle/src/routes/config-extract.ts index dc514c34b..ea5aeea54 100644 --- a/packages/agent-bundle/src/routes/config-extract.ts +++ b/packages/agent-bundle/src/routes/config-extract.ts @@ -30,14 +30,15 @@ export interface ExtractedRouteConfig { * methods, or accessors); * - array literals without spreads or holes; * - string literals and substitution-free template literals; - * - numeric literals, optionally wrapped in unary `+`/`-`; + * - finite numeric literals, optionally wrapped in unary `+`/`-`; * - `true`, `false`, and `null`; * - `as`/`satisfies` casts, non-null assertions, and parentheses around any * accepted form (they unwrap to their inner expression). * * Everything else — identifier references, calls, functions, templates with - * substitutions, `undefined`, bigints, regular expressions — is dynamic and - * raises AB4806 naming the offending construct. + * substitutions, `undefined`, bigints, regular expressions, non-finite + * numbers such as `1e999` — is dynamic and raises AB4806 naming the + * offending construct. */ export const routeConfigGrammar = 'object/array/string/number/boolean/null literals, with as-const, satisfies, non-null, and parenthesis wrappers'; @@ -111,6 +112,17 @@ const literalPropertyName = (name: ts.PropertyName): string | undefined => { return undefined; }; +/** + * Numeric literals must extract to finite numbers: an overflowing literal + * such as `1e999` evaluates to `Infinity`, which `JSON.stringify` collapses + * to `null` — the digest and inspection output could no longer distinguish + * the config from one that declared `null`. + */ +const finiteNumber = (value: number, node: ts.Node): Extraction => + Number.isFinite(value) + ? { kind: 'value', value } + : dynamic(`the non-finite number \`${String(value)}\``, node); + const extractExpression = (expression: ts.Expression): Extraction => { const node = unwrapExpression(expression); switch (node.kind) { @@ -126,7 +138,7 @@ const extractExpression = (expression: ts.Expression): Extraction => { if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) { return { kind: 'value', value: node.text }; } - if (ts.isNumericLiteral(node)) return { kind: 'value', value: Number(node.text) }; + if (ts.isNumericLiteral(node)) return finiteNumber(Number(node.text), node); if (ts.isPrefixUnaryExpression(node)) { const operand = unwrapExpression(node.operand); if ( @@ -134,7 +146,7 @@ const extractExpression = (expression: ts.Expression): Extraction => { (node.operator === ts.SyntaxKind.MinusToken || node.operator === ts.SyntaxKind.PlusToken) ) { const magnitude = Number(operand.text); - return { kind: 'value', value: node.operator === ts.SyntaxKind.MinusToken ? -magnitude : magnitude }; + return finiteNumber(node.operator === ts.SyntaxKind.MinusToken ? -magnitude : magnitude, node); } return dynamic(describeExpression(node), node); } @@ -151,7 +163,10 @@ const extractExpression = (expression: ts.Expression): Extraction => { return { kind: 'value', value: values }; } if (ts.isObjectLiteralExpression(node)) { - const value: Record = {}; + // A null-prototype carrier keeps a literal `__proto__` key an ordinary + // own property; assigning through a plain `{}` would invoke the legacy + // prototype setter and silently drop the declared property. + const value: Record = Object.create(null) as Record; for (const property of node.properties) { if (!ts.isPropertyAssignment(property)) return dynamic(describeExpression(property), property); const name = literalPropertyName(property.name); @@ -214,8 +229,11 @@ const findConfigExport = (sourceFile: ts.SourceFile): ConfigExportSite | undefin return undefined; }; -const scriptKindOf = (relativePath: string): ts.ScriptKind => - relativePath.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS; +const scriptKindOf = (relativePath: string): ts.ScriptKind => { + if (relativePath.endsWith('.tsx')) return ts.ScriptKind.TSX; + if (relativePath.endsWith('.jsx')) return ts.ScriptKind.JSX; + return ts.ScriptKind.TS; +}; const positionOf = (sourceFile: ts.SourceFile, node: ts.Node): string => { const { character, line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)); diff --git a/packages/agent-bundle/src/routes/graph.ts b/packages/agent-bundle/src/routes/graph.ts index 67126061e..db5c85d59 100644 --- a/packages/agent-bundle/src/routes/graph.ts +++ b/packages/agent-bundle/src/routes/graph.ts @@ -37,7 +37,9 @@ const routeGlobs = [ 'src/events/*/*.{ts,tsx}', 'src/providers/*.{ts,tsx}', 'src/cli/**/*.{ts,tsx}', - 'src/scripts/**/*.{ts,tsx}', + // Scripts also discover .jsx: the stage-1 script gate judges rendered + // modules (AB4807), so a .jsx script must surface there, never vanish. + 'src/scripts/**/*.{ts,tsx,jsx}', ]; const mcpRouteKinds: Readonly> = { diff --git a/packages/agent-bundle/tests/normalization.test.ts b/packages/agent-bundle/tests/normalization.test.ts index 320f3426a..ba61344d7 100644 --- a/packages/agent-bundle/tests/normalization.test.ts +++ b/packages/agent-bundle/tests/normalization.test.ts @@ -892,6 +892,7 @@ it('gates rendered, nested, and conflicting conventional script routes as AB4807 'src/scripts/detect-risk.ts', 'src/scripts/release/tag.ts', 'src/scripts/render-notes.tsx', + 'src/scripts/render-poster.jsx', 'src/scripts/verify-release.ts', ]), skills: [], @@ -922,6 +923,13 @@ it('gates rendered, nested, and conflicting conventional script routes as AB4807 severity: 'error', sourcePath: `${root}/src/scripts/render-notes.tsx`, }, + { + code: 'AB4807', + message: 'Conventional script src/scripts/render-poster.jsx is a rendered-script module; rendered scripts are not supported yet.', + recovery: 'Rename the module to .ts to ship a plain script, prefix a path segment with "_" to keep it private, or declare it under scripts in config to opt into plain bundling.', + severity: 'error', + sourcePath: `${root}/src/scripts/render-poster.jsx`, + }, ]); }); diff --git a/packages/agent-bundle/tests/route-config-extract.test.ts b/packages/agent-bundle/tests/route-config-extract.test.ts index 311ba848d..b98381b10 100644 --- a/packages/agent-bundle/tests/route-config-extract.test.ts +++ b/packages/agent-bundle/tests/route-config-extract.test.ts @@ -46,6 +46,31 @@ it('parses TSX modules whose bodies contain JSX', () => { expect(config).toEqual({ title: 'App' }); }); +it('parses JSX modules whose bodies contain JSX', () => { + const { config, diagnostics } = extract([ + "export const config = { title: 'Poster' };", + 'export default function Poster() { return
poster
; }', + ].join('\n'), 'src/scripts/render-poster.jsx'); + expect(diagnostics).toEqual([]); + expect(config).toEqual({ title: 'Poster' }); +}); + +it('preserves a literal "__proto__" key as an own config property', () => { + const { config, diagnostics } = extract( + 'export const config = { "__proto__": { injected: true }, title: \'safe\' };', + ); + expect(diagnostics).toEqual([]); + // The key is an ordinary own data property: enumerated, serialized, and + // frozen like any other — never a prototype swap that inspection and the + // digest would silently drop. + expect(Object.keys(config)).toEqual(['__proto__', 'title']); + const descriptor = Object.getOwnPropertyDescriptor(config, '__proto__'); + expect(descriptor?.value).toEqual({ injected: true }); + expect(JSON.parse(JSON.stringify(config))).toMatchObject({ title: 'safe' }); + expect(JSON.stringify(config)).toContain('"__proto__":{"injected":true}'); + expect(Object.isFrozen(descriptor?.value)).toBe(true); +}); + it('extracts silently to the empty config when no config export exists', () => { const { config, diagnostics } = extract('export default () => null;\nconst config = { hidden: true };'); expect(diagnostics).toEqual([]); @@ -62,6 +87,8 @@ it.each([ ['method', 'export const config = { run() { return 1; } };', 'AB4806', 'a method or accessor'], ['array spread', 'export const config = { tags: [...list] };', 'AB4806', 'a spread'], ['undefined value', 'export const config = { title: undefined };', 'AB4806', 'the non-JSON value `undefined`'], + ['overflowing numeric literal', 'export const config = { limit: 1e999 };', 'AB4806', 'the non-finite number `Infinity`'], + ['negated overflowing numeric literal', 'export const config = { limit: -1e999 };', 'AB4806', 'the non-finite number `-Infinity`'], ['bigint literal', 'export const config = { big: 1n };', 'AB4806', 'a bigint literal'], ['let declaration', 'export let config = {};', 'AB4805', 'a mutable `let`/`var` declaration'], ['destructuring', 'export const { config } = source;', 'AB4805', 'a destructuring declaration'], diff --git a/packages/agent-bundle/tests/route-graph.test.ts b/packages/agent-bundle/tests/route-graph.test.ts index b11b97f64..46b214844 100644 --- a/packages/agent-bundle/tests/route-graph.test.ts +++ b/packages/agent-bundle/tests/route-graph.test.ts @@ -139,6 +139,25 @@ it('skips ignored paths, private segments, and declaration files', async () => { expect(graph.scripts).toEqual([]); }); +it('discovers .jsx script routes so the rendered-script gate can judge them', async () => { + const root = await createRoot(); + await writeTree(root, { + 'src/scripts/rebuild-index.ts': moduleSource, + 'src/scripts/render-poster.jsx': 'export default async () =>
poster
;\n', + }); + const graph = await compileRouteGraph(root, fixtureConfig()); + + // Discovery is not a packaging choice: the .jsx module compiles into the + // graph so source validation can gate it as AB4807 instead of dropping it. + expect(graph.diagnostics).toEqual([]); + expect(graph.scripts.map((route) => route.id)).toEqual(['script:rebuild-index', 'script:render-poster']); + expect(graph.scripts.find((route) => route.id === 'script:render-poster')).toMatchObject({ + kind: 'script', + provenance: { kind: 'conventional', relativePath: 'src/scripts/render-poster.jsx' }, + source: join(root, 'src/scripts/render-poster.jsx'), + }); +}); + it('never compiles a module explicit configuration claims: config always wins', async () => { const root = await createRoot(); await writeTree(root, {