chore: v3.5.13 — 2026-05-10 quality pass

.github/workflows/ci.yml

@@ -39,6 +39,9 @@ jobs:
       - name: Check glossary consistency
         run: node scripts/check-glossary.js
 
+      - name: Check i18n key coverage
+        run: node scripts/check-i18n-keys.js
+
       - name: Check shared constants sync
         run: node scripts/check-bg-sync.js
 

CHANGELOG.md

@@ -6,6 +6,33 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+## [3.5.13] - 2026-05-11
+
+### Refactor
+- Split `src/content/sidebar-chat.js` (1224 → 815 lines, –33%) into three modules with a shared `window._sb._chat` namespace:
+  - `src/content/chat-render.js` (new) — `formatResponse`, `applyInline`, `sanitizeHtml`. Pure DOM-free markdown→HTML + the trusted-structure HTML sanitizer used by both the live chat bubble and the history detail view.
+  - `src/content/chat-history.js` (new) — IndexedDB conversation store, history sub-panel, detail view. Owns `chat-history` IDB store + the panel UI; calls back into sidebar-chat through `_sb._chat.{closeSubPanel,formatResponse,sanitizeHtml}`.
+  - `src/content/sidebar-chat.js` keeps panel infrastructure (sidebar inject, chat input, sub-panel state machinery, focus trap, flashcards, PDF export, send-chat stream).
+  - Sub-panel state (`savedChatHTML`, `historyPanelOpen`, `flashcardPanelOpen`) hoisted to `_sb._chat.state` so history and flashcard modules share one source of truth instead of duplicating local flags.
+  - `manifest.json` `content_scripts.js` order updated to load `chat-render.js` before `sidebar-chat.js` (which now exposes the `_sb._chat` panel helpers) and `chat-history.js` after (which consumes them).
+  - `tests/format-response.test.js` follows `formatResponse`/`applyInline` to its new home in `chat-render.js`.
+
+### Added
+- `src/lib/_sb-typedef.js` — JSDoc-only contract for the `window._sb` shared namespace and the `_chat`/`ProtectedTermsApi`/`GeminiBlockApi` sub-namespaces. Not loaded at runtime (no manifest entry); picked up by IDEs and `tsc --noEmit` via the new `tsconfig.json`.
+- `tsconfig.json` — `allowJs` + `checkJs` for IDE/local type checking against existing JSDoc. `strict: false` so the rollout doesn't surface a wave of pre-existing nullability warnings; tighten incrementally as files migrate.
+- `scripts/check-i18n-keys.js` + `npm run check:i18n` — validates (1) every locale under `_locales/` matches the English `messages.json` key set so Chrome doesn't silently fall back, and (2) every `*_LABELS` / `*_GREETINGS` / `*_PLACEHOLDERS` dictionary in `src/lib/constants.js` is shape-consistent across the 11 premium languages, both for flat `{ en, ko, … }` maps and section-outer `{ key: { en, ko, … } }` maps. Wired into the CI `validate` job.
+- `docs/E2E_PLAN.md` — working spec for the deferred Playwright suite. Records the 6 priority coverage targets (golden translation, SPA mid-stream, cache-cleanup alarm, stream-cancel, protected-terms, panel switch) so the next person who picks it up doesn't restart from zero.
+
+### Fixed
+- `restoreProtectedTerms(text)` now defends against three production-realistic edge cases that previously corrupted output or crashed: `null`/`undefined`/non-string input (returned safe fallback instead of throwing on `.includes`); empty-string wrong-forms in the dictionary (`String.prototype.replaceAll('', x)` would have inserted the correct form between every char); and self-mapping entries where a wrong-form equals its correct form (silent no-op cycle bloating the hot loop on long pages).
+- Background SW + content-script handlers gain a `_logMisroutedMessage` defensive log: if a `{ action: ... }`-shaped message reaches the background (or a `{ type: ... }` reaches a content script) it warns loudly with the discriminator. Catches the v3.5.6 cache-cleanup class of bug at first occurrence in dev rather than silently falling through "Unknown action".
+
+### Build
+- `scripts/build-bundle.js` esbuild now passes `pure: ['console.debug', 'console.info']` to the content + background bundles. With `minify: true` already on, those calls get tree-shaken from production output (verified 0 occurrences in `dist/bundled/*.bundle.js`). `console.warn` / `console.error` deliberately preserved so real degradation/errors still reach DevTools.
+
+### Tests
+- `tests/protected-terms.test.js` (+6 tests): null/undefined/non-string input safety, idempotence (`f(f(x)) == f(x)`), empty-wrong-form skip, self-mapping skip, non-string-array-element skip. Total now 24 tests against the protected-terms helper.
+
 ## [3.5.12] - 2026-05-11
 
 ### Fixed

