Skip to content

fix(diagrams): preserve literal SVG comments in CDATA (FU-1015) #1835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 5 commits into from
Oct 22, 2026
Merged

fix(diagrams): preserve literal SVG comments in CDATA (FU-1015) #1835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a3a1eab
test(diagrams): reproduce comment loss inside SVG CDATA
Cuarroc Oct 22, 2026
417eec2
fix(diagrams): preserve literal SVG comments inside CDATA
Cuarroc Oct 22, 2026
9208233
test(diagrams): reproduce comment leak after a processing instruction
Cuarroc Oct 22, 2026
9cf8738
fix(diagrams): preserve processing instructions before scanning CDATA
Cuarroc Oct 22, 2026
fc1d466
test(diagrams): run stripper cases in the export gate
Cuarroc Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/PLAN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ Quelle: `opt-ci-merge-20261015.md` (pa-orch); Dispatch laut Root, Ereignisse 16:
| OPT-PIPE-0 | Durchlauf ausschließlich lesend messen | srv-opt-pipeline; Root klärt Basis | korrekte Auftragsbasis | Server | C (Bericht) | Blockiert: geplant nach Basisklärung; Server-Auftrag srv-opt-pipeline endete 16:12 MESZ wegen HEAD-Abweichung | Messbericht mit Methode und Einheiten; keine Implementierung | OPT-PIPE-0 |
| CI-SEC-CODEQL | [Archiv][pakete] | Root | – | – | B | Erledigt: #1581 (merged 2026-10-15T18:25:41Z, `ded1eef5`); Alarm 57 laut GitHub Code Scanning `fixed` | siehe Archiv | – |
| CI-SEC-CODEQL-2 | [Archiv][pakete] | Root; `.github/` gehört laut CODEOWNERS dem Nutzer | Nutzerentscheid CODEQL-59-60 | – | B | Erledigt: CodeQL-Alarme 59/60 am 16.10.2026 nach Owner-Entscheid K1 (A: als Fehlalarm abweisen) als false positive abgewiesen (GitHub: `dismissed`, 00:08:30/31 UTC); neu prüfen bei Änderungen an Cache/Event-Konfiguration in `ci.yml` | siehe Archiv | – |
| FU-1015 | Nachläufer aus den Reviews vom 15.10.: #1551 G3 (KR-09m), #1556 K4 und toter CSS-Rest (KR-08a), #1560 K2 (CI-SPLIT-LINUX), #1561 G2 (DS-01), #1568 Lows (FABRIK-TAB-1), #1569 B1/B3 (KR-04a), #1581 K1/K3–K5 (CI-SEC-CODEQL) | Root disponiert | jeweiliger Eltern-PR gemergt | – | je Punkt | Geplant: nicht disponiert (laut Root, Stand 15.10.) | je Punkt eigenes Paket ≤ 300 Zeilen mit eigener Abnahme | – |
| FU-1015 | Nachläufer aus den Reviews vom 15.10.: #1551 G3 (KR-09m), #1556 K4 und toter CSS-Rest (KR-08a), #1560 K2 (CI-SPLIT-LINUX), #1561 G2 (DS-01), #1568 Lows (FABRIK-TAB-1), #1569 B1/B3 (KR-04a), #1581 K1/K3–K5 (CI-SEC-CODEQL) | Root disponiert | jeweiliger Eltern-PR gemergt | – | je Punkt | In Arbeit: Teillieferung #1581 K1 durch diesen PR #1835 nach Merge; K3 durch #1833 gemergt (`72ae3f75`); K4/K5 separat in #1839, Merge offen; #1551/#1556/#1560/#1561/#1568/#1569 bleiben offen | je Punkt eigenes Paket ≤ 300 Zeilen mit eigener Abnahme | – |
| OPS-CLOUD-PREFLIGHT | Vorabprüfung für den Codex-Cloud-Weg vor jedem Auftrag (laut Root 15.10.; Quelle: nur diese Mitteilung belegt, Datei geprüft 16.10. und nicht gefunden: weder Auftrag noch Bericht noch Ereignislog des Server-Auftragsordners nennt die ID; Wortlaut und Zielort liefert Root, sonst Zeile streichen) | Root disponiert | – | – | – | Geplant: nicht disponiert (15.10.) | Abnahme vor Dispatch festlegen | – |
| OPS-JOB-ENDZEILE | Endzeile jedes Auftrags nennt private Abrechnungs- und Token-Messwerte; keine Werte im öffentlichen Repo (laut Root 15.10.; Quelle: nur diese Mitteilung belegt, Datei geprüft 16.10. und nicht gefunden; Wortlaut und Zielort liefert Root, sonst Zeile streichen) | Root disponiert | – | – | – | Geplant: nicht disponiert (15.10.) | Abnahme vor Dispatch festlegen | – |
| OPS-AUTO-LOCK | Auto-Skript: doppelte Sperre (laut Root 15.10. „duplicate lock“; Quelle: nur diese Mitteilung belegt, Datei geprüft 16.10. und nicht gefunden; Bedeutung bleibt Unbekannt, Root liefert Wortlaut und Zielort, sonst Zeile streichen) | Root disponiert | – | – | – | Geplant: nicht disponiert (15.10.) | Abnahme vor Dispatch festlegen | – |
Expand Down
1 change: 1 addition & 0 deletions docs/diagrams/export-svg.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { stripComments } from './strip-comments.mjs';
import './strip-comments.test.mjs';

