feat: pilot Monaco editor in Code Formatter tool

[vuon9]

Summary

Pilots Monaco as the code editor in the Code Formatter tool only, behind an impl="monaco" prop on ToolEditorPane. All other tools continue using CodeMirror so we can A/B the experience before committing to a wider migration.

• Monaco is bundled locally (workers via Vite ?worker imports); no CDN dependency, keeping the app offline-first
• New imperative MonacoCodeEditor / MonacoHighlightedCode components (same props contract as their CodeMirror counterparts), lazy-loaded so only the pilot page pulls the ~2.6MB chunk
• Editor theme is derived from the active theme palette; CSS-variable token colors are resolved to hex and invalid tokens are skipped instead of crashing (Monaco throws on non-hex colors)
• ThemeContext now exposes palette for editor theming
• e2e helpers understand Monaco's DOM (view-lines) and wait for async editor init; clipboard/typing helpers made deterministic against EditContext commits

Testing

• bun run test: 48 unit tests pass
• bunx playwright test: full e2e suite green (172 passed), incl. all 12 Code Formatter specs run 4x consecutively against a production build
• go test ./internal/...: pass

Screenshots

N/A

Checklist

• Formatted (bun run format)
• Backend changes: N/A

[github-actions]

PR Review — #163 pilot Monaco in Code Formatter — feat/monaco-pilot-code-formatter

Great isolation strategy (impl="monaco" prop, lazy chunk, workers via ?worker). Full e2e green is strong. Below are blocking/correctness issues + quality suggestions.

---

🔴 Bugs / Logic errors / Edge cases

1. normalizeColor rejects all 6-digit hex — breaks custom themes frontend/src/components/inputs/monaco/monacoSetup.js:71-75

if (/^#[0-9a-f]{3,8}$/i.test(c)) {
  if (c.length === 5 || c.length === 7) return null; // ← len 7 is #RRGGBB
}

#282c34 (one-dark-pro.json:6) and #fafafa are 7 chars incl. # and get null → pick() falls back to #1f2428/#ffffff. Also rejects #RGBA (len 5) which is valid CSS. Intent was to reject 5/9? Fix:

// allow #RGB(4), #RGBA(5), #RRGGBB(7), #RRGGBBAA(9)
if (c.length===5 || c.length===4) expand…; // actually keep #RGBA
// or simply: if c.length===9 && !8 hex → slice ; reject len 6(=7 with #) was wrong

Add quick unit test: expect(normalizeColor('#282c34')).toBe('#282c34').

2. Editable ToolEditorPane never shows line numbers frontend/src/components/inputs/ToolEditorPane.jsx:117-125, frontend/src/components/inputs/MonacoCodeEditor.jsx:60

<MonacoCodeEditor … /> // missing showLineNumbers={showLineNumbers}

MonacoCodeEditor implements lineNumbers: showLineNumbers?'on':'off' but prop is never forwarded. Same omission for fallback CodeEditor. Non-blocking for readonly output but inconsistent contract. Either forward it or drop prop from editable path explicitly.

3. Double model.dispose() after editor.dispose() frontend/src/components/inputs/MonacoCodeEditor.jsx:73-78, MonacoHighlightedCode.jsx:71-77
When editor is created with value: it owns its model; editor.dispose() already disposes it. Second model?.dispose() can throw model is disposed. Safer:

const model = editor.getModel();
editor.dispose();
if (model && !model.isDisposed()) model.dispose(); // or just omit

4. useMonacoDevtoolboxTheme stale-theme race frontend/src/components/inputs/monaco/useMonacoTheme.js:10-29

• First effect [] captures stale actualType/palette at mount; getMonaco().then applies stale theme if palette changed before promise resolves.
• Second effect checks if(monacoRef) which is still null while loading, so new palette never applied.
  Fix: keep palette/actualType in refs or chain getMonaco().then(()=>applyDevtoolboxTheme(monaco, currentPalette)) with deps, or store pending theme. Add cleanup flag.

5. Redundant theme apply frontend/src/components/inputs/MonacoHighlightedCode.jsx:40-58
Calls both useMonacoDevtoolboxTheme() and applyDevtoolboxTheme inside creation effect. Remove one; hook already handles updates.

6. self.MonacoEnvironment + window.__monaco leaks / env assumption frontend/src/components/inputs/monaco/monacoSetup.js:12-22

• self is not defined in SSR/jsdom unit tests (ReferenceError). Use globalThis.
• Exposing window.__monaco unconditionally helps e2e (e2e/helpers/editor.js:45) but leaks monaco API to production. Guard with import.meta.env.DEV or delete after e2e.
• getWorker is assigned after monaco import but before any worker creation — OK today, but if monaco eager-spawns worker it races.

7. placeholder is not a Monaco option frontend/src/components/inputs/MonacoCodeEditor.jsx:63
monaco.editor.IStandaloneEditorConstructionOptions has no placeholder. Passing it is silently ignored; overlay placeholder never shows (you hide it with ready flag anyway). Either implement custom overlay or drop the option spread.

8. External value sync resets undo/scroll frontend/src/components/inputs/MonacoCodeEditor.jsx:90 and MonacoHighlightedCode.jsx:87
model.setValue(value) replaces entire model, wiping undo stack and resetting cursor/scroll. For CodeFormatter output this is fine, but for editable input prefer editor.executeEdits or preserve selection. At least document.

---

🟡 Code quality / maintainability

• Duplicated Suspense fallback ToolEditorPane.jsx:60-127 — same HighlightedCode/CodeEditor rendered twice (fallback vs non-monaco). Extract const FallbackReadonly = <HighlightedCode …/> etc. Also error-border style={{border:…}} on pane wrapper conflicts with Monaco container border:1px solid var(--border) — error not visible for Monaco.
• Un-dismissed lint rule MonacoCodeEditor.jsx:43-46 useEffect(()=>{onChangeRef.current=onChange;valueRef.current=value;}); missing deps array intentionally. Add // eslint-disable-next-line comment or switch to useEffect(..., [onChange, value]).
• Hard-coded BASE_OPTIONS MonacoCodeEditor.jsx:6-20 typo? scrollBeyondLastLine:false, overviewRulerLanes:0 good, but folding:false + glyphMargin:false wastes vertical space? Keep or comment.
• No error handling for getMonaco() rejection — if chunk load fails, editor stays forever on Loading editor... (MonacoCodeEditor.jsx:158-174). Add .catch(()=>setLoadError(true)) and render textarea fallback like CodeEditor does.
• languages.js:1-16 incomplete — pilot covers json/xml/html/css but mapping misses xml→xml (present), good. Future tools will need yaml, sql, etc. Consider reusing @codemirror/lang-* loader map or adding javascript/typescript explicitly.
• File naming inconsistency MonacoCodeEditor.jsx default export vs ToolEditorPane.jsx imports via React.lazy(()=>import('./MonacoCodeEditor')) — OK but MonacoHighlightedCode.jsx also default export; prefer named exports for consistency with CodeEditor.

🔒 Security / unsafe patterns

• No injection risk — values go via editor.setValue; Monaco sanitizes.
• Low: window.__monaco gives page JS full editor API — not exploitable per se, but any XSS could drive editor. Gate behind if (import.meta.env.DEV).
• Worker imports via ?worker — Vite native, no CDN, preserves offline-first — good. Ensure vite.config.js docs mention worker support (no plugin needed in Vite 6+).

🧪 Tests — gaps

