Skip to content

Commit ffcf1d9

Browse files
committed
fix(vba): strip a UTF-8 BOM before parsing Dysflow JSON artifacts
`.json` is not in VBA_FAMILY_RE, so test manifests and test sequences are read with a plain utf-8 readFile and never pass through readVbaSource, which is what strips the BOM for the rest of the VBA family. Node keeps the mark as a leading \uFEFF and JSON.parse rejects it, so a BOM-carrying manifest produced no file node and no vba-test-manifest references at all — only a warning-severity parse error. PowerShell 5.1 writes that BOM by default for -Encoding UTF8, so hand-edited manifests hit this routinely on Windows. Add stripJsonBom in a dedicated json-source module and apply it in both extractors. It is deliberately not folded into vba-source.ts: JSON is UTF-8 by specification, so the CP1252 fallback there would accept a mis-encoded manifest as silently wrong data instead of failing loudly. Malformed JSON still reports the same warning-severity parse_error, and a BOM does not carry a wrong-shaped document past the content gates. Refs #316
1 parent 7b4319e commit ffcf1d9

6 files changed

Lines changed: 121 additions & 3 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
1313

1414
- Fork documentation now describes v1.17.1 capabilities and pins the integrated upstream v1.4.1 lineage, replacing stale version, feature-parity, and implementation-plan claims. (#312)
1515
- Dysflow exports are no longer read twice. Every form and report layout, test manifest and test sequence was being parsed once by the VBA dispatcher and again by the Dysflow framework hook, which made a full index do double the work on those files and left duplicate pending references behind. Turning Dysflow expansion off with `vba.dysflowExport: false` is also honoured again in every case. (#314)
16+
- A test manifest or test sequence saved with a UTF-8 byte-order mark is read correctly again. Until now such a file was rejected outright, so every test it registered quietly vanished from the graph with nothing but a warning to show for it — and on Windows that mark is what PowerShell writes by default. (#316)
1617

1718
## [1.17.1] - 2026-09-12
1819

‎__tests__/extraction-vba-test-manifest.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,43 @@ describe('VbaTestManifestExtractor — manifest references (SUB-2)', () => {
139139
expect(refs.map((u) => u.referenceName)).toEqual(['Test_Ok']);
140140
});
141141
});
142+
143+
describe('issue #316 — a UTF-8 BOM must not discard a manifest', () => {
144+
// `.json` is not in VBA_FAMILY_RE, so manifests are read as plain UTF-8 and
145+
// Node keeps the BOM as a leading . `JSON.parse` rejects it, which used
146+
// to turn a valid manifest into a warning and zero references. PowerShell 5.1
147+
// writes that BOM by default for `-Encoding UTF8`.
148+
const MANIFEST = JSON.stringify({
149+
tests: [{ procedure: 'Test_Bom_RunAll', name: 'bom', tags: ['smoke'] }],
150+
});
151+
152+
it('emits the same nodes and references as the same manifest without a BOM', () => {
153+
const plain = extract('tests/tests.vba.bom.json', MANIFEST);
154+
const withBom = extract('tests/tests.vba.bom.json', '' + MANIFEST);
155+
156+
expect(withBom.errors).toEqual([]);
157+
expect(withBom.nodes.map((n) => n.id)).toEqual(plain.nodes.map((n) => n.id));
158+
expect(withBom.unresolvedReferences).toEqual(plain.unresolvedReferences);
159+
expect(
160+
withBom.unresolvedReferences.map((u) => u.referenceName),
161+
).toEqual(['Test_Bom_RunAll']);
162+
});
163+
164+
it('still reports genuinely malformed JSON as a warning with no nodes', () => {
165+
const r = extract('tests/tests.vba.broken.json', '{ "tests": [');
166+
expect(r.nodes).toEqual([]);
167+
expect(r.errors).toHaveLength(1);
168+
expect(r.errors[0]?.severity).toBe('warning');
169+
expect(r.errors[0]?.code).toBe('parse_error');
170+
});
171+
172+
it('does not let a BOM sneak a wrong-shaped JSON past the content gate', () => {
173+
const r = extract(
174+
'tests/tests.vba.notamanifest.json',
175+
'' + JSON.stringify({ slices: [{ submanifests: [] }] }),
176+
);
177+
expect(r.nodes).toEqual([]);
178+
expect(r.unresolvedReferences).toEqual([]);
179+
expect(r.errors).toEqual([]);
180+
});
181+
});

‎__tests__/extraction-vba-test-sequence.test.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,4 +312,44 @@ describe('VbaTestSequenceExtractor — resolver integration', () => {
312312

313313
await cg.destroy();
314314
});
315-
});
315+
});
316+
317+
describe('issue #316 — a UTF-8 BOM must not discard a sequence', () => {
318+
// Same failure mode as the test manifests: `sequences/*.json` is read as
319+
// plain UTF-8, Node keeps the leading , and `JSON.parse` rejects it.
320+
const SEQUENCE = JSON.stringify({
321+
description: 'bom',
322+
runnerPolicy: { tool: 'dysflow', sequential: true },
323+
procedures: ['Test_Bom_RunSlice', 'Test_Bom_Reset'],
324+
});
325+
326+
it('emits the same nodes and references as the same sequence without a BOM', () => {
327+
const plain = extract('tests/sequences/bom.json', SEQUENCE);
328+
const withBom = extract('tests/sequences/bom.json', '' + SEQUENCE);
329+
330+
expect(withBom.errors).toEqual([]);
331+
expect(withBom.nodes.map((n) => n.id)).toEqual(plain.nodes.map((n) => n.id));
332+
expect(withBom.unresolvedReferences).toEqual(plain.unresolvedReferences);
333+
expect(
334+
withBom.unresolvedReferences.map((u) => u.referenceName),
335+
).toEqual(['Test_Bom_RunSlice', 'Test_Bom_Reset']);
336+
});
337+
338+
it('still reports genuinely malformed JSON as a warning with no nodes', () => {
339+
const r = extract('tests/sequences/broken.json', '{ "procedures": [');
340+
expect(r.nodes).toEqual([]);
341+
expect(r.errors).toHaveLength(1);
342+
expect(r.errors[0]?.severity).toBe('warning');
343+
expect(r.errors[0]?.code).toBe('parse_error');
344+
});
345+
346+
it('does not let a BOM sneak the strict-sequence shape past the content gate', () => {
347+
const r = extract(
348+
'tests/sequences/strict.json',
349+
'' + JSON.stringify({ executionUnits: ['tests/tests.vba.json'] }),
350+
);
351+
expect(r.nodes).toEqual([]);
352+
expect(r.unresolvedReferences).toEqual([]);
353+
expect(r.errors).toEqual([]);
354+
});
355+
});