docs/E2E_PLAN.md

@@ -0,0 +1,91 @@
+# E2E Test Plan (Deferred)
+
+Status: **planned**, not yet implemented. Tracked from the v3.5.13 quality
+pass on 2026-05-10. The current Jest suite covers pure functions and message
+shapes well, but never exercises the real load order / inter-module wiring
+(content.js → banners.js → chat-render.js → sidebar-chat.js → chat-history.js,
+plus the page-bridge / background trip).
+
+This file is the working spec for what to add when we get there. Update it
+when scope changes; do not let it bit-rot silently.
+
+## Why we need this
+
+The recent v3.5.6 → v3.5.12 hotfix train surfaced a recurring class of bug
+that no unit test could catch:
+
+- v3.5.6 cache-cleanup `action` vs `type` discriminator mismatch
+- v3.5.7 translator + bridge race during SPA navigation
+- v3.5.8 `fetchWithRetry` 4xx fail-fast bug
+- v3.5.9 sidebar stream cancel / IDB resilience
+- v3.5.10 YouTube subtitle timer leak
+
+All of these required a real extension load + a real Skilljar-shaped page to
+reproduce. We've been catching them in production. Playwright + a minimal
+fixture page would close that gap.
+
+## Stack
+
+- **Playwright** (`@playwright/test`) — Chromium + Firefox launchers, native
+  extension loading via `--load-extension=<dist/firefox>` / `--disable-extensions-except`.
+- **Local fixture pages** under `tests/e2e/fixtures/` — static HTML that
+  mimics the Skilljar DOM shape (lesson body, quiz form, header, video
+  container) without depending on a live skilljar.com URL.
+- **Network stubbing** via Playwright `route()` for:
+  - `https://translate.googleapis.com/*` — return canned translations
+  - `https://api.github.com/*` — return canned latest-release JSON
+  - `https://api.puter.com/*` (or whatever the page-bridge fetches) — return
+    canned Gemini stream chunks
+
+## Coverage targets (priority order)
+
+1. **Golden page translation** — load fixture, set language=ko in popup,
+   assert visible body text becomes Korean and the cache hit path works on
+   second visit.
+2. **SPA navigation mid-translation** — start translating, push a new URL via
+   `history.pushState`, assert no stale translation lands in the new page
+   (verifies `_langGeneration` invariant).
+3. **Cache cleanup alarm** — fast-forward Chrome `alarms` to fire
+   `cache-cleanup` after a stale entry is in IDB, assert it's purged.
+4. **Stream cancel on sidebar close** — open chat, start a (stubbed) stream,
+   close sidebar, assert `AbortController.signal.aborted === true` and no
+   half-saved IDB entry.
+5. **Protected Terms restoration** — fixture page with "클로드" in body,
+   target=Korean, assert it ends as "Claude" after the verify step.
+6. **History panel + flashcard panel switch** — open history, then open
+   flashcards from chat without restoring chat first; assert we don't blow
+   away `savedChatHTML` (this was a near-miss bug during the v3.5.13 split).
+
+## File layout (when implemented)
+
+```
+tests/e2e/
+  fixtures/
+    skilljar-lesson.html
+    skilljar-quiz.html
+    youtube-embed.html
+  helpers/
+    extension.ts          — launch helper (loads dist/firefox into Playwright)
+    network-stubs.ts      — GT / GitHub / Puter route handlers
+  golden-translation.spec.ts
+  spa-navigation.spec.ts
+  cache-cleanup.spec.ts
+  stream-cancel.spec.ts
+  protected-terms.spec.ts
+  panel-switch.spec.ts
+playwright.config.ts
+```
+
+Add `e2e:install` (`playwright install chromium`) and `e2e` (`playwright test`)
+scripts to `package.json`. Wire into a separate CI job — keep the fast `test`
+job under 30 s by not running E2E on every push.
+
+## Open questions
+
+- Do we ship a real Skilljar lesson HTML snapshot, or hand-craft a minimal
+  fixture? Real snapshot catches more selector regressions; minimal fixture
+  is cheaper to maintain.
+- Service worker lifecycle in Playwright — Chromium suspends it; we may need
+  `chrome.runtime.getBackgroundPage()` (deprecated in MV3) replacement via
+  Playwright's `serviceWorkers()` API.
+- Firefox MV3 differences — separate spec file or per-test guards?