• Zero unit tests for new code — bun run test 48 tests still pass, but none cover normalizeColor, applyDevtoolboxTheme, MONACO_LANGUAGE_IDS, useMonacoTheme, or MonacoCodeEditor/HighlightedCode lifecycle. Add table-driven tests:

  // monacoSetup.test.js
  expect(normalizeColor('var(--background)')).toMatch(/^#/);
  expect(normalizeColor('#282c34')).toBe('#282c34');
  expect(normalizeColor('notacolor')).toBeNull();
  expect(normalizeColor('rgba(255 0 0 / 0.5)')).toBe('#ff000080');

  And mock canvas.getContext/ getComputedStyle edge cases.

• E2e helper flakiness frontend/e2e/helpers/editor.js:39,42-52

  • await page.waitForTimeout(1000) hard sleep slows suite 12 specs ×1s ×4 runs ≈48s. Replace with pure poll.
  • window.__monaco.editor.getModels().some(m=>m.getValue()===v || m.getValue().endsWith('\n'+v)) matches any model (input vs output) and endsWith hides leftover content bugs. Scope to the container: stash model id on container editor.getModel().id via evaluate and poll that id only.
  • readEditorText trimEnd() per line loses intentional trailing spaces (edge for formatter). Consider replace(/\u00a0/g,' ') only.

• New spec codeFormatter.spec.js:66-71 invalid JSON shows error tight timeout 2000ms may flake on slower CI with Monaco debounce 300ms + Monaco init poll 15000ms. Keep but make helper deterministic (wait for output poll instead of getByText).

---

💡 Concrete improvements (copy-paste ready)

1. Fix hex logic monacoSetup.js:71

   if (/^#[0-9a-f]{3,8}$/i.test(c)) {
     if (c.length===4) return '#' + [...c.slice(1)].map(ch=>ch+ch).join('');
     if (c.length===5) return '#' + [...c.slice(1)].map(ch=>ch+ch).join(''); // #RGBA -> #RRGGBBAA
     if (c.length===9) return c; // #RRGGBBAA
     if (c.length===7) return c; // #RRGGBB — was wrongly rejected
     if (c.length===3+1) return c; // already handled
   }

2. Forward showLineNumbers ToolEditorPane.jsx:117

   <MonacoCodeEditor showLineNumbers={showLineNumbers} … />
   <CodeEditor showLineNumbers={showLineNumbers} … />

3. Harden monacoSetup.js

   const g = globalThis;
   g.MonacoEnvironment = { …
   if (import.meta.env.DEV) window.__monaco = monaco;

4. Theme hook fix useMonacoTheme.js

   useEffect(()=>{
     let cancel=false;
     getMonaco().then(m=>{ if(cancel) return; monacoRef=m; applyDevtoolboxTheme(m,{actualType,colors:palette.colors,tokenColors:palette.tokenColors});});
     return ()=>{cancel=true};
   },[actualType,palette]);

   Remove the duplicate [] effect.
5. Add rejection handling MonacoCodeEditor.jsx:50

   getMonaco().then(...).catch(()=> setReady(false) /* + error state */)

Overall: solid pilot, correctly code-split and theme-aware. Fix the hex rejection (would break 1st custom-theme QA) and showLineNumbers forwarding before merging; rest can be follow-ups.

opencode session  |  github run

[github-actions]

PR Review — #163 feat/monaco-pilot-code-formatter

Verdict: Comment — fix placeholder / window.__monaco / load-error + hex validation before merge. Isolation strategy is correct (impl="monaco", lazy chunk, ?worker offline). 21b15e9 correctly fixed the main normalizeColor + theme race + showLineNumbers issues.

1) Bugs / Logic — still open (after 21b15e9 fixes)

✅ Fixed in 21b15e9 — no longer blocking

• frontend/src/components/inputs/monaco/monacoSetup.js:71 hex reject #RRGGBB — now passes #282c34 via hex.length===3/4 expand else return '#'+hex.
• frontend/src/components/inputs/ToolEditorPane.jsx:105 missing showLineNumbers on editable Monaco — now 117: showLineNumbers={showLineNumbers}.
• frontend/src/components/inputs/MonacoCodeEditor.jsx:78 / MonacoHighlightedCode.jsx:69 double model.dispose() — now guarded if(!model.isDisposed()).
• frontend/src/components/inputs/monaco/useMonacoTheme.js:5 stale race — now themeRef.current + two effects (mount + re-apply on [actualType,palette]).

🔴 Remaining

• Invalid placeholder option frontend/src/components/inputs/MonacoCodeEditor.jsx:63 + 95-103 — IStandaloneEditorConstructionOptions has no placeholder. Spread is no-op. Placeholder never shows; !ready overlay at 158 is the only visible placeholder. Drop the spread or implement overlay element.

• Incomplete hex validation frontend/src/components/inputs/monaco/monacoSetup.js:71-77 — regex #[0-9a-f]{3,8} allows hex 5/7 (#12345 / #1234567) which are never valid. Current code expands only 3/4 and passes rest through, so normalizeColor('#12345') → #12345 (5 hex) reaches applyDevtoolboxTheme as foreground: '12345' which Monaco rejects. Should return null for hex.length===5||7:

  if (hex.length===3||hex.length===4) return '#'+[...hex].map(c=>c+c).join('');
  if (hex.length===6||hex.length===8) return '#'+hex;
  return null;

• No getMonaco() rejection handling frontend/src/components/inputs/MonacoCodeEditor.jsx:50 / MonacoHighlightedCode.jsx:50 — getMonaco().then(...) has no .catch. Chunk load failure → forever Loading editor... (158-174). CodeEditor.jsx:83-128 has loadError → textarea fallback; Monaco paths need setLoadError + fallback.

• Error border invisible for Monaco frontend/src/components/inputs/ToolEditorPane.jsx:56 style={{border: error?'1px solid #ef4444':undefined}} on wrapper, but inner MonacoCodeEditor.jsx:126 / MonacoHighlightedCode.jsx:114 render border:1px solid var(--border) + backgroundColor:var(--background) — wrapper border is clipped (overflow:hidden + borderRadius). Either pass error into Monaco container style or use outline.

• useEffect without deps frontend/src/components/inputs/MonacoCodeEditor.jsx:43 and MonacoHighlightedCode.jsx:43 — syncs refs every render intentionally but no // eslint-disable-next-line; noisy lint and will regress if react-hooks/exhaustive-deps is enforced.

2) Security / Unsafe

• frontend/src/components/inputs/monaco/monacoSetup.js:12 self.MonacoEnvironment — self is undefined in SSR/jsdom (throws ReferenceError). Use globalThis. Low runtime impact (only browser bundle) but breaks Vitest if getMonaco() ever invoked there.
• frontend/src/components/inputs/monaco/monacoSetup.js:22 window.__monaco = monaco unconditionally — exposes full monaco.editor API to page JS. Harmless if no XSS, but widens post-XSS blast radius. Gate behind import.meta.env.DEV or import.meta.env.MODE !== 'production'; e2e can inject via page.evaluate instead. frontend/e2e/helpers/editor.js:46 depends on it — consider if(import.meta.env.DEV) window.__monaco=monaco and update helper to stash model id on container (editor.getModel().id).

3) Code Quality / Maintainability

• Duplicated Suspense fallback frontend/src/components/inputs/ToolEditorPane.jsx:60-81 vs 58-91 — same HighlightedCode props rendered twice. Extract fallbackReadonly / fallbackEditable constants. Editable fallback CodeEditor at 106-114 omits showLineNumbers prop even though Monaco branch forwards it.
• BASE_OPTIONS / READ_ONLY_OPTIONS MonacoCodeEditor.jsx:6 + MonacoHighlightedCode.jsx:7 duplicate minimap/scrollBeyondLastLine/overviewRulerLanes — extract COMMON_MONACO_OPTIONS. Comment why folding:false,glyphMargin:false.
• External value sync wipes undo MonacoCodeEditor.jsx:90 model.setValue(value) + MonacoHighlightedCode.jsx:81 — resets undo stack/cursor. Acceptable for formatter output, but for input should document or use executeEdits + preserve selection.
• Language map stale vs CodeMirror map frontend/src/components/inputs/monaco/languages.js:1 vs frontend/src/components/inputs/CodeEditor.jsx:8 — Monaco map includes go/python/markdown not in CM map; CM includes legacy swift path. Keep single source of truth or add comment that Monaco pilot only needs json/xml/html/css but sql/java already mapped.
• **debounce in frontend/src/pages/CodeFormatter/index.jsx:32 inside useCallback([],[]) — new debounce closure never cleaned on unmount, holds stale setError/setOutput if unmounted mid-format. Consider useMemo + cleanup clearTimeout.

4) Tests — gaps

• Monaco unit coverage still thin frontend/src/components/inputs/monaco/monacoSetup.test.js:1 added 9 cases (good direction) but missing: rgba alpha hex (rgba(255 0 0 / 0.5) → #ff000080), var(--missing) → null, getColorCtx null path, and applyDevtoolboxTheme token skip on invalid color. No tests for MonacoCodeEditor/MonacoHighlightedCode lifecycle (mount/dispose, lineNumbers toggle, setModelLanguage branch 113-115).
• E2E flakiness frontend/e2e/helpers/editor.js:39 hard waitForTimeout(1000) per fillEditor — 12 specs ×1s ≈ slow suite; replace with expect.poll on model value only (already at 42-52). page.evaluate((v)=>getModels().some(m=>m.getValue()===v||endsWith)) at 46-48 matches any model and masks leftover \n — scope to container's model id: set container.dataset.modelId=editor.getModel().id on creation and poll that id.
• readEditorText trims trailing spaces frontend/e2e/helpers/editor.js:74 trimEnd() per Monaco line loses intentional whitespace (formatter edge). Needed for \u00a0 rendering, but note caveat.

5) Concrete follow-ups