‎src/extraction/json-source.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Decoding helpers for the JSON artifacts Dysflow writes beside a VBA export
3+
* (`tests(.<slice>)*.json` test manifests, `sequences/*.json` test sequences).
4+
*
5+
* These are the only indexed files that are parsed as JSON rather than scanned
6+
* as text, and they are deliberately NOT part of `isVbaFamilyFile` — JSON is
7+
* UTF-8 by specification, so the CP1252 fallback in `readVbaSource` would turn
8+
* a mis-encoded manifest into silently wrong data instead of a loud parse
9+
* error. What they do share with the rest of the VBA family is the byte-order
10+
* mark.
11+
*/
12+
13+
/**
14+
* Strip a leading UTF-8 BOM from already-decoded JSON text.
15+
*
16+
* A `.json` is read with a plain `fs.readFile(path, 'utf-8')`, and Node keeps
17+
* the BOM as a leading `\uFEFF` rather than consuming it. `JSON.parse` rejects
18+
* that character outright, so a BOM-carrying manifest used to contribute
19+
* nothing to the graph but a `warning`-severity parse error — every `Test_*`
20+
* link it declared silently vanished (issue #316). On Windows this is the
21+
* common case, not an exotic one: PowerShell 5.1 writes the BOM by default for
22+
* `-Encoding UTF8` in both `Set-Content` and `Out-File`.
23+
*
24+
* Only a BOM at position 0 is removed; a `\uFEFF` anywhere else is data and is
25+
* left for `JSON.parse` to judge.
26+
*/
27+
export function stripJsonBom(text: string): string {
28+
return text.charCodeAt(0) === 0xfeff ? text.slice(1) : text;
29+
}

‎src/extraction/vba-test-manifest-extractor.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import * as path from 'path';
2525
import { Node, ExtractionResult, ExtractionError, UnresolvedReference } from '../types';
2626
import { generateNodeId } from './tree-sitter-helpers';
27+
import { stripJsonBom } from './json-source';
2728

2829
/** One `tests[]` entry that carries a string `procedure`. */
2930
interface ManifestTestEntry {
@@ -66,7 +67,10 @@ export class VbaTestManifestExtractor {
6667

6768
let parsed: unknown;
6869
try {
69-
parsed = JSON.parse(this.source);
70+
// A leading UTF-8 BOM makes `JSON.parse` throw, which would turn a
71+
// perfectly valid manifest into a warning and zero references (#316).
72+
// PowerShell 5.1 writes that BOM by default for `-Encoding UTF8`.
73+
parsed = JSON.parse(stripJsonBom(this.source));
7074
} catch (error) {
7175
// Malformed manifest — low-severity so it never breaks the index.
7276
this.errors.push({

‎src/extraction/vba-test-sequence-extractor.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import * as path from 'path';
3535
import { Node, ExtractionResult, ExtractionError, UnresolvedReference } from '../types';
3636
import { generateNodeId } from './tree-sitter-helpers';
37+
import { stripJsonBom } from './json-source';
3738

3839
/**
3940
* Content-shape gate: is `parsed` a Dysflow VBA test sequence — a top-level
@@ -74,7 +75,10 @@ export class VbaTestSequenceExtractor {
7475

7576
let parsed: unknown;
7677
try {
77-
parsed = JSON.parse(this.source);
78+
// A leading UTF-8 BOM makes `JSON.parse` throw, which would turn a
79+
// perfectly valid sequence into a warning and zero references (#316).
80+
// PowerShell 5.1 writes that BOM by default for `-Encoding UTF8`.
81+
parsed = JSON.parse(stripJsonBom(this.source));
7882
} catch (error) {
7983
this.errors.push({
8084
message: `VBA test sequence parse error: ${

0 commit comments

Comments
 (0)