manifest.json

@@ -2,7 +2,7 @@
   "manifest_version": 3,
   "name": "__MSG_extName__",
   "description": "__MSG_extDescription__",
-  "version": "3.5.12",
+  "version": "3.5.13",
   "minimum_chrome_version": "120",
   "author": "SkillBridge Contributors",
   "homepage_url": "https://github.com/heznpc/skillbridge",
@@ -28,7 +28,7 @@
   "content_scripts": [
     {
       "matches": ["https://*.skilljar.com/*"],
-      "js": ["src/lib/browser-polyfill.js", "src/lib/selectors.js", "src/lib/constants.js", "src/lib/translator.js", "src/lib/youtube-subtitles.js", "src/lib/protected-terms.js", "src/lib/gemini-block.js", "src/content/content.js", "src/content/banners.js", "src/content/code-comments.js", "src/content/header-controls.js", "src/content/text-selection.js", "src/content/sidebar-chat.js", "src/content/keyboard-shortcuts.js"],
+      "js": ["src/lib/browser-polyfill.js", "src/lib/selectors.js", "src/lib/constants.js", "src/lib/translator.js", "src/lib/youtube-subtitles.js", "src/lib/protected-terms.js", "src/lib/gemini-block.js", "src/content/content.js", "src/content/banners.js", "src/content/code-comments.js", "src/content/header-controls.js", "src/content/text-selection.js", "src/content/chat-render.js", "src/content/sidebar-chat.js", "src/content/chat-history.js", "src/content/keyboard-shortcuts.js"],
       "css": ["src/content/content.css"],
       "run_at": "document_idle"
     }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "skillbridge",
-  "version": "3.5.12",
+  "version": "3.5.13",
   "private": true,
   "scripts": {
     "test": "jest --verbose",
@@ -14,6 +14,7 @@
     "check:selectors": "node scripts/check-selectors.js",
     "check:dicts": "node scripts/check-dicts.js",
     "check:sync": "node scripts/check-bg-sync.js",
+    "check:i18n": "node scripts/check-i18n-keys.js",
     "docs": "node scripts/generate-docs.js",
     "lint": "eslint src/ tests/ scripts/",
     "lint:fix": "eslint src/ tests/ scripts/ --fix",

scripts/build-bundle.js

@@ -23,13 +23,20 @@ async function build() {
   const contentEntryPath = path.join(DIST, '_content-entry.js');
   fs.writeFileSync(contentEntryPath, contentEntry);
 
+  // `pure: ['console.debug', 'console.info']` lets minify drop those calls
+  // entirely from the production bundle (their return values are unused, so
+  // marking them pure tree-shakes the call-sites). `console.warn`/`error` are
+  // preserved on purpose so real degradation/errors still reach DevTools.
+  const PROD_PURE = ['console.debug', 'console.info'];
+
   await esbuild.build({
     entryPoints: [contentEntryPath],
     outfile: path.join(DIST, 'content.bundle.js'),
     bundle: false, // Already concatenated, just minify
     minify: true,
     target: ['chrome120'],
     format: 'iife',
+    pure: PROD_PURE,
   });
 
   // Bundle background service worker
@@ -40,6 +47,7 @@ async function build() {
     minify: true,
     target: ['chrome120'],
     format: 'iife',
+    pure: PROD_PURE,
   });
 
   // Bundle CSS

scripts/check-i18n-keys.js

@@ -0,0 +1,187 @@
+#!/usr/bin/env node
+/**
+ * SkillBridge — i18n Key Coverage Check
+ *
+ * Two checks:
+ *   (1) `_locales/<lang>/messages.json` files all share the same key set as the
+ *       English baseline. Chrome rejects an extension if `default_locale` keys
+ *       are missing in other locales used at install time, and divergence
+ *       silently falls back to English without warning.
+ *   (2) The label dictionaries declared in `src/lib/constants.js`
+ *       (POPUP_LABELS, A11Y_LABELS, etc.) are object-shaped and every language
+ *       sub-object exposes the same key set, so nested labels never quietly
+ *       drop a sub-key on translation.
+ *
+ * Exit code 1 on hard mismatches; warnings are non-fatal.
+ *
+ * Usage: node scripts/check-i18n-keys.js
+ */
+
+const fs = require('fs');
+const path = require('path');
+
+const ROOT = path.resolve(__dirname, '..');
+const LOCALES_DIR = path.join(ROOT, '_locales');
+const CONSTANTS_FILE = path.join(ROOT, 'src', 'lib', 'constants.js');
+
+let errors = 0;
+let warnings = 0;
+
+const log = {
+  pass: (m) => console.log('  ✓', m),
+  warn: (m) => {
+    warnings++;
+    console.warn('  ⚠', m);
+  },
+  fail: (m) => {
+    errors++;
+    console.error('  ✗', m);
+  },
+};
+
+// ==================== CHECK 1: _locales coverage ====================
+
+console.log('\n--- Check 1: _locales/<lang>/messages.json key coverage ---');
+const baselineLocale = 'en';
+const baselinePath = path.join(LOCALES_DIR, baselineLocale, 'messages.json');
+if (!fs.existsSync(baselinePath)) {
+  log.fail(`Baseline locale missing: ${baselineLocale}/messages.json`);
+} else {
+  const baseline = JSON.parse(fs.readFileSync(baselinePath, 'utf8'));
+  const baselineKeys = new Set(Object.keys(baseline));
+  const locales = fs.readdirSync(LOCALES_DIR).filter((d) => fs.statSync(path.join(LOCALES_DIR, d)).isDirectory());
+
+  for (const lang of locales) {
+    if (lang === baselineLocale) continue;
+    const file = path.join(LOCALES_DIR, lang, 'messages.json');
+    if (!fs.existsSync(file)) {
+      log.fail(`Missing messages.json for locale: ${lang}`);
+      continue;
+    }
+    let data;
+    try {
+      data = JSON.parse(fs.readFileSync(file, 'utf8'));
+    } catch (e) {
+      log.fail(`Invalid JSON in ${lang}/messages.json: ${e.message}`);
+      continue;
+    }
+    const keys = new Set(Object.keys(data));
+    const missing = [...baselineKeys].filter((k) => !keys.has(k));
+    const extra = [...keys].filter((k) => !baselineKeys.has(k));
+    if (missing.length || extra.length) {
+      const parts = [];
+      if (missing.length) parts.push(`missing ${missing.join(', ')}`);
+      if (extra.length) parts.push(`extra ${extra.join(', ')}`);
+      log.fail(`${lang}: ${parts.join('; ')}`);
+    }
+  }
+  if (errors === 0) log.pass(`All ${locales.length} locales match ${baselineLocale} key set`);
+}
+
+// ==================== CHECK 2: constants.js label dict shape ====================
+
+console.log('\n--- Check 2: constants.js label dictionaries ---');
+
+// constants.js references selectors.js' globals at the top, so load both into
+// the same Function scope before extracting the LABEL dicts.
+const SELECTORS_FILE = path.join(ROOT, 'src', 'lib', 'selectors.js');
+const selectorsSrc = fs.readFileSync(SELECTORS_FILE, 'utf8');
+const constantsSrc = fs.readFileSync(CONSTANTS_FILE, 'utf8');
+
+function matchExports(src) {
+  const names = [];
+  const re = /^const ([A-Z_][A-Z0-9_]*(?:LABELS|GREETINGS|PLACEHOLDERS|UI|QUESTIONS|DESCRIPTIONS))\s*=/gm;
+  let m;
+  while ((m = re.exec(src)) !== null) names.push(m[1]);
+  return names;
+}
+
+let dicts;
+try {
+  const names = matchExports(constantsSrc);
+  // Both source files reference `window` at the top; provide a stub so the
+  // top-level `if (typeof window !== 'undefined')` guards don't trip.
+  const runner = new Function('window', `${selectorsSrc}\n${constantsSrc}\nreturn { ${names.join(', ')} };`);
+  dicts = runner({});
+} catch (e) {
+  log.fail(`Failed to load constants.js: ${e.message}`);
+  printSummary();
+  process.exit(errors > 0 ? 1 : 0);
+}
+
+const expectedLangs = new Set(['en', 'ko', 'ja', 'zh-CN', 'zh-TW', 'es', 'fr', 'de', 'pt-BR', 'ru', 'vi']);
+
+/**
+ * Validate a flat lang map: { en: 'x', ko: 'x', ... }.
+ * @returns {boolean} true if at least one expected lang is present.
+ */
+function looksLikeLangMap(obj) {
+  if (!obj || typeof obj !== 'object') return false;
+  return Object.keys(obj).some((k) => expectedLangs.has(k));
+}
+
+function checkLangCoverage(label, dict) {
+  const missing = [...expectedLangs].filter((l) => !(l in dict));
+  if (missing.length) log.warn(`${label}: missing language(s) ${missing.join(', ')}`);
+}
+
+function checkLangOuter(name, dict) {
+  // { en: object|string, ko: object|string, ... }
+  checkLangCoverage(name, dict);
+  const enValue = dict.en;
+  if (enValue && typeof enValue === 'object' && !Array.isArray(enValue)) {
+    const baseline = new Set(Object.keys(enValue));
+    for (const lang of Object.keys(dict)) {
+      const v = dict[lang];
+      if (!v || typeof v !== 'object' || Array.isArray(v)) {
+        log.fail(`${name}.${lang}: expected object (matching .en), got ${typeof v}`);
+        continue;
+      }
+      const sub = new Set(Object.keys(v));
+      const miss = [...baseline].filter((k) => !sub.has(k));
+      const extra = [...sub].filter((k) => !baseline.has(k));
+      if (miss.length) log.fail(`${name}.${lang}: missing sub-keys ${miss.join(', ')}`);
+      if (extra.length) log.warn(`${name}.${lang}: extra sub-keys ${extra.join(', ')}`);
+    }
+  }
+}
+
+function checkSectionOuter(name, dict) {
+  // { sectionA: { en, ko, ... }, sectionB: {...}, ... }
+  // Skip if no value is an object (e.g. SKILLBRIDGE_MODEL_LABELS is a flat
+  // language-agnostic lookup, not an i18n dict).
+  const hasObjectValues = Object.values(dict).some((v) => v && typeof v === 'object');
+  if (!hasObjectValues) return;
+  for (const [section, langMap] of Object.entries(dict)) {
+    if (!langMap || typeof langMap !== 'object') continue;
+    if (!looksLikeLangMap(langMap)) {
+      // Nested deeper — recurse one level
+      for (const [sub, deeper] of Object.entries(langMap)) {
+        if (looksLikeLangMap(deeper)) checkLangCoverage(`${name}.${section}.${sub}`, deeper);
+      }
+      continue;
+    }
+    checkLangCoverage(`${name}.${section}`, langMap);
+  }
+}
+
+for (const [name, dict] of Object.entries(dicts)) {
+  if (!dict || typeof dict !== 'object') {
+    log.warn(`${name}: not an object, skipping`);
+    continue;
+  }
+  if (looksLikeLangMap(dict)) {
+    checkLangOuter(name, dict);
+  } else {
+    checkSectionOuter(name, dict);
+  }
+}
+
+if (errors === 0) log.pass(`All label dictionaries shape-consistent`);
+
+printSummary();
+process.exit(errors > 0 ? 1 : 0);
+
+function printSummary() {
+  console.log(`\n${errors} error(s), ${warnings} warning(s)`);
+}