// monacoSetup.js — fix hex + globalThis + dev-only expose
const g = globalThis;
g.MonacoEnvironment = { getWorker(_id,label){...} };
if (import.meta.env.DEV) window.__monaco = monaco;

// normalizeColor — reject lengths 5/7
if (/^#[0-9a-f]{3,8}$/i.test(c)) {
  const hex=c.slice(1);
  if (hex.length===3||hex.length===4) return '#'+[...hex].map(ch=>ch+ch).join('');
  if (hex.length===6||hex.length===8) return '#'+hex;
  return null;
}

// MonacoCodeEditor.jsx — drop invalid placeholder, add error handling
const [loadError,setLoadError]=useState(false);
getMonaco().then(monaco=>{...}).catch(()=>setLoadError(true));
// render textarea fallback if loadError (mirror CodeEditor.jsx:183)

// ToolEditorPane.jsx — error visible + DRY fallback
const fallbackRO=<HighlightedCode ... showLineNumbers .../>;
const fallbackEdit=<CodeEditor ... showLineNumbers={showLineNumbers} .../>;
<div style={{outline: error?'1px solid #ef4444':undefined, outlineOffset:-1}}>

// helpers/editor.js — scope model id instead of any model
await page.evaluate((id,v)=> window.__monaco.editor.getModel(id)?.getValue()===v, modelId, value)

No blocking security vuln. Address the four 🔴 items, otherwise ready to pilot-merge and widen migration after next theme QA.

opencode session  |  github run

[github-actions]

PR Review — #163 feat/monaco-pilot-code-formatter

Overall: Correct isolation (impl="monaco" + React.lazy + ?worker offline). Pilot limited to Code Formatter is the right call before wider migration. Findings below are ordered by severity — 2 blocking-ish bugs remain from the 21b15e9 follow-up.

---

🔴 Bugs / Logic / Edge cases

1. normalizeColor still accepts invalid 5/7 hex frontend/src/components/inputs/monaco/monacoSetup.js:71-78

if (/^#[0-9a-f]{3,8}$/i.test(c)) {
  const hex = c.slice(1);
  if (hex.length===3||hex.length===4) return '#'+[...hex].map(c=>c+c).join('');
  return '#'+hex; // ← #12345 (5) and #1234567 (7) fall through as '#12345'
}

