Skip to content

Commit 70ef8ab

Browse files
fix(config): review follow-ups from #135/#143 (#165)
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
1 parent 7a9887b commit 70ef8ab

5 files changed

Lines changed: 83 additions & 9 deletions

File tree

‎packages/agent-bundle/src/routes/config-extract.ts‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,15 @@ export interface ExtractedRouteConfig {
3030
* methods, or accessors);
3131
* - array literals without spreads or holes;
3232
* - string literals and substitution-free template literals;
33-
* - numeric literals, optionally wrapped in unary `+`/`-`;
33+
* - finite numeric literals, optionally wrapped in unary `+`/`-`;
3434
* - `true`, `false`, and `null`;
3535
* - `as`/`satisfies` casts, non-null assertions, and parentheses around any
3636
* accepted form (they unwrap to their inner expression).
3737
*
3838
* Everything else — identifier references, calls, functions, templates with
39-
* substitutions, `undefined`, bigints, regular expressions — is dynamic and
40-
* raises AB4806 naming the offending construct.
39+
* substitutions, `undefined`, bigints, regular expressions, non-finite
40+
* numbers such as `1e999` — is dynamic and raises AB4806 naming the
41+
* offending construct.
4142
*/
4243
export const routeConfigGrammar = 'object/array/string/number/boolean/null literals, with as-const, satisfies, non-null, and parenthesis wrappers';
4344

@@ -111,6 +112,17 @@ const literalPropertyName = (name: ts.PropertyName): string | undefined => {
111112
return undefined;
112113
};
113114

115+
/**
116+
* Numeric literals must extract to finite numbers: an overflowing literal
117+
* such as `1e999` evaluates to `Infinity`, which `JSON.stringify` collapses
118+
* to `null` — the digest and inspection output could no longer distinguish
119+
* the config from one that declared `null`.
120+
*/
121+
const finiteNumber = (value: number, node: ts.Node): Extraction =>
122+
Number.isFinite(value)
123+
? { kind: 'value', value }
124+
: dynamic(`the non-finite number \`${String(value)}\``, node);
125+
114126
const extractExpression = (expression: ts.Expression): Extraction => {
115127
const node = unwrapExpression(expression);
116128
switch (node.kind) {
@@ -126,15 +138,15 @@ const extractExpression = (expression: ts.Expression): Extraction => {
126138
if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) {
127139
return { kind: 'value', value: node.text };
128140
}
129-
if (ts.isNumericLiteral(node)) return { kind: 'value', value: Number(node.text) };
141+
if (ts.isNumericLiteral(node)) return finiteNumber(Number(node.text), node);
130142
if (ts.isPrefixUnaryExpression(node)) {
131143
const operand = unwrapExpression(node.operand);
132144
if (
133145
ts.isNumericLiteral(operand) &&
134146
(node.operator === ts.SyntaxKind.MinusToken || node.operator === ts.SyntaxKind.PlusToken)
135147
) {
136148
const magnitude = Number(operand.text);
137-
return { kind: 'value', value: node.operator === ts.SyntaxKind.MinusToken ? -magnitude : magnitude };
149+
return finiteNumber(node.operator === ts.SyntaxKind.MinusToken ? -magnitude : magnitude, node);
138150
}
139151
return dynamic(describeExpression(node), node);
140152
}
@@ -151,7 +163,10 @@ const extractExpression = (expression: ts.Expression): Extraction => {
151163
return { kind: 'value', value: values };
152164
}
153165
if (ts.isObjectLiteralExpression(node)) {
154-
const value: Record<string, unknown> = {};
166+
// A null-prototype carrier keeps a literal `__proto__` key an ordinary
167+
// own property; assigning through a plain `{}` would invoke the legacy
168+
// prototype setter and silently drop the declared property.
169+
const value: Record<string, unknown> = Object.create(null) as Record<string, unknown>;
155170
for (const property of node.properties) {
156171
if (!ts.isPropertyAssignment(property)) return dynamic(describeExpression(property), property);
157172
const name = literalPropertyName(property.name);
@@ -214,8 +229,11 @@ const findConfigExport = (sourceFile: ts.SourceFile): ConfigExportSite | undefin
214229
return undefined;
215230
};
216231

217-
const scriptKindOf = (relativePath: string): ts.ScriptKind =>
218-
relativePath.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS;
232+
const scriptKindOf = (relativePath: string): ts.ScriptKind => {
233+
if (relativePath.endsWith('.tsx')) return ts.ScriptKind.TSX;
234+
if (relativePath.endsWith('.jsx')) return ts.ScriptKind.JSX;
235+
return ts.ScriptKind.TS;
236+
};
219237

220238
const positionOf = (sourceFile: ts.SourceFile, node: ts.Node): string => {
221239
const { character, line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile));

‎packages/agent-bundle/src/routes/graph.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ const routeGlobs = [
3737
'src/events/*/*.{ts,tsx}',
3838
'src/providers/*.{ts,tsx}',
3939
'src/cli/**/*.{ts,tsx}',
40-
'src/scripts/**/*.{ts,tsx}',
40+
// Scripts also discover .jsx: the stage-1 script gate judges rendered
41+
// modules (AB4807), so a .jsx script must surface there, never vanish.
42+
'src/scripts/**/*.{ts,tsx,jsx}',
4143
];
4244

4345
const mcpRouteKinds: Readonly<Record<string, CompiledRouteKind>> = {

‎packages/agent-bundle/tests/normalization.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,7 @@ it('gates rendered, nested, and conflicting conventional script routes as AB4807
892892
'src/scripts/detect-risk.ts',
893893
'src/scripts/release/tag.ts',
894894
'src/scripts/render-notes.tsx',
895+
'src/scripts/render-poster.jsx',
895896
'src/scripts/verify-release.ts',
896897
]),
897898
skills: [],
@@ -922,6 +923,13 @@ it('gates rendered, nested, and conflicting conventional script routes as AB4807
922923
severity: 'error',
923924
sourcePath: `${root}/src/scripts/render-notes.tsx`,
924925
},
926+
{
927+
code: 'AB4807',
928+
message: 'Conventional script src/scripts/render-poster.jsx is a rendered-script module; rendered scripts are not supported yet.',
929+
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.',
930+
severity: 'error',
931+
sourcePath: `${root}/src/scripts/render-poster.jsx`,
932+
},
925933
]);
926934
});
927935