const script = fileURLToPath(new URL('./export-svg.mjs', import.meta.url));

Expand Down
15 changes: 14 additions & 1 deletion docs/diagrams/strip-comments.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,21 @@
// Separate from export-svg.mjs so tests can import it without Chromium or argv.
export function stripComments(s) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the FU-1015 plan status

When this implementation lands, docs/PLAN.md:365 will still describe FU-1015 as “Geplant: nicht disponiert,” even though this commit implements its #1581 K1 follow-up. Update the row in this commit so the canonical plan reflects the post-merge state; the repository explicitly treats this mismatch as a finding that must be resolved before acceptance.

AGENTS.md reference: AGENTS.md:L139-L139

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Obtain the second Tier-B review

Before accepting or merging this Tier-B change, obtain and record the required second independent review. The commit’s review disposition names only one non-author reviewer (glm-5.2:cloud) before declaring acceptance, while Tier B requires two reviewers from different non-author families, using the documented subscription/OpenRouter pairing or its E1 substitution.

AGENTS.md reference: AGENTS.md:L28-L33

Useful? React with 👍 / 👎.

const parts = [];
let start = 0;
// Consume processing instructions and comments before interpreting CDATA.
for (const match of s.matchAll(/<\?[\s\S]*?\?>|<!\[CDATA\[[\s\S]*?\]\]>|<!--(?:[\s\S]*?-->)?/g)) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore CDATA openers in DTD entity values

When the input is a valid SVG with an internal DTD subset, such as <!DOCTYPE svg [<!ENTITY marker "<![CDATA[">]><svg><!-- remove --><![CDATA[x]]></svg>, this matcher treats the quoted entity value as a real CDATA opener and consumes through the document's genuine CDATA terminator. As a result, <!-- remove --> survives unchanged; lex the DOCTYPE so quoted values cannot open CDATA, and add this sibling case to the regression coverage.

AGENTS.md reference: AGENTS.md:L110-L112

Useful? React with 👍 / 👎.

if (match[0].startsWith('<!--')) continue;
parts.push(stripMarkupComments(s.slice(start, match.index)), match[0]);
start = match.index + match[0].length;
}
parts.push(stripMarkupComments(s.slice(start)));
return parts.join('');
}

// The optional group also removes an unterminated `<!--`; repeating until the
// string is stable catches openers that a removed comment re-assembled
// (`<!<!---->-->`), so none survives (CodeQL js/incomplete-multi-character-sanitization).
export function stripComments(s) {
function stripMarkupComments(s) {
let prev;
do { prev = s; s = s.replace(/<!--(?:[\s\S]*?-->)?/g, ''); } while (s !== prev);
return s;
Expand Down
62 changes: 62 additions & 0 deletions docs/diagrams/strip-comments.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { stripComments } from './strip-comments.mjs';

test('stripComments treats processing instructions as opaque before CDATA', () => {
const cases = [
[
'<svg><?probe <![CDATA[ ?><!-- remove --><![CDATA[x]]></svg>',
'<svg><?probe <![CDATA[ ?><![CDATA[x]]></svg>',
],
[
'<svg><?probe <!-- literal --> ?><![CDATA[<!-- keep -->]]><!-- remove --></svg>',
'<svg><?probe <!-- literal --> ?><![CDATA[<!-- keep -->]]></svg>',
],
[
'<svg><!-- <?probe <![CDATA[ ?> --><![CDATA[<?probe <!-- keep --> ?>]]></svg>',
'<svg><![CDATA[<?probe <!-- keep --> ?>]]></svg>',
],
];
for (const [input, expected] of cases) assert.equal(stripComments(input), expected);
});

test('stripComments preserves literal comment text in CDATA', () => {
const cases = [
[
'<svg><!-- remove --><text><![CDATA[A <!-- literal --> B]]></text><!-- remove too --></svg>',
'<svg><text><![CDATA[A <!-- literal --> B]]></text></svg>',
],
[
'<svg><text><![CDATA[<!-- unfinished]]></text><!-- remove --></svg>',
'<svg><text><![CDATA[<!-- unfinished]]></text></svg>',
],
[
'<svg><text><![CDATA[<!<!---->-->]]></text></svg>',
'<svg><text><![CDATA[<!<!---->-->]]></text></svg>',
],
[
'<svg><!-- <![CDATA[hidden]]> --><text><![CDATA[<!-- keep -->]]></text></svg>',
'<svg><text><![CDATA[<!-- keep -->]]></text></svg>',
],
[
'<svg><text><![CDATA[left <!-- a -->]]><![CDATA[<!-- b --> right]]></text></svg>',
'<svg><text><![CDATA[left <!-- a -->]]><![CDATA[<!-- b --> right]]></text></svg>',
],
];
for (const [input, expected] of cases) assert.equal(stripComments(input), expected);
});

test('stripComments retains removal behavior outside CDATA', () => {
const cases = [
['', ''],
['<svg/>', '<svg/>'],
['<svg><!-- one\n two --><text>&lt;!-- literal --&gt;</text></svg>', '<svg><text>&lt;!-- literal --&gt;</text></svg>'],
['<!<!---->-->', '>'],
['<!<!---->--', ''],
['<!--<!-- a -->', ''],
['<<!---->!-- x -->', ''],
['a<!-- never closed', 'a never closed'],
['<![C<!---->DATA[<!<!---->--x]]>', '<![CDATA[x]]>'],
];
for (const [input, expected] of cases) assert.equal(stripComments(input), expected);
});
Loading