Regex allows 3..8, only 3/4 are expanded — 5 (#RGBA short? no) and 7 (#RRGGBB+1) are invalid and must be null. Monaco rejects foreground:"12345" and the fallback pick() hides it. Fix:

if (hex.length===3||hex.length===4) return '#'+[...hex].map(ch=>ch+ch).join('');
if (hex.length===6||hex.length===8) return '#'+hex;
return null;

Add assertion: expect(normalizeColor('#12345')).toBeNull().

2. placeholder is not a valid Monaco option frontend/src/components/inputs/MonacoCodeEditor.jsx:63,95-103
IStandaloneEditorConstructionOptions has no placeholder; spread is a no-op. The only visible placeholder is the !ready overlay MonacoCodeEditor.jsx:158-174. Either drop the spread or implement a real overlay. Same stale spread in updateOptions effect triggers unnecessary updateOptions calls.

3. No getMonaco() rejection path → infinite Loading editor... frontend/src/components/inputs/MonacoCodeEditor.jsx:50-71 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:49-71
CodeEditor.jsx:83-128 has loadError => textarea fallback. Monaco path has no .catch, no loadError state. Chunk failure (network/CSP) hangs forever. Add:

const [loadError,setLoadError]=useState(false);
getMonaco().then(monaco=>{...}).catch(()=>setLoadError(true));
// render same textarea fallback as CodeEditor when loadError

4. Error border invisible for Monaco frontend/src/components/inputs/ToolEditorPane.jsx:56 vs MonacoCodeEditor.jsx:126 / MonacoHighlightedCode.jsx:113
Wrapper sets style={{border: error?'1px solid #ef4444':undefined}} but inner Monaco container renders its own border:1px solid var(--border) + overflow:hidden + borderRadius. Wrapper border is clipped. Pass error into Monaco style.borderColor or use outline:1px solid #ef4444; outlineOffset:-1px.

5. self.MonacoEnvironment assumes self frontend/src/components/inputs/monaco/monacoSetup.js:12
ReferenceError: self is not defined in SSR/jsdom/ Vitest (vitest.config.js:8 environment:jsdom has window but not always self). Use globalThis.MonacoEnvironment. Low runtime impact (browser bundle) but breaks if getMonaco() is ever imported in a unit test.

6. useEffect without deps — intentional but noisy frontend/src/components/inputs/MonacoCodeEditor.jsx:43-46 / MonacoHighlightedCode.jsx:42-44

useEffect(()=>{onChangeRef.current=onChange; valueRef.current=value;});

Silences exhaustive-deps. Add // eslint-disable-next-line or explicit [onChange,value] — otherwise future lint enforcement will flag.

7. value sync wipes undo/scroll frontend/src/components/inputs/MonacoCodeEditor.jsx:89-90 / MonacoHighlightedCode.jsx:80
model.setValue(value) replaces entire model, resets undo stack/cursor/selection. Fine for formatter output, but for editable input document the trade-off or use editor.executeEdits + preserve selection. At least add comment.

---

🔒 Security / Unsafe patterns

• frontend/src/components/inputs/monaco/monacoSetup.js:22 window.__monaco = monaco unconditionally — exposes full editor API to page JS. Not exploitable alone, but widens post-XSS blast radius. Gate behind import.meta.env.DEV (frontend/e2e/helpers/editor.js:46 depends on it — update helper to stash modelId via evaluate instead, or guard with if(import.meta.env.DEV)).
• Workers via Vite ?worker — offline-first preserved, no CDN. Correct for Vite 6+. No plugin config needed in frontend/vite.config.js:9 — note in comment.
• No injection via editor.setValue — Monaco sanitizes.

---

🟡 Code Quality / Maintainability

• Duplicated Suspense fallbacks frontend/src/components/inputs/ToolEditorPane.jsx:60-81,104-114 renders HighlightedCode/CodeEditor twice (fallback vs non-monaco). Extract const FallbackRO = <HighlightedCode .../>. Also editable fallback ToolEditorPane.jsx:106-114 omits showLineNumbers prop even though Monaco branch forwards it — inconsistent.
• Duplicated BASE_OPTIONS / READ_ONLY_OPTIONS MonacoCodeEditor.jsx:6-20 / MonacoHighlightedCode.jsx:7-23 (minimap, scrollBeyondLastLine, overviewRulerLanes, fontFamily). Extract COMMON_MONACO_OPTIONS. Comment why folding:false,glyphMargin:false (saves height?).
• Theme hook redundancy frontend/src/components/inputs/monaco/useMonacoTheme.js:11-31 keeps two effects (mount via themeRef + re-apply on [actualType,palette]). Works, but can be single useEffect([actualType,palette]) with cancelled flag. Second effect currently lacks cancellation — stale apply may fire after unmount.
• Stale debounce closure frontend/src/pages/CodeFormatter/index.jsx:32-42,202-235 debounce inside useCallback([],[]) creates a closure capturing setOutput/setError forever, never cleared on unmount. Prefer useMemo + return ()=>clearTimeout(timeout) or useRef for timeout.
• Language map diverges from CodeMirror frontend/src/components/inputs/monaco/languages.js:1-16 includes go/python/markdown/shell not in CodeEditor.jsx:8-22; CM includes swift legacy path. Keep single source of truth or document pilot scope (json/xml/html/css only).

---

🧪 Tests — Missing / Weak

• Unit coverage added but thin frontend/src/components/inputs/monaco/monacoSetup.test.js:1-49 9 cases cover normalizeColor hex/expand/var, but missing: rgba(255 0 0 / 0.5) → #ff000080, hsla/named colors, var(--missing) → null, getColorCtx() → false path, applyDevtoolboxTheme skipping invalid token (color:null) and try/catch on defineTheme. No tests for useMonacoTheme or editor lifecycle (mount/dispose, lineNumbers toggle, setModelLanguage branch MonacoCodeEditor.jsx:113-115).
• E2E helpers — deterministic but slow/broad frontend/e2e/helpers/editor.js:39-52
  • Hard waitForTimeout(1000) per fillEditor inflates suite (12 specs ×1s). Replace with pure expect.poll.
  • window.__monaco.editor.getModels().some(m=>m.getValue()===v || endsWith) matches any model (input vs output) and endsWith('\n'+v) masks leftover content. Scope to container: on create set container.dataset.modelId=editor.getModel().id and poll that id.
  • readEditorText trimEnd() per line (:74) drops intentional trailing spaces (formatter edge) — intentionally for \u00a0 but note caveat.
• Timeout tightness frontend/e2e/codeFormatter.spec.js:66-71 expect(.../invalid JSON/).toBeVisible({timeout:2000}) may flake with Monaco init (expect.poll timeout 15000) + debounce 300ms. Keep but poll output/error state instead of raw getByText.

---

💡 Concrete follow-ups (copy-paste)

// monacoSetup.js — fix hex + globalThis + dev guard
const g = globalThis;
g.MonacoEnvironment = { getWorker(_id,label){ ... } };
if (typeof window!=='undefined' && import.meta.env.DEV) window.__monaco = monaco;

// normalizeColor
if (/^#[0-9a-f]{3,8}$/i.test(c)) {
  const hex=c.slice(1);
  if (hex.length===3||hex.length===4) return '#'+[...hex].map(ch=>ch+ch).join('');
  if (hex.length===6||hex.length===8) return '#'+hex;
  return null;
}

// MonacoCodeEditor.jsx — drop placeholder option, add error fallback
const [loadError,setLoadError]=useState(false);
getMonaco().then(monaco=>{...}).catch(()=>setLoadError(true));
if (loadError) return <textarea value={value} onChange={e=>onChange?.(e.target.value)} .../>

Address 1-4 before merge; rest can be follow-ups on the next theme QA.

opencode session  |  github run

[github-actions]

Review — #163 feat/monaco-pilot-code-formatter @ 3258f1a

Verdict: Approvable pilot with minor follow-ups. 3258f1a fixed the prior blocking issues (hex rejection, double model.dispose(), showLineNumbers forwarding, self→globalThis, infinite Loading… fallback). Isolation strategy is correct (impl="monaco" + React.lazy + ?worker offline, no CDN). No blocking security vuln.

Fixes confirmed in 3258f1a:

• frontend/src/components/inputs/monaco/monacoSetup.js:75-84 hex 5/7 now return null — monacoSetup.test.js:34 covers it. Was #282c34→null fallback.
• frontend/src/components/inputs/MonacoCodeEditor.jsx:86 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:74 guarded if(!model.isDisposed()) before dispose.
• frontend/src/components/inputs/ToolEditorPane.jsx:123 now forwards showLineNumbers to editable Monaco.
• frontend/src/components/inputs/monaco/monacoSetup.js:12 globalThis.MonacoEnvironment; frontend/src/components/inputs/monaco/monacoSetup.js:25 reduced window.__monaco leak to globalThis.__monacoModels: () => models.map(m=>m.getValue()).
• frontend/src/components/inputs/MonacoCodeEditor.jsx:77,186 / MonacoHighlightedCode.jsx:67,160 added loadError → textarea/pre fallback + catch(()=>setLoadError).

---

🔴 Remaining bugs / logic — non-blocking but fix pre-widen

1. Invalid placeholder Monaco option still spread frontend/src/components/inputs/MonacoCodeEditor.jsx:68,114
IStandaloneEditorConstructionOptions has no placeholder — spread is no-op and forces an extra updateOptions cycle. Remove or implement overlay. The visible placeholder is already !ready && !loadError at frontend/src/components/inputs/MonacoCodeEditor.jsx:169:

// drop: ...(placeholder ? { placeholder } : {})
ariaLabel: ariaLabel || label || placeholder || 'Code editor',

2. Editable Monaco missing error prop + double border frontend/src/components/inputs/ToolEditorPane.jsx:53-57 vs frontend/src/components/inputs/MonacoCodeEditor.jsx:137 vs frontend/src/components/inputs/MonacoHighlightedCode.jsx:121
ToolEditorPane.jsx:56 sets style={{border: error?'1px solid #ef4444':undefined}} on wrapper. Both Monaco panes now also render border: error ? '1px solid #ef4444' : … internally, yielding a double border (wrapper + inner) and the wrapper’s overflow:hidden previously clipped it. Forward error to the editable path and let panes own the border, or remove wrapper border:

// ToolEditorPane.jsx:118
<MonacoCodeEditor error={error} … />
// and drop style.border on the wrapper div:53, or keep only one.

3. Suspense fallbacks inconsistent frontend/src/components/inputs/ToolEditorPane.jsx:60-81 vs 105-127
Read-only fallback forwards showLineNumbers; editable fallback CodeEditor at 107-115 omits showLineNumbers (and error/readOnly). Extract:

const fallbackRO = <HighlightedCode … showLineNumbers … />;
const fallbackEdit = <CodeEditor … showLineNumbers={showLineNumbers} … />;

4. useMonacoDevtoolboxTheme second effect has no cancellation frontend/src/components/inputs/monaco/useMonacoTheme.js:23-31
First effect at 11-20 correctly cancels via cancelled. Second at 23 does getMonaco().then(apply…) without guard — palette change during unmount can call defineTheme on disposed instance. Add same cancelled flag or collapse to single useEffect([actualType,palette]) using themeRef.

5. value sync resets undo/cursor frontend/src/components/inputs/MonacoCodeEditor.jsx:97-103 / MonacoHighlightedCode.jsx:84-90
model.setValue wipes undo stack/scroll — documented at MonacoCodeEditor.jsx:94 now (good), but still surprising for future editable tools. Follow-up: use executeEdits + preserve selection, or at least keep comment on both files.

🟡 Code quality / maintainability

• Duplicated BASE_OPTIONS / READ_ONLY_OPTIONS frontend/src/components/inputs/MonacoCodeEditor.jsx:6 / MonacoHighlightedCode.jsx:7 (minimap, scrollBeyondLastLine, overviewRulerLanes, fontFamily). Extract COMMON_MONACO_OPTIONS; comment why folding:false,glyphMargin:false,renderLineHighlight:'none'.
• Debounce closure never cleared frontend/src/pages/CodeFormatter/index.jsx:32-42,202-235 debounce inside useCallback([],[]) captures setOutput/setError forever, leaks timeout on unmount. Prefer useMemo + cleanup or useRef timeout.
• Language map drift frontend/src/components/inputs/monaco/languages.js:1 vs frontend/src/components/inputs/CodeEditor.jsx:8 — Monaco adds go/python/markdown/shell not in CM, CM has swift legacy. Fine for pilot (json/xml/html/css) but add comment or single source of truth before wider migration.
• Stray {' '} frontend/src/components/inputs/ToolEditorPane.jsx:127 artifact from formatting.

🔒 Security

• frontend/src/components/inputs/monaco/monacoSetup.js:25 still exposes globalThis.__monacoModels unconditionally. Safer than full window.__monaco but still leaks model contents to any page script. Low risk (offline app), but consider if(import.meta.env.DEV) globalThis.__monacoModels = … — e2e runs vite preview in mode: production, so gate on an e2e-specific flag or delete after helper reads it.
• getMonaco() workers via ?worker — preserves offline-first, no CDN. No vite.config.js change needed (Vite 6 handles ?worker). Good.
• editor.setValue sanitizes — no injection.

🧪 Tests — improved but thin

• frontend/src/components/inputs/monaco/monacoSetup.test.js:1-40 now 9 cases + 3/4 expand, 5/7 reject (good). Still missing: rgba(255 0 0 / 0.5) → #ff000080, named/hsla paths via canvas, var(--missing)→null, getColorCtx()==false branch, applyDevtoolboxTheme skipping invalid token and try/catch around defineTheme.
• No unit coverage for MonacoCodeEditor/MonacoHighlightedCode lifecycle (mount/dispose, lineNumbers toggle, setModelLanguage branch MonacoCodeEditor.jsx:120-126).
• E2E helper frontend/e2e/helpers/editor.js:39,45 — hard waitForTimeout(1000) inflates suite (~12 specs×1s) and __monacoModels().some(m===v||endsWith('\n'+v)) matches any model (input vs output) and masks leftover content. Scope to container’s model id (stash editor.getModel().id on creation) and remove sleep, rely on existing expect.poll 42-49. readEditorText:71 trimEnd() per line drops intentional trailing whitespace — acceptable for formatter but note caveat.

---

Concrete one-liners to copy:

// monacoSetup.js — already fixed, keep
// MonacoCodeEditor.jsx:68 + 114 — remove placeholder spread
// ToolEditorPane.jsx:118
<MonacoCodeEditor error={error} showLineNumbers={showLineNumbers} … />
// ToolEditorPane.jsx:56 — let panes own border or use outline
// e2e/helpers/editor.js — replace 1000ms sleep + any-model poll with model-id scoped poll

Overall: solid pilot, correctly code-split, theme-aware, and now degrades gracefully. Address 1-2 before merge or in a quick follow-up before widening to other tools.

opencode session  |  github run

[github-actions]

Review — #163 feat/monaco-pilot-code-formatter @ 6152a22

Verdict: Approve pilot with non-blocking follow-ups. Isolation is correct (impl="monaco" prop frontend/src/components/inputs/ToolEditorPane.jsx:31, React.lazy + Vite ?worker frontend/src/components/inputs/monaco/monacoSetup.js:15-18 keeps offline-first, no CDN). Previous blocking bugs are fixed in 21b15e9/3258f1a/6152a22: hex 5/7 rejection, double model.dispose() guard MonacoCodeEditor.jsx:86, showLineNumbers forwarding ToolEditorPane.jsx:123, globalThis monacoSetup.js:20, loadError fallback MonacoCodeEditor.jsx:77,186.

---

🔴 Bugs / Logic — remaining (pre-widen)

1. placeholder is invalid Monaco option — noisy no-op frontend/src/components/inputs/MonacoCodeEditor.jsx:68,114

...(placeholder ? { placeholder } : {}),

IStandaloneEditorConstructionOptions has no placeholder. Spread silently ignored; visible placeholder is already !ready && !loadError overlay MonacoCodeEditor.jsx:169-184. Drops an unnecessary updateOptions cycle. Fix: delete both spreads, keep ariaLabel: ariaLabel || label || placeholder || 'Code editor' MonacoCodeEditor.jsx:67.

2. Double error border — wrapper + inner frontend/src/components/inputs/ToolEditorPane.jsx:56 vs frontend/src/components/inputs/MonacoCodeEditor.jsx:137 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:121
Wrapper sets style={{border: error?'1px solid #ef4444':undefined}} and Monaco panes also render border: error?'1px solid #ef4444':'1px solid var(--border)'. Result: double border (wrapper overflow:hidden previously clipped it). Pick one owner: forward error to editable pane and let Monaco pane own its border, remove wrapper border for impl==="monaco":

// ToolEditorPane.jsx:118
<MonacoCodeEditor error={error} showLineNumbers={showLineNumbers} ... />
// and drop style.border on wrapper when monaco, or switch wrapper to outline

3. Suspense fallback diverges from Monaco branch frontend/src/components/inputs/ToolEditorPane.jsx:105-127
Fallback CodeEditor at 107-115 omits showLineNumbers (and error/readOnly/style) even though Monaco branch 118-127 forwards showLineNumbers. Causes layout shift while chunk loads. Extract constants:

const fallbackRO = <HighlightedCode code={value} language={language} copyable={false} showLineNumbers={showLineNumbers} .../>;
const fallbackEdit = <CodeEditor value={value} language={language} showLineNumbers={showLineNumbers} placeholder={...} .../>;

Also stray {' '} ToolEditorPane.jsx:127 — formatting artifact.

4. useMonacoDevtoolboxTheme second effect leaks on unmount frontend/src/components/inputs/monaco/useMonacoTheme.js:23-31
First effect 11-20 correctly guards cancelled; second has no guard — palette change during unmount can call defineTheme on disposed instance. Add same flag or collapse to single useEffect([actualType,palette]) via themeRef.

5. value sync wipes undo/cursor — documented but still surprising frontend/src/components/inputs/MonacoCodeEditor.jsx:94-103 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:84-90
model.setValue is correct for formatter output, note is present in CodeEditor 94-96, add same comment in HighlightedCode for future editable reuse.

🟡 Code quality / consistency

• Duplicated options MonacoCodeEditor.jsx:6-20 vs MonacoHighlightedCode.jsx:7-23 (minimap, scrollBeyondLastLine, overviewRulerLanes, fontFamily, padding). Extract COMMON_MONACO_OPTIONS. Comment why folding:false,glyphMargin:false,renderLineHighlight:'none'.
• useEffect without deps is intentional MonacoCodeEditor.jsx:46-50 — now has eslint-disable comment (good); duplicate pattern MonacoHighlightedCode.jsx:45-48 same — keep consistent.
• debounce inside useCallback([],[]) never clears on unmount frontend/src/pages/CodeFormatter/index.jsx:32-42,202-235 — holds setOutput/setError closure, leaks timeout if unmounted mid-format. Prefer useRef for timer + cleanup or useMemo + return()=>clearTimeout.
• Language registration now correct monacoSetup.js:8-13 added json/css/html contributions + xml/swift Monarch — fixes 6152a22 per-language tokenization. languages.js:1-16 drift vs CodeEditor.jsx:8-22 (go/python/markdown/shell in Monaco only, swift legacy in CM) is acceptable for pilot but add comment that pilot scope is json/xml/html/css before wider migration.
• Layout fix frontend/src/components/layout/ToolLayout.jsx:75-76 display:flex; flexDirection:column ensures Monaco height:100% panes fill correctly — verified, keep.

🔒 Security / unsafe patterns

• No injection via editor.setValue — safe.
• globalThis.__monacoModels monacoSetup.js:33 is minimal read-only snapshot (good reduction from window.__monaco), but still exposed unconditionally to page JS. Gate if (import.meta.env.DEV) globalThis.__monacoModels = … — e2e runs vite preview in production mode so either set VITE_E2E=1 flag or have helper inject via page.evaluate.
• globalThis.MonacoEnvironment monacoSetup.js:20 correct — avoids self is not defined in jsdom vitest.config.js:8.

🧪 Tests

• Improved: frontend/src/components/inputs/monaco/monacoSetup.test.js:1-51 now covers hex 6/8 pass, 3/4 expand 16-20, 5/7 reject 34-35, var(--x) 22-27. Good.
• Still thin: missing rgba(255 0 0 / 0.5)->#ff000080, named/hsla via canvas, getColorCtx()==false branch, applyDevtoolboxTheme skipping invalid token monacoSetup.js:129-131 and try/catch 161-163. No lifecycle tests for lineNumbers toggle MonacoCodeEditor.jsx:112 / setModelLanguage 125.
• E2E helper frontend/e2e/helpers/editor.js:39,45 hard waitForTimeout(1000) per fillEditor inflates suite (12 specs ×1s) and __monacoModels().some(m===v||endsWith('\n'+v)) 45 matches any model and masks leftover \n. Scope to container's model id (stash container.dataset.modelId=editor.getModel().id on create) and remove sleep — existing expect.poll 42-49 is sufficient. readEditorText:71 trimEnd() per Monaco line intentionally handles \u00a0 but drops intentional trailing spaces — note caveat, acceptable for formatter.

💡 Concrete patches

// MonacoCodeEditor.jsx:68 + 114 — drop invalid option
// delete ...(placeholder ? { placeholder } : {})

// ToolEditorPane.jsx — forward error, DRY fallbacks, fix wrapper
<div style={{ outline: error?'1px solid #ef4444':undefined, outlineOffset:-1 }}>
const fallbackEdit = <CodeEditor value={value} language={language} showLineNumbers={showLineNumbers} ... />;

// useMonacoTheme.js — add cancellation
useEffect(()=>{ let cancelled=false; getMonaco().then(m=>{ if(cancelled) return; applyDevtoolboxTheme(m,{actualType,colors:palette.colors,tokenColors:palette.tokenColors});}); return()=>{cancelled=true};},[actualType,palette]);

// monacoSetup.js — dev-gate model leak
if (import.meta.env.DEV) globalThis.__monacoModels = () => monaco.editor.getModels().map(m=>m.getValue());

No blocking vuln. Clean up placeholder spread + error border ownership before widening to other tools.

opencode session  |  github run

[github-actions]

Review — #163 feat/monaco-pilot-code-formatter @ a4a7bf3

Verdict: Approve pilot as isolated (impl="monaco"). Prior blocking fixes verified; remaining items are non-blocking pre-widen polish.

Fixes confirmed in 21b15e9 / 3258f1a / 6152a22 / a4a7bf3:

• frontend/src/components/inputs/monaco/monacoSetup.js:96-106 hex 5/7 now return null with tests frontend/src/components/inputs/monaco/monacoSetup.test.js:34
• frontend/src/components/inputs/ToolEditorPane.jsx:123 showLineNumbers forwarded for editable Monaco
• frontend/src/components/inputs/MonacoCodeEditor.jsx:86 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:76 if(!model.isDisposed()) guard
• frontend/src/components/inputs/monaco/monacoSetup.js:32 globalThis.MonacoEnvironment
• frontend/src/components/inputs/MonacoCodeEditor.jsx:77,186 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:67 loadError fallback with .catch

---

1) Bugs / Logic / Edge cases — remaining

1. Invalid placeholder option still spread frontend/src/components/inputs/MonacoCodeEditor.jsx:68 and frontend/src/components/inputs/MonacoCodeEditor.jsx:114
IStandaloneEditorConstructionOptions has no placeholder. Spread is silently ignored; visible placeholder is already !ready && !loadError overlay frontend/src/components/inputs/MonacoCodeEditor.jsx:169. Remove both spreads; keep ariaLabel: ariaLabel || label || placeholder || 'Code editor' at :67.

2. Editable error not forwarded + double border frontend/src/components/inputs/ToolEditorPane.jsx:56 vs frontend/src/components/inputs/MonacoCodeEditor.jsx:137 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:121
Wrapper sets style={{border: error?'1px solid #ef4444':undefined}} and inner panes also render border: error?'1px solid #ef4444':'1px solid var(--border)'. For read-only the double border is intentional (wrapper + inner with overflow:hidden); for editable MonacoCodeEditor at frontend/src/components/inputs/ToolEditorPane.jsx:118 error is never passed. Fix: forward error and let pane own border, or remove wrapper border when impl==="monaco".

// frontend/src/components/inputs/ToolEditorPane.jsx:118
<MonacoCodeEditor error={error} showLineNumbers={showLineNumbers} .../>

3. Suspense fallback diverges frontend/src/components/inputs/ToolEditorPane.jsx:107-115 vs frontend/src/components/inputs/ToolEditorPane.jsx:105-127
Fallback CodeEditor omits showLineNumbers (and error/readOnly/style) while Monaco branch forwards showLineNumbers. Causes layout shift while chunk loads. Extract:

const fallbackRO = <HighlightedCode ... showLineNumbers={showLineNumbers} .../>;
const fallbackEdit = <CodeEditor ... showLineNumbers={showLineNumbers} placeholder={placeholder} .../>;

Also stray {' '} at frontend/src/components/inputs/ToolEditorPane.jsx:127.

4. useMonacoDevtoolboxTheme second effect uncancellable frontend/src/components/inputs/monaco/useMonacoTheme.js:23-31
First effect 11-20 guards cancelled; second has no guard — palette change during unmount can call monaco.editor.defineTheme after dispose. Add same flag or collapse to single useEffect([actualType,palette]) via themeRef.

5. Async setModelLanguage after dispose frontend/src/components/inputs/MonacoCodeEditor.jsx:125 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:108
getMonaco().then(m=>m.editor.setModelLanguage(editor.getModel(), id)) not guarded; if editor unmounted before resolve, editor.getModel() is null. Guard if(!editorRef.current) return.

6. value sync wipes undo/scroll frontend/src/components/inputs/MonacoCodeEditor.jsx:101 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:88
model.setValue is correct for formatter output (documented at MonacoCodeEditor.jsx:94), but resets undo stack/cursor. Keep comment on both files; for future editable tools use executeEdits + preserve selection.

2) Security / Unsafe patterns

• frontend/src/components/inputs/monaco/monacoSetup.js:45 globalThis.__monacoModels exposed unconditionally — minimal snapshot (good reduction from full window.__monaco), but widens attack surface if XSS exists. Gate if(import.meta.env.DEV) — note e2e runs vite preview in production mode, so need VITE_E2E flag or inject via page.evaluate instead. frontend/e2e/helpers/editor.js:45 depends on it.
• frontend/src/components/inputs/monaco/monacoSetup.js:32 globalThis.MonacoEnvironment correct (fixes self is not defined in jsdom); workers via ?worker at :15-18 keep offline-first, no CDN — good, no vite.config.js change needed in Vite 6.
• No injection via editor.setValue; Monaco sanitizes.

3) Code quality / Maintainability

• Duplicated options frontend/src/components/inputs/MonacoCodeEditor.jsx:6 vs frontend/src/components/inputs/MonacoHighlightedCode.jsx:7 (minimap/scrollBeyondLastLine/overviewRulerLanes/fontFamily/padding). Extract COMMON_MONACO_OPTIONS. Comment why folding:false,glyphMargin:false,renderLineHighlight:'none'.
• debounce leaks timeout on unmount frontend/src/pages/CodeFormatter/index.jsx:32,202 debounce inside useCallback([],[]) captures setOutput/setError forever, no clearTimeout on unmount. Use useRef timer + cleanup or useMemo with return ()=>clearTimeout.
• Language map drift frontend/src/components/inputs/monaco/languages.js:1 vs frontend/src/components/inputs/CodeEditor.jsx:8 — Monaco adds go/python/markdown/shell, CM has legacy swift StreamLanguage. Acceptable for pilot (json/xml/html/css) but add comment before wider migration.
• Canvas context assumes document frontend/src/components/inputs/monaco/monacoSetup.js:75 document.createElement('canvas') throws if document undefined (SSR). Guard if(typeof document==='undefined') return false.
• frontend/src/components/layout/ToolLayout.jsx:75 display:flex; flexDirection:column needed for Monaco height:100% — verified, keep.

4) Tests — gaps

• frontend/src/components/inputs/monaco/monacoSetup.test.js:1 now 9 cases (3/4 expand 16, 5/7 reject 34, var(--x) 23) — good. Still missing: rgba(255 0 0 / 0.5)->#ff000080, named/hsla via canvas fillStyle, getColorCtx()==false branch, applyDevtoolboxTheme skipping invalid token monacoSetup.js:142 and try/catch 161, pick() fallback #1f2428/#ffffff.
• No lifecycle tests for MonacoCodeEditor/MonacoHighlightedCode (mount/dispose, lineNumbers toggle 112, setModelLanguage branch).
• frontend/e2e/helpers/editor.js:39,45 hard waitForTimeout(1000) per fillEditor inflates suite (~12 specs×1s) and __monacoModels().some(m===v||endsWith('\n'+v)) matches any model and masks leftover content. Scope to container's model id (stash editor.getModel().id on creation) and remove sleep — existing expect.poll 42-49 is sufficient. readEditorText 71 trimEnd() per Monaco line intentional for \u00a0 but drops intentional trailing spaces — note caveat, acceptable for formatter.

5) Concrete follow-ups

// frontend/src/components/inputs/MonacoCodeEditor.jsx:68 + 114 — remove
// ...(placeholder ? { placeholder } : {})

// frontend/src/components/inputs/ToolEditorPane.jsx:53 — let panes own border
<div style={{ outline: error?'1px solid #ef4444':undefined, outlineOffset:-1 }}>
// frontend/src/components/inputs/monaco/useMonacoTheme.js:23
useEffect(()=>{ let c=false; getMonaco().then(m=>{ if(c) return; applyDevtoolboxTheme(m,{actualType,colors:palette.colors,tokenColors:palette.tokenColors});}); return()=>{c=true};},[actualType,palette]);

// frontend/src/components/inputs/monaco/monacoSetup.js:45 — dev gate
if(import.meta.env.DEV) globalThis.__monacoModels = () => monaco.editor.getModels().map(m=>m.getValue());

// frontend/src/components/inputs/MonacoCodeEditor.jsx:125 — guard async
getMonaco().then(monaco=>{ if(!editorRef.current) return; monaco.editor.setModelLanguage(editor.getModel(), id);});

No blocking vuln. Fix 1-2 before widening migration; rest can be follow-ups.

opencode session  |  github run

[github-actions]

Review — #163 feat/monaco-pilot-code-formatter @ be7a6a7

