Skip to content

Commit d8cca00

Browse files
ecthelion77Olivier Gintrandcrivetimihai
authored andcommitted
fix: resolve Alpine.js MutationObserver race condition in observability sub-views (#3967)
* fix: resolve Alpine.js MutationObserver race condition in observability sub-views When observability sub-partials (Tools, Metrics, Prompts, Resources) are dynamically loaded via fetch(), Alpine.js's MutationObserver auto-initializes x-data components before the inline <script> tags defining their controllers have executed, causing 'createToolsController is not defined' errors. Move script extraction logic to a native <script> block (browser-executed, not Alpine's new Function() evaluator): - Extract <script> blocks from raw HTML via regex (new RegExp with string concatenation avoids </script> literal breaking HTML parsing) - Execute via document.createElement('script').text + appendChild (jQuery globalEval pattern — synchronous, global scope) - Insert clean (script-free) HTML inside Alpine.mutateDom() to suppress MutationObserver, then call Alpine.initTree() explicitly Fixes #3966 Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com> * test(ui): add unit tests for observability __obsExecAndStrip helper Add vitest/jsdom tests covering the script extraction, execution, and Alpine.mutateDom + initTree injection pattern introduced in the observability dashboard race condition fix. Covers: script ordering, attribute handling, error recovery, cleanup, controller factory simulation, and Alpine fallback paths. Ref: #3967 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(ui): harden Alpine guard and warn on unsupported external scripts Address review feedback on the observability race-condition fix: - Harden Alpine.js guard to check typeof mutateDom/initTree === 'function' before calling, consistent with initialization.js pattern (L956, L1505). - Detect and warn on <script src="..."> in dynamic partials instead of silently dropping them. No sub-partials currently use external scripts. - Update __obsExecAndStrip regex to capture attributes separately for the src detection. - Add 5 new tests: external script warning, partial Alpine API fallback (mutateDom missing, initTree missing, non-function values). - Add drift-risk note to test file header. Ref: #3967 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(ui): prevent false-positive src detection on data-src attributes The \bsrc\s*= regex matched data-src= because - is a word boundary, causing inline scripts with data-src attributes to be silently skipped. Use (?:^|\s)src\s*= to require whitespace before src, avoiding false positives on data-src, ng-src, or similar hyphenated attributes. Add regression test for data-src false-positive case. Ref: #3967 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Olivier Gintrand <olivier.gintrand@forterro.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Olivier Gintrand <olivier.gintrand@forterro.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 697b9bb commit d8cca00

2 files changed

Lines changed: 433 additions & 77 deletions

File tree

mcpgateway/templates/observability_partial.html