‎packages/agent-bundle/tests/route-config-extract.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,31 @@ it('parses TSX modules whose bodies contain JSX', () => {
4646
expect(config).toEqual({ title: 'App' });
4747
});
4848

49+
it('parses JSX modules whose bodies contain JSX', () => {
50+
const { config, diagnostics } = extract([
51+
"export const config = { title: 'Poster' };",
52+
'export default function Poster() { return <section>poster</section>; }',
53+
].join('\n'), 'src/scripts/render-poster.jsx');
54+
expect(diagnostics).toEqual([]);
55+
expect(config).toEqual({ title: 'Poster' });
56+
});
57+
58+
it('preserves a literal "__proto__" key as an own config property', () => {
59+
const { config, diagnostics } = extract(
60+
'export const config = { "__proto__": { injected: true }, title: \'safe\' };',
61+
);
62+
expect(diagnostics).toEqual([]);
63+
// The key is an ordinary own data property: enumerated, serialized, and
64+
// frozen like any other — never a prototype swap that inspection and the
65+
// digest would silently drop.
66+
expect(Object.keys(config)).toEqual(['__proto__', 'title']);
67+
const descriptor = Object.getOwnPropertyDescriptor(config, '__proto__');
68+
expect(descriptor?.value).toEqual({ injected: true });
69+
expect(JSON.parse(JSON.stringify(config))).toMatchObject({ title: 'safe' });
70+
expect(JSON.stringify(config)).toContain('"__proto__":{"injected":true}');
71+
expect(Object.isFrozen(descriptor?.value)).toBe(true);
72+
});
73+
4974
it('extracts silently to the empty config when no config export exists', () => {
5075
const { config, diagnostics } = extract('export default () => null;\nconst config = { hidden: true };');
5176
expect(diagnostics).toEqual([]);
@@ -62,6 +87,8 @@ it.each([
6287
['method', 'export const config = { run() { return 1; } };', 'AB4806', 'a method or accessor'],
6388
['array spread', 'export const config = { tags: [...list] };', 'AB4806', 'a spread'],
6489
['undefined value', 'export const config = { title: undefined };', 'AB4806', 'the non-JSON value `undefined`'],
90+
['overflowing numeric literal', 'export const config = { limit: 1e999 };', 'AB4806', 'the non-finite number `Infinity`'],
91+
['negated overflowing numeric literal', 'export const config = { limit: -1e999 };', 'AB4806', 'the non-finite number `-Infinity`'],
6592
['bigint literal', 'export const config = { big: 1n };', 'AB4806', 'a bigint literal'],
6693
['let declaration', 'export let config = {};', 'AB4805', 'a mutable `let`/`var` declaration'],
6794
['destructuring', 'export const { config } = source;', 'AB4805', 'a destructuring declaration'],

‎packages/agent-bundle/tests/route-graph.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,25 @@ it('skips ignored paths, private segments, and declaration files', async () => {
139139
expect(graph.scripts).toEqual([]);
140140
});
141141

142+
it('discovers .jsx script routes so the rendered-script gate can judge them', async () => {
143+
const root = await createRoot();
144+
await writeTree(root, {
145+
'src/scripts/rebuild-index.ts': moduleSource,
146+
'src/scripts/render-poster.jsx': 'export default async () => <section>poster</section>;\n',
147+
});
148+
const graph = await compileRouteGraph(root, fixtureConfig());
149+
150+
// Discovery is not a packaging choice: the .jsx module compiles into the
151+
// graph so source validation can gate it as AB4807 instead of dropping it.
152+
expect(graph.diagnostics).toEqual([]);
153+
expect(graph.scripts.map((route) => route.id)).toEqual(['script:rebuild-index', 'script:render-poster']);
154+
expect(graph.scripts.find((route) => route.id === 'script:render-poster')).toMatchObject({
155+
kind: 'script',
156+
provenance: { kind: 'conventional', relativePath: 'src/scripts/render-poster.jsx' },
157+
source: join(root, 'src/scripts/render-poster.jsx'),
158+
});
159+
});
160+
142161
it('never compiles a module explicit configuration claims: config always wins', async () => {
143162
const root = await createRoot();
144163
await writeTree(root, {

0 commit comments

Comments
 (0)