fix(plugins): restore window.esc for out-of-tree plugins

[byrongamatos]

What testers saw

MIDI control no longer works — "MIDI Access denied esc is not defined"

MIDI access was never denied. It was granted. The error message is two unrelated things glued together.

plugins/midi/screen.js builds its device dropdown with esc(o.name), and the call sits inside the try block guarding requestMIDIAccess():

try {
    _midiAccess = await navigator.requestMIDIAccess({ sysex: false });  // succeeds
    _updateMidiDevices();   // throws ReferenceError: esc is not defined
} catch (e) {
    // ...renders "MIDI access denied" with e.message
}

So a ReferenceError from the device list gets reported by the handler meant for permission failures. Testers with working hardware and granted permission still see "access denied".

Root cause

esc() was a top-level function in static/app.js back when app.js was a classic script, which made it an implicit global any plugin could call.

• a9fce29 made app.js an ES module → top-level names became module-scoped.
• 14b4058 carved esc out into js/dom.js.
• The Object.assign(window, {...}) block that hands globals back to plugins was rebuilt without esc.

In-tree plugins were unaffected because they each define a local esc (career, achievements, input_setup, …). Out-of-tree plugins rely on the host's.

Why fix it here and not in the MIDI plugin

Four bundled plugins call esc() bare with no local definition — midi, multiplayer, nam_tone, practice. Only MIDI was reported. One entry in the export list fixes all four; patching MIDI alone leaves three broken.

The guard

tests/test_plugin_context_contract.py already pins the server-side plugin context by name, and its docstring says exactly why:

"Same lesson the frontend carve learned the hard way: a contract that only external code reads cannot be found by a call-graph scan, so it has to be pinned by name."

That lesson was written down but only applied to the server side. This adds the frontend twin — a literal-list pin of the plugin-facing window surface, same reasoning, same no-derived-list rule. Relevant with the module-refactor epic still open (#879, #901–#904, #917).

Verification

Both directions, against a running app in real Chromium:

• With the fix → spec passes.
• Without it → fails with window globals plugins depend on are missing or not functions: esc — the tester's bug, reproduced.

⚠️ The ./static:/app/static live-reload mount in docker-compose.yml does not propagate edits — the container snapshots at boot. My first run passed without the fix because it was serving a stale file; md5sum on disk vs. in-container confirmed the drift. Every result above required docker compose restart web. Worth knowing for anyone trusting that mount.

• npm run lint → 0 errors (12 pre-existing warnings)
• npm run test:js → 1099/1099
• codex exec preflight → NO ISSUES

Not in this PR

• Core-only. Testers won't see it until a desktop re-bundle of resources/slopsmith/.
• The MIDI plugin's catch reports any exception as "access denied". Moving _updateMidiDevices() outside the try would stop future bugs wearing a permissions costume — that's an out-of-tree repo, so it's a separate change.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Restored the globally available esc utility for browser plugins.
  • Ensured HTML content is safely escaped, helping prevent unescaped markup from being rendered.

• Tests

  • Added browser coverage verifying required plugin globals are available and function correctly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 08a7f5ac-41fb-4a96-b7b1-79e7389f455d

📥 Commits

Reviewing files that changed from the base of the PR and between 0b4b174 and f7b065d.

📒 Files selected for processing (2)

• static/app.js
• tests/browser/plugin-globals-contract.spec.ts

---

📝 Walkthrough

Walkthrough

The window export contract now includes esc, and Playwright tests verify required plugin globals plus HTML escaping through window.esc.

Changes

Plugin global contract

Layer / File(s)	Summary
Expose esc on the window contract static/app.js	Exports esc through Object.assign(window, ...) for out-of-tree plugins.
Validate plugin globals and escaping tests/browser/plugin-globals-contract.spec.ts	Adds a pinned global allowlist test and regression coverage confirming window.esc escapes HTML metacharacters.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• got-feedBack/feedBack#874: Updates the same window contract and plugin global tests.
• got-feedBack/feedBack#876: Covers the ES-module window export changes that prompted this contract update.

Suggested reviewers: omikronapex, chrisbewithyou

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and matches the main change: restoring window.esc for out-of-tree plugins.
Description check	✅ Passed	The description clearly explains the bug, root cause, and verification, though it does not follow the template sections exactly.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/plugin-esc-global

---

_{Comment @coderabbitai help to get the list of available commands.}