Lines changed: 64 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,34 @@
1313
};
1414
};
1515
}
16-
</script>
16+
/**
17+
* Execute <script> blocks from raw HTML text and return clean HTML.
18+
* Uses regex to extract scripts (avoids DOM innerHTML parsing issues)
19+
* and document.createElement('script') for reliable execution.
20+
*/
21+
window.__obsExecAndStrip = function(html) {
22+
var scriptRe = new RegExp('<' + 'script\\b([^>]*)>([\\s\\S]*?)</' + 'script>', 'gi');
23+
var m;
24+
while ((m = scriptRe.exec(html)) !== null) {
25+
var attrs = m[1];
26+
var code = m[2].trim();
27+
if (/(?:^|\s)src\s*=/i.test(attrs)) {
28+
console.warn('[obs] external <script src> not supported in dynamic partials, skipped');
29+
continue;
30+
}
31+
if (code) {
32+
try {
33+
var s = document.createElement('script');
34+
s.text = code;
35+
document.head.appendChild(s);
36+
s.remove();
37+
} catch (e) {
38+
console.error('[obs] script exec error:', e);
39+
}
40+
}
41+
}
42+
return html.replace(new RegExp('<' + 'script\\b[^>]*>[\\s\\S]*?</' + 'script>', 'gi'), '');
43+
};</script>
1744
<div class="observability-container" x-data="{
1845
viewMode: 'traces',
1946
selectedTrace: null,
@@ -79,26 +106,19 @@
79106
return;
80107
}
81108
82-
container.innerHTML = html;
83-
84-
// Execute any script tags (innerHTML doesn't execute them)
85-
const scripts = container.querySelectorAll('script');
86-
scripts.forEach(script => {
87-
const newScript = document.createElement('script');
88-
if (script.src) {
89-
newScript.src = script.src;
90-
} else {
91-
newScript.textContent = script.textContent;
92-
}
93-
document.head.appendChild(newScript);
94-
});
95-
96-
// Wait for scripts to execute and charts to render
97-
await new Promise(resolve => setTimeout(resolve, 100));
98-
99-
// Re-initialize Alpine.js
100-
if (window.Alpine) {
109+
// Extract scripts from raw text and execute synchronously,
110+
// then get clean HTML without <script> tags
111+
const cleanHtml = window.__obsExecAndStrip(html);
112+
113+
// Insert script-free HTML with Alpine MutationObserver disabled,
114+
// then explicitly initialize Alpine components
115+
if (window.Alpine && typeof window.Alpine.mutateDom === 'function' && typeof window.Alpine.initTree === 'function') {
116+
window.Alpine.mutateDom(() => {
117+
container.innerHTML = cleanHtml;
118+
});
101119
window.Alpine.initTree(container);
120+
} else {
121+
container.innerHTML = cleanHtml;
102122
}
103123
104124
// Only mark as loaded if we're still on the metrics view
@@ -149,26 +169,15 @@
149169
return;
150170
}
151171
152-
container.innerHTML = html;
153-
154-
// Execute any script tags (innerHTML doesn't execute them)
155-
const scripts = container.querySelectorAll('script');
156-
scripts.forEach(script => {
157-
const newScript = document.createElement('script');
158-
if (script.src) {
159-
newScript.src = script.src;
160-
} else {
161-
newScript.textContent = script.textContent;
162-
}
163-
document.head.appendChild(newScript);
164-
});
165-
166-
// Wait for scripts to execute and charts to render
167-
await new Promise(resolve => setTimeout(resolve, 100));
168-
169-
// Re-initialize Alpine.js
170-
if (window.Alpine) {
172+
const cleanHtml = window.__obsExecAndStrip(html);
173+
174+
if (window.Alpine && typeof window.Alpine.mutateDom === 'function' && typeof window.Alpine.initTree === 'function') {
175+
window.Alpine.mutateDom(() => {
176+
container.innerHTML = cleanHtml;
177+
});
171178
window.Alpine.initTree(container);
179+
} else {
180+
container.innerHTML = cleanHtml;
172181
}
173182
174183
// Only mark as loaded if we're still on the tools view
@@ -219,26 +228,15 @@
219228
return;
220229
}
221230
222-
container.innerHTML = html;
223-
224-
// Execute any script tags (innerHTML doesn't execute them)
225-
const scripts = container.querySelectorAll('script');
226-
scripts.forEach(script => {
227-
const newScript = document.createElement('script');
228-
if (script.src) {
229-
newScript.src = script.src;
230-
} else {
231-
newScript.textContent = script.textContent;
232-
}
233-
document.head.appendChild(newScript);
234-
});
235-
236-
// Wait for scripts to execute and charts to render
237-
await new Promise(resolve => setTimeout(resolve, 100));
238-
239-
// Re-initialize Alpine.js
240-
if (window.Alpine) {
231+
const cleanHtml = window.__obsExecAndStrip(html);
232+
233+
if (window.Alpine && typeof window.Alpine.mutateDom === 'function' && typeof window.Alpine.initTree === 'function') {
234+
window.Alpine.mutateDom(() => {
235+
container.innerHTML = cleanHtml;
236+
});
241237
window.Alpine.initTree(container);
238+
} else {
239+
container.innerHTML = cleanHtml;
242240
}
243241
244242
// Only mark as loaded if we're still on the prompts view
@@ -289,26 +287,15 @@
289287
return;
290288
}
291289
292-
container.innerHTML = html;
293-
294-
// Execute any script tags (innerHTML doesn't execute them)
295-
const scripts = container.querySelectorAll('script');
296-
scripts.forEach(script => {
297-
const newScript = document.createElement('script');
298-
if (script.src) {
299-
newScript.src = script.src;
300-
} else {
301-
newScript.textContent = script.textContent;
302-
}
303-
document.head.appendChild(newScript);
304-
});
305-
306-
// Wait for scripts to execute
307-
await new Promise(resolve => setTimeout(resolve, 100));
308-
309-
// Re-initialize Alpine.js
310-
if (window.Alpine) {
290+
const cleanHtml = window.__obsExecAndStrip(html);
291+
292+
if (window.Alpine && typeof window.Alpine.mutateDom === 'function' && typeof window.Alpine.initTree === 'function') {
293+
window.Alpine.mutateDom(() => {
294+
container.innerHTML = cleanHtml;
295+
});
311296
window.Alpine.initTree(container);
297+
} else {
298+
container.innerHTML = cleanHtml;
312299
}
313300
314301
// Only mark as loaded if we're still on the resources view

0 commit comments

Comments
 (0)