Verdict: Approve pilot with non-blocking follow-ups before widening. Isolation is correct (impl="monaco" ToolEditorPane.jsx:30, React.lazy ToolEditorPane.jsx:7, local workers via ?worker monacoSetup.js:17 keeps offline-first, no CDN). Blocking issues from earlier rounds are fixed in 21b15e9/3258f1a/6152a22: hex 5/7 rejection, globalThis monacoSetup.js:34, loadError fallback MonacoCodeEditor.jsx:79, showLineNumbers forwarding ToolEditorPane.jsx:125, guarded model.dispose() MonacoCodeEditor.jsx:88.

---

🔴 Bugs / logic — remaining (pre-widen, not merge-blocking)

1. Invalid placeholder option still spread MonacoCodeEditor.jsx:70 and MonacoCodeEditor.jsx:117
IStandaloneEditorConstructionOptions has no placeholder; spread is a silent no-op and triggers an extra updateOptions cycle. Visible placeholder is already the !ready && !loadError overlay MonacoCodeEditor.jsx:172. Remove both spreads; keep ariaLabel: ariaLabel || label || placeholder || 'Code editor' MonacoCodeEditor.jsx:69 for a11y.

// delete
...(placeholder ? { placeholder } : {})

2. Editable error not forwarded — double/invisible border ToolEditorPane.jsx:57 vs MonacoCodeEditor.jsx:140 vs MonacoHighlightedCode.jsx:123
Wrapper sets style={{border: error?'1px solid #ef4444':undefined}} ToolEditorPane.jsx:57. Read-only Monaco correctly receives error={error} ToolEditorPane.jsx:82 and renders its own border MonacoHighlightedCode.jsx:123. Editable MonacoCodeEditor ToolEditorPane.jsx:120 never receives error, so its inner border: error ? … MonacoCodeEditor.jsx:140 is dead and wrapper border is clipped by inner overflow:hidden/borderRadius. Forward it and let panes own the border:

<MonacoCodeEditor error={error} showLineNumbers={showLineNumbers} .../>
// and for impl==="monaco" drop wrapper style.border or switch to outline

3. Suspense fallback diverges ToolEditorPane.jsx:108 vs ToolEditorPane.jsx:120
Fallback CodeEditor ToolEditorPane.jsx:109 omits showLineNumbers/wordWrap/error even though Monaco branch forwards them; causes layout shift while chunk loads. Also stray {' '} ToolEditorPane.jsx:130. Extract:

const fallbackRO = <HighlightedCode code={value} language={language} showLineNumbers={showLineNumbers} .../>;
const fallbackEdit = <CodeEditor value={value} language={language} showLineNumbers={showLineNumbers} wordWrap={wordWrap} placeholder={placeholder} .../>;

4. useMonacoDevtoolboxTheme second effect has no cancellation useMonacoTheme.js:23
First effect useMonacoTheme.js:11 guards cancelled; second does getMonaco().then(apply…) with no guard — palette change during unmount can call defineTheme after dispose. Add flag or collapse to single useEffect([actualType,palette]) via themeRef:

useEffect(()=>{ let c=false; getMonaco().then(m=>{ if(c) return; applyDevtoolboxTheme(m,{actualType,colors:palette.colors,tokenColors:palette.tokenColors});}); return()=>{c=true};},[actualType,palette]);

5. Async setModelLanguage after dispose MonacoCodeEditor.jsx:128 / MonacoHighlightedCode.jsx:110
getMonaco().then(m=>m.editor.setModelLanguage(editor.getModel(), id)) not guarded; if unmounted before resolve, editor.getModel() is null → throws. Guard:

getMonaco().then(monaco=>{ if(!editorRef.current) return; const model=editorRef.current.getModel(); if(model) monaco.editor.setModelLanguage(model,id);});

6. getColorCtx assumes document monacoSetup.js:79
document.createElement('canvas') throws ReferenceError if document is undefined (SSR/edge). Guard if(typeof document==='undefined') return false; at monacoSetup.js:76. Current try/catch does not catch ReferenceError on document identifier lookup in some envs.

---

🔒 Security / unsafe patterns

• monacoSetup.js:46 globalThis.__monacoModels exposed unconditionally — minimal snapshot (good reduction from full window.__monaco), but still leaks all model contents to any page script. Gate if(import.meta.env.DEV) — note e2e runs vite preview in production, so need VITE_E2E=1 flag or inject via page.evaluate instead. Low risk for offline app, but widens post-XSS blast radius. e2e/helpers/editor.js:45 depends on it.
• globalThis.MonacoEnvironment monacoSetup.js:34 correct — fixes self is not defined in jsdom (vitest environment:jsdom). Workers via ?worker preserve offline-first; Vite 6 needs no plugin — good.
• No injection via editor.setValue/model.setValue — Monaco sanitizes MonacoCodeEditor.jsx:103 / MonacoHighlightedCode.jsx:90.

---

🟡 Code quality / maintainability

• Duplicated options MonacoCodeEditor.jsx:6 BASE_OPTIONS vs MonacoHighlightedCode.jsx:7 READ_ONLY_OPTIONS (minimap/scrollBeyondLastLine/overviewRulerLanes/fontFamily/padding). Extract COMMON_MONACO_OPTIONS. Comment why folding:false,glyphMargin:false,renderLineHighlight:'none' MonacoCodeEditor.jsx:14.
• debounce leak CodeFormatter/index.jsx:32 / CodeFormatter/index.jsx:205 debounce inside useCallback([],[]) captures setOutput/setError forever, never cleared on unmount. Prefer useRef timer + cleanup or useMemo with return()=>clearTimeout.
• Language map drift monaco/languages.js:1 vs CodeEditor.jsx:8 — Monaco adds go/python/markdown/shell not in CM; CM keeps swift via StreamLanguage. Acceptable for pilot (json/xml/html/css) but add comment before wider migration.
• value sync resets undo/cursor MonacoCodeEditor.jsx:96 model.setValue documented as acceptable for formatter; same note missing in MonacoHighlightedCode.jsx:85. Future editable tools should use executeEdits + preserve selection.
• Duplicate CSS registration monacoSetup.js:9 monaco/contribution.js for css plus definitions/css/register.js monacoSetup.js:14 — redundant but harmless; keep comment.

---

🧪 Tests — gaps

• Unit monacoSetup.test.js:1 now 9 cases (3/4 expand, 5/7 reject monacoSetup.test.js:34 good). Still missing: rgba(255 0 0 / 0.5) → #ff000080 monacoSetup.js:130, named/hsla via canvas monacoSetup.js:112, var(--missing)→null, getColorCtx()==false branch, applyDevtoolboxTheme skipping invalid token monacoSetup.js:144 and try/catch monacoSetup.js:176, pick() fallback #1f2428/#ffffff monacoSetup.js:154.
• No lifecycle tests for MonacoCodeEditor/MonacoHighlightedCode (mount/dispose, lineNumbers toggle MonacoCodeEditor.jsx:114, setModelLanguage branch MonacoCodeEditor.jsx:127, loadError fallback MonacoCodeEditor.jsx:189).
• E2E e2e/helpers/editor.js:39 hard waitForTimeout(1000) per fillEditor inflates suite (~12 specs×1s). Already polled at e2e/helpers/editor.js:42 → remove sleep. e2e/helpers/editor.js:45 __monacoModels().some(m===v||endsWith('\n'+v)) matches any model and masks leftover \n; scope to container's model id (stash editor.getModel().id on creation). readEditorText e2e/helpers/editor.js:71 trimEnd() per line handles \u00a0 but drops intentional trailing spaces — note caveat, acceptable for formatter.

Concrete patches to copy

// MonacoCodeEditor.jsx — drop invalid option (lines 70 and 117)
// ToolEditorPane.jsx:120 — forward error
<MonacoCodeEditor error={error} showLineNumbers={showLineNumbers} wordWrap={wordWrap} .../>
// useMonacoTheme.js:23 — add cancellation (see above)
// monacoSetup.js:79 — SSR guard
function getColorCtx(){ if(typeof document==='undefined') return false; ... }
// MonacoCodeEditor.jsx:128 — guard async language switch
getMonaco().then(monaco=>{ if(!editorRef.current) return; const m=editorRef.current.getModel(); if(m) monaco.editor.setModelLanguage(m,id);});

No blocking vuln. Fix 1–2 before widening migration; rest can be follow-ups.

opencode session  |  github run

[github-actions]

Review — PR #163 feat/monaco-pilot-code-formatter @ 96b932e

