Skip to content

emrg: fix Monaco AMD define hijacking UMD marked — markdown degraded to plain text (rant 2026-08-15T10:48:58) - #805

Merged
argszero merged 2 commits into
masterfrom
feature/monaco-amd-markdown-fix
Aug 15, 2026
Merged

emrg: fix Monaco AMD define hijacking UMD marked — markdown degraded to plain text (rant 2026-08-15T10:48:58)#805
argszero merged 2 commits into
masterfrom
feature/monaco-amd-markdown-fix

Conversation

@argszero

Copy link
Copy Markdown
Owner

Critical regression fix: after #801 (Monaco editor), chat and rant-detail markdown degraded to plain text.

Root cause (100% code-confirmed):

  • monaco-editor 0.56.0's vendor/monaco/vs/loader.js defines GLOBAL AMD define/require on window (loader.js L1347 u.global.define = p).
  • marked.min.js is UMD: it detects define.amd and takes the AMD define branch → window.marked is never set.
  • index.html loaded monaco loader.js (L390) BEFORE marked/dompurify/highlight (L403-405) → window.marked empty → renderMarkdown/streamProject all degraded to escapeHtml plain text (chat + rant details).

Fixes:

  1. index.html: marked/dompurify/highlight now load BEFORE monaco loader.js — no AMD present at their load time → UMD branch → window.marked set; Monaco's AMD define afterwards does not affect already-loaded globals.
  2. markdown.js: defensive AMD-module-table recovery (recoverAmdModule("marked") reads monaco require.s/define.amd.modules) before degrading; upgraded the missing-vendor log to a console.error naming the AMD-hijack cause for future diagnosis.
  3. Tests: build-config.test.js order guard (marked/dompurify/highlight must precede loader.js — fails if order regresses) + renderer.smoke AMD-recovery test (window.marked missing + AMD table present → renders via recovered marked; both missing → escapeHtml fallback).
  4. Agent.md GUI count synced 253→255 (renderer smoke 127, build-config 7).

Verification: GUI npm test 255/255; pytest 824 passed/1 skipped; doc-count guard 3/3; import + CLI checks green. Rant details markdown (and its pending 【】-header enhancement) now render again once this lands.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle

Reviewed the full diff: root cause correctly identified (monaco loader.js defines global AMD define/require at L1347 → UMD marked takes the define branch → window.marked never set → plain-text degradation); index.html order fix (UMD vendors before monaco loader) is the right structural fix; recoverAmdModule() fallback reading require.s module table is a sound defensive layer with positive/negative test coverage; console.error diagnostic names the cause; order-guard test locks the fix against regression. CI test + test-windows both SUCCESS (run 31860446814). Agent.md counts synced (253→255).

@pm25coder

Copy link
Copy Markdown
Contributor

I tested this PR end-to-end on Windows and the root-cause fix checks out.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle

Reviewed the full diff: script-order fix is the correct root-cause resolution (UMD marked sees AMD define from monaco loader → never sets window.marked; loading marked/dompurify/highlight first makes them take the UMD branch, and monaco's AMD define afterwards doesn't disturb already-set globals). The AMD module-table recovery in markdown.js is a sound defensive layer (require.s / define.amd.modules), and the console.error names the exact cause for future diagnosis. Order-guard test (build-config) + AMD-recovery test (renderer.smoke) both cover the regression; Agent.md counts synced (253→255). CI test + test-windows SUCCESS (run 31860446814).

@argszero
argszero merged commit a65a2a8 into master Aug 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants