Skip to content

Commit 0524fc7

Browse files
czlonkowskiclaude
andcommitted
fix: fail closed on partial parses and widen the skew warning (review round 4)
- Reject any syntactic diagnostic before walking the AST: createSourceFile recovers from errors, so a truncated declarations file would otherwise yield a partial property set that reads as "no entity-only properties". The internal parseDiagnostics field disappearing also throws. - Collect only top-level IWorkflowSettings declarations - a same-named interface inside a namespace does not merge with the export. - The residual-skew warning now covers both pins (the fallback release can match n8n-workflow while shipping a different n8n-nodes-base) and the pins-unfetchable case. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
1 parent c113cde commit 0524fc7

2 files changed

Lines changed: 67 additions & 15 deletions

File tree

scripts/check-settings-drift.ts

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,32 @@ export function parseSchemaProperties(yaml: string): Set<string> {
131131
export function parseEntitySettingsProperties(dts: string): Set<string> {
132132
const source = ts.createSourceFile('interfaces.d.ts', dts, ts.ScriptTarget.Latest);
133133

134-
const declarations: ts.InterfaceDeclaration[] = [];
135-
const visit = (node: ts.Node): void => {
136-
if (ts.isInterfaceDeclaration(node) && node.name.text === ENTITY_INTERFACE) {
137-
declarations.push(node);
138-
}
139-
ts.forEachChild(node, visit);
140-
};
141-
visit(source);
134+
// createSourceFile recovers from syntax errors, so a truncated file (missing brace,
135+
// unterminated string) would yield a PARTIAL property set - reject anything that does not
136+
// parse cleanly. parseDiagnostics is internal API, so its disappearance must also throw
137+
// rather than quietly skipping the syntax gate.
138+
const diagnostics = (source as unknown as { parseDiagnostics?: readonly ts.Diagnostic[] })
139+
.parseDiagnostics;
140+
if (!Array.isArray(diagnostics)) {
141+
throw new Error(
142+
'TypeScript no longer exposes parseDiagnostics on SourceFile - rework this check to get ' +
143+
'syntax diagnostics from a Program before trusting the parse.'
144+
);
145+
}
146+
if (diagnostics.length > 0) {
147+
throw new Error(
148+
`n8n-workflow's declarations do not parse cleanly (` +
149+
`${ts.flattenDiagnosticMessageText(diagnostics[0].messageText, ' ')}) - a partial ` +
150+
'parse would under-report properties.'
151+
);
152+
}
153+
154+
// Top-level statements only: a same-named interface inside a namespace or module does not
155+
// merge with the export this check is after.
156+
const declarations = source.statements.filter(
157+
(statement): statement is ts.InterfaceDeclaration =>
158+
ts.isInterfaceDeclaration(statement) && statement.name.text === ENTITY_INTERFACE
159+
);
142160

143161
if (declarations.length === 0) {
144162
throw new Error(
@@ -363,13 +381,28 @@ async function main(): Promise<void> {
363381

364382
if (explicitVersion) {
365383
console.log('ℹ️ Entity axis skipped: the installed n8n-workflow may not match the requested version.\n');
366-
} else if (installedEntity && releasePins && releasePins.workflow !== installedEntity) {
367-
// Residual skew after resolution. The axis still runs: it can only fail loudly (a human
368-
// investigates at update time), never silently pass what a matching set would fail.
369-
console.log(
370-
`⚠️ Installed n8n-workflow ${installedEntity} differs from n8n ${version}'s pin ` +
371-
`${releasePins.workflow} - entity findings may reflect a neighbouring release.\n`
372-
);
384+
} else {
385+
// Residual skew after resolution, in either pin - the fallback release can match on
386+
// n8n-workflow while shipping a different n8n-nodes-base (and so a different schema).
387+
// The axis still runs: it can only fail loudly (a human investigates at update time),
388+
// never silently pass what a matching set would fail.
389+
const installedNodesBase = resolveVersion();
390+
const skews: string[] = [];
391+
if (releasePins && releasePins.nodesBase !== installedNodesBase) {
392+
skews.push(`n8n-nodes-base ${installedNodesBase} vs pin ${releasePins.nodesBase}`);
393+
}
394+
if (releasePins && installedEntity && releasePins.workflow !== installedEntity) {
395+
skews.push(`n8n-workflow ${installedEntity} vs pin ${releasePins.workflow}`);
396+
}
397+
if (!releasePins) {
398+
skews.push(`n8n ${version}'s pins could not be fetched`);
399+
}
400+
if (skews.length > 0) {
401+
console.log(
402+
`⚠️ Installed packages differ from n8n ${version}'s (${skews.join('; ')}) - ` +
403+
'findings may reflect a neighbouring release.\n'
404+
);
405+
}
373406
}
374407

375408
const schemaProperties = parseSchemaProperties(await fetchSchemaFile(version));

tests/unit/scripts/check-settings-drift.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,25 @@ describe('check-settings-drift parseEntitySettingsProperties', () => {
206206
expect([...parseEntitySettingsProperties(dts)]).toEqual(['first', 'second', 'third']);
207207
});
208208

209+
it('throws on a truncated file instead of returning the partial property set', () => {
210+
const dts = 'export interface IWorkflowSettings {\n timezone?: string;';
211+
expect(() => parseEntitySettingsProperties(dts)).toThrow(/parse cleanly/);
212+
});
213+
214+
it('does not accept a same-named interface nested in a namespace as the target', () => {
215+
const dts = [
216+
'export namespace Other {',
217+
' export interface IWorkflowSettings {',
218+
' ghost?: string;',
219+
' }',
220+
'}',
221+
'export interface IWorkflowSettings {',
222+
' timezone?: string;',
223+
'}',
224+
].join('\n');
225+
expect([...parseEntitySettingsProperties(dts)]).toEqual(['timezone']);
226+
});
227+
209228
it('throws on a member it cannot enumerate rather than skipping it', () => {
210229
const dts = [
211230
'export interface IWorkflowSettings {',

0 commit comments

Comments
 (0)