Verdict: Approvable pilot — isolate to impl="monaco" + lazy + ?worker offline-first is correct. Prior blocking issues fixed (#RRGGBB hex, self→globalThis, model.isDisposed() guard, loadError fallback, showLineNumbers forwarding). Remaining items are pre-widen polish, but two are user-visible.

Fixes verified:

• frontend/src/components/inputs/monaco/monacoSetup.js:118-128 hex 5/7 now return null — test at frontend/src/components/inputs/monaco/monacoSetup.test.js:34
• frontend/src/components/inputs/monaco/monacoSetup.js:34 globalThis.MonacoEnvironment
• frontend/src/components/inputs/MonacoCodeEditor.jsx:88 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:78 guarded dispose
• frontend/src/components/inputs/MonacoCodeEditor.jsx:79 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:69 .catch(()=>setLoadError)
• frontend/src/components/inputs/ToolEditorPane.jsx:125 editable showLineNumbers forwarded

---

🔴 Bugs / Logic

1. Invalid placeholder Monaco option still spread frontend/src/components/inputs/MonacoCodeEditor.jsx:70 and frontend/src/components/inputs/MonacoCodeEditor.jsx:117

...(placeholder ? { placeholder } : {})

IStandaloneEditorConstructionOptions has no placeholder. No-op + extra updateOptions cycle. Visible placeholder is already !ready && !loadError overlay frontend/src/components/inputs/MonacoCodeEditor.jsx:172. Delete both spreads; keep ariaLabel: ariaLabel || label || placeholder || 'Code editor' at :69.

2. Editable error not forwarded + double border frontend/src/components/inputs/ToolEditorPane.jsx:57 vs frontend/src/components/inputs/MonacoCodeEditor.jsx:140 vs frontend/src/components/inputs/MonacoHighlightedCode.jsx:123
Wrapper does style={{border: error?'1px solid #ef4444':undefined}}. Read-only correctly forwards error at frontend/src/components/inputs/ToolEditorPane.jsx:82, editable MonacoCodeEditor at :120 never receives it → inner border: error?... at frontend/src/components/inputs/MonacoCodeEditor.jsx:140 is dead and wrapper border is clipped by inner overflow:hidden+borderRadius. Forward error and let panes own border, or remove wrapper style.border when impl==="monaco".

3. MonacoHighlightedCode ignores wordWrap after mount frontend/src/components/inputs/MonacoHighlightedCode.jsx:95-102
ToolEditorPane.jsx:79 passes wordWrap, but sync effect only updates lineNumbers. Toggle via ToolLayout does nothing for read-only output. Add wordWrap: wordWrap?'on':'off' to updateOptions and dep array (mirrors frontend/src/components/inputs/MonacoCodeEditor.jsx:111-120).

4. Async setModelLanguage after unmount frontend/src/components/inputs/MonacoCodeEditor.jsx:128 / frontend/src/components/inputs/MonacoHighlightedCode.jsx:110

getMonaco().then(m=>m.editor.setModelLanguage(editor.getModel(), id))

Not guarded — if unmounted before getMonaco() resolves, editor.getModel() is null → throw. Guard:

getMonaco().then(monaco=>{ if(!editorRef.current) return; const m=editorRef.current.getModel(); if(m) monaco.editor.setModelLanguage(m,id); });

5. useMonacoDevtoolboxTheme second effect leaks frontend/src/components/inputs/monaco/useMonacoTheme.js:23-31
First effect :11-20 guards cancelled; second has no guard — palette change during unmount can defineTheme after dispose. Add let c=false; getMonaco().then(...if(c)return)...; return()=>{c=true} or collapse to single useEffect([actualType,palette]) via themeRef.

6. Suspense fallback diverges frontend/src/components/inputs/ToolEditorPane.jsx:108-118
Fallback CodeEditor omits showLineNumbers, wordWrap, error, readOnly while Monaco branch forwards them → layout shift while chunk loads. Also stray {' '} at :130. Extract:

const fallbackRO = <HighlightedCode ... showLineNumbers={showLineNumbers} ... />;
const fallbackEdit = <CodeEditor ... showLineNumbers={showLineNumbers} wordWrap={wordWrap} placeholder={placeholder} ... />;

🔒 Security

• frontend/src/components/inputs/monaco/monacoSetup.js:49-67 globalThis.__monacoModels/__monacoGetValue/__monacoSetValue exposed unconditionally. Minimal read-only snapshot (good reduction from window.__monaco), but widens post-XSS surface. Gate if(import.meta.env.DEV) globalThis.__monaco... — note e2e runs vite preview in production, so need VITE_E2E=1 flag or keep as-is with comment. frontend/e2e/helpers/editor.js:34,43,74 depends on it.
• frontend/src/components/inputs/monaco/monacoSetup.js:34 globalThis.MonacoEnvironment correct (fixes self is not defined in jsdom vitest.config.js:8).
• No injection via model.setValue / editor.getValue() — Monaco sanitizes.

🟡 Quality / Maintainability

• Duplicated options frontend/src/components/inputs/MonacoCodeEditor.jsx:6 vs frontend/src/components/inputs/MonacoHighlightedCode.jsx:7 (minimap/scrollBeyondLastLine/overviewRulerLanes/fontFamily/padding). Extract COMMON_MONACO_OPTIONS; comment why folding:false,glyphMargin:false,renderLineHighlight:'none'.
• Scope drift — PR description says Code Formatter only, but 96b932e also sets impl="monaco" in frontend/src/pages/CodeConverter/index.jsx:147,161. Update description or keep pilot to one tool.
• debounce leak frontend/src/pages/CodeFormatter/index.jsx:32,206 debounce inside useCallback([],[]) captures setOutput/setError forever, never clearTimeout on unmount. Use useRef timer + cleanup useEffect(()=>()=>clearTimeout).
• getColorCtx assumes document frontend/src/components/inputs/monaco/monacoSetup.js:99 — try does catch ReferenceError, but explicit if(typeof document==='undefined') return false at :96 is clearer for SSR.
• Language map drift frontend/src/components/inputs/monaco/languages.js:1 vs frontend/src/components/inputs/CodeEditor.jsx:8 (Monaco adds go/python/markdown/shell, CM has legacy swift via StreamLanguage). Acceptable for pilot (json/xml/html/css) but add comment before widening.
• CSS worker duplication frontend/src/components/inputs/monaco/monacoSetup.js:9 + :14 (css contribution vs definitions/css/register.js) — harmless, keep comment.

🧪 Tests

• Improved frontend/src/components/inputs/monaco/monacoSetup.test.js:1 now covers 6/8 hex, 3/4 expand :16, 5/7 reject :34, var(--x) :22. Good.
• Still thin: missing rgba(255 0 0 / 0.5)→#ff000080 :130, named/hsla via canvas :112, getColorCtx()==false branch, applyDevtoolboxTheme skipping invalid token :164 and try/catch :196, pick() fallback #1f2428/#ffffff :174. No lifecycle tests for mount/dispose, lineNumbers toggle :114, setModelLanguage branch :128, loadError fallback :189.
• E2E helper frontend/e2e/helpers/editor.js:35,50 hard waitForTimeout(300) ×2 per fillEditor inflates suite. readEditorText via __monacoGetValue :73 is now correctly scoped to testId-content (good) — prior any-model poll fixed. fillEditor still does content.click()+Ctrl+A before __monacoSetValue — redundant.

Concrete patches:

// MonacoCodeEditor.jsx:70+117 delete placeholder spread
// ToolEditorPane.jsx:120 forward error
<MonacoCodeEditor error={error} showLineNumbers={showLineNumbers} wordWrap={wordWrap} ... />
// MonacoHighlightedCode.jsx:98 add wordWrap sync
editor.updateOptions({ lineNumbers:..., wordWrap: wordWrap?'on':'off' })
// useMonacoTheme.js:23 add cancellation (see above)
// MonacoCodeEditor.jsx:128 guard async language

No blocking vuln. Fix 1-3 before widening; rest follow-up.

opencode session  |  github run

[github-actions]

Package	Line Rate	Health
devtoolbox/internal/barcode	93%	✔
devtoolbox/internal/codeformatter	80%	➖
devtoolbox/internal/converter	65%	➖
devtoolbox/internal/datagenerator	81%	✔
devtoolbox/internal/datetimeconverter	75%	➖
devtoolbox/internal/jwt	47%	❌
devtoolbox/internal/numberconverter	98%	✔
devtoolbox/internal/settings	0%	❌
devtoolbox/internal/themes	0%	❌
Summary	70% (3726 / 5294)	➖

Minimum allowed line rate is 60%