Skip to content

fix(drawer): three regressions from PR #338 — client-side sanitizer crash + HTP card bypassed sanitizer + STORIES header still first-person#343

Merged
mitwilli-create merged 1 commit into
mainfrom
fix/sanitizer-htp-card-2026-05-29-claude-9f65ce
May 30, 2026
Merged

fix(drawer): three regressions from PR #338 — client-side sanitizer crash + HTP card bypassed sanitizer + STORIES header still first-person#343
mitwilli-create merged 1 commit into
mainfrom
fix/sanitizer-htp-card-2026-05-29-claude-9f65ce

Conversation

@mitwilli-create
Copy link
Copy Markdown
Owner

Summary

Three regressions from PR #338 discovered via live Chrome MCP audit on 2026-05-30 (same session, drawer-quality follow-up).

The 3 fixes

1. Client-side sanitizer crash (CRITICAL — broke entire COMP INTELLIGENCE section)

_renderHMIntel runs in BROWSER context (emitted as inline <script>). PR #338 wired d = sanitizeObjectStrings(d) inside it, but sanitizeObjectStrings is a Node-side import at the top of build-dashboard.mjs — server-side only. At runtime the browser threw Error: sanitizeObjectStrings is not defined, replacing the COMP INTELLIGENCE section with that error string.

Fix: removed the client-side call. Server-side sanitization on the richSummary build pass (from PR #338) stays active and is extended to renderHowToPosition (see fix 2).

2. HTP card bypassed sanitizer (load-bearing for "Nivel detectado:" removal)

The posCard at line 3586 renders via renderHowToPosition(positioning) — a separate BUILD-TIME path. It was NOT covered by PR #338's _renderHMIntel or richSummary sanitization wiring. Result: 26 occurrences of "Nivel detectado:" Spanish residue still in served HTML.

Fix: wrap positioning with sanitizeDrawerText() before passing to renderHowToPosition.

3. STORIES card first-person header (bug 8 from original audit, missed in PR #338)

"Stories to use in your application" → "Stories Mitchell can use in his application" (both occurrences at lines 3737 + 3757).

Verification (curl on served HTML at https://dashboard.careers-ops.com/)

$ grep -c "Nivel detectado" /tmp/served.html
0   (was 26)

$ grep -c "Stories Mitchell can use" /tmp/served.html
299 (was 0)

$ grep -c "Stories to use in your application" /tmp/served.html
0   (was 57)

$ grep -c "d = sanitizeObjectStrings(d)" /tmp/served.html
0   (the broken client-side call)

$ grep -c "sanitizeObjectStrings is not defined" /tmp/served.html
0   (no more runtime error)

Chrome MCP visual verification: WorkOS Applied AI Engineer drawer renders fully — HOW TO POSITION section now reads "Detected level: Mid-to-senior IC..." instead of "Nivel detectado:". STORIES card label reads "Stories Mitchell can use in his application". COMP INTELLIGENCE renders the comp table (no more JS error breaking the section).

Why this wasn't caught pre-#338

Lesson for the follow-up retroactive sweep spec: any client-side renderer that calls a sanitizer needs the sanitizer rules INLINED into the browser bundle. Cross-context imports don't work across the build-time / runtime boundary.

Test plan

  • node --check scripts/build-dashboard.mjs (clean)
  • node scripts/build-dashboard.mjs (clean build; 8 inline <script> blocks parse)
  • Live Chrome MCP visual verification on https://dashboard.careers-ops.com/ — WorkOS drawer renders all sections without JS errors
  • Grep verification of all 3 fix targets in served HTML

🤖 Generated with Claude Code

…rash + HTP card bypassed sanitizer + STORIES header still first-person

PR #338 introduced 3 regressions discovered via live Chrome MCP audit on 2026-05-30:

1. **Client-side sanitizer crash** — _renderHMIntel runs in BROWSER context (emitted as inline <script>). The Node-side `import { sanitizeObjectStrings } from '../lib/drawer-content-sanitizer.mjs'` at line 28 is server-side only. PR #338 wired `d = sanitizeObjectStrings(d)` inside _renderHMIntel — at runtime the browser crashed with "Error: sanitizeObjectStrings is not defined", breaking the COMP INTELLIGENCE section render entirely. Fix: remove the client-side call. Server-side sanitization on richSummary build pass (added in PR #338) still active and now also applied to renderHowToPosition (see fix 2).

2. **HTP card bypassed sanitizer** — the posCard at line 3586 renders via `renderHowToPosition(positioning)` which is a separate BUILD-TIME path that bypasses both _renderHMIntel and richSummary sanitization. Result: 26 occurrences of "Nivel detectado:" Spanish residue in served HTML. Fix: wrap `positioning` with `sanitizeDrawerText()` before passing to renderHowToPosition.

3. **STORIES card first-person header** — bug 8 from the original audit, missed in PR #338. "Stories to use in your application" → "Stories Mitchell can use in his application" (both occurrences at lines 3737 + 3757). Voice rules now applied to all visible drawer section headers.

Audit notes: .claude/audit/drawer-hotfix-2026-05-30/notes.md
SKIP_UI_VERIFY=1 used because Chrome MCP verification was done LIVE during the iteration cycle (multiple bootout/bootstrap + screenshot cycles); rationale: live screenshot already exists in this session's transcript.

Verification (curl on served HTML):
- "Nivel detectado" = 0 (was 26)
- "Stories Mitchell can use" = 299 (was 0)
- "Stories to use in your application" = 0 (was 57)
- "d = sanitizeObjectStrings(d)" = 0 (the broken client-side call removed)
- "sanitizeObjectStrings is not defined" runtime error = 0
- Server HTTP 200 across bootout/bootstrap cycles

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

⚠️ Critical file overlap detected

This PR modifies files that are also modified by one or more other open PRs.
Merging in the wrong order can produce structural merge conflicts.


⚠️  CRITICAL FILE OVERLAP DETECTED
============================================================

PR #343 shares critical files with 1 other open PR(s).
These files need to be merged in sequence to avoid structural conflicts.

  PR #319 "fix(dashboard): consistent 'Deep Refresh' capitalization across 10 user-visible labels"
    Branch: fix/dashboard-deep-refresh-button-rename-2026-05-29
    Overlap: scripts/build-dashboard.mjs

Options:
  1. Merge the other PR first, then rebase this branch onto main
  2. Coordinate with the other PR author to split responsibility
  3. If your changes are independent sections, proceed — but rebase after the other PR merges

See AGENTS.md § Bug class: pr-conflict-mirage-from-parallel-shipping

This is a warning, not a block. CI still passes. Merge order matters — coordinate before merging.

See AGENTS.md § Bug class: pr-conflict-mirage-from-parallel-shipping for the triage + recovery playbook.

@mitwilli-create mitwilli-create merged commit 5b7f597 into main May 30, 2026
10 checks passed
mitwilli-create pushed a commit that referenced this pull request May 30, 2026
…(2026-05-30)

Phase 4 merged PR #341 (Tahoe TCC wrapper move) + PR #343 (drawer
regression fix). Deferred PR #344 (safe-dashboard-deploy) for real
CodeQL HIGH on lint-browser-context-refs.mjs:79.

Discovery during verification: network-database-build.plist invokes
node with cron-run.sh (bash) as arg — pre-existing bug from before
PR #341 (commit 3a917c7 era). All 4 sibling plists use /bin/bash
correctly. Not a regression. Mitchell follow-up needed.

Phase 6D auto-reseed: baselines anchored to 5b7f597 +
data/deployment-report-2026-05-30-0849.md.

Session spend: ~\$0.26.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mitwilli-create pushed a commit that referenced this pull request May 30, 2026
… 2026-05-30 4-fold cascade

Codifies the dashboard deploy ritual into a single guarded wrapper. Each gate
corresponds to a real failure mode from the 2026-05-29/30 cascade in PRs #338/#343
that wasted ~2 hours debug time across multiple Claude instances.

What landed (10 files):
- scripts/safe-dashboard-deploy.sh — 6-gate orchestrator, exit codes 2-9
- scripts/lint-browser-context-refs.mjs — Acorn AST walker catches client-side-ref-to-server-side-import
- scripts/lint-backtick-in-comment.mjs — Acorn parse-based outer-template-unescape detector
- scripts/dashboard-headless-canary.mjs — Playwright headless, forbidden-pattern console check
- scripts/hooks/commit-msg — extended with safe-deploy guard refusing ad-hoc dashboard/index.html commits
- tests/safe-dashboard-deploy.test.mjs — 17/17 pass
- AGENTS.md — 3 new bug-class entries (client-side-reference-to-server-side-import,
  stale-worktree-cp-backward-merge, ad-hoc-cp-of-build-artifact)
- CLAUDE.md — Session Notes 2026-05-30
- package.json + package-lock.json — acorn-walk@8.3.5 dev dep

Verified gate-by-gate:
- Gate 1 (exit 2): synthetic stale-worktree at HEAD~5 → "source HEAD is BEHIND origin/main"
- Gate 2 (exit 3): missing data symlink → actionable ln -sfn recovery
- Gate 3 (exit 5): end-to-end run on current main → catches the PR #343 sanitizeObjectStrings bug
- Gate 4 (exit 6): unit-tested with synthetic backtick-in-comment
- Gates 5+6: unit-tested at contract level

PR #343 (sanitizer hotfix) is the precondition for the wrapper's first clean
end-to-end run. Gate 3 correctly refuses current main until #343 lands —
this is the wrapper working as designed.

Spec: data/spec-safe-dashboard-deploy-2026-05-30.md

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mitwilli-create added a commit that referenced this pull request May 30, 2026
…y + 3 bug-class entries (#344)

* feat(infra): safe-dashboard-deploy wrapper — 6-gate guard against the 2026-05-30 4-fold cascade

Codifies the dashboard deploy ritual into a single guarded wrapper. Each gate
corresponds to a real failure mode from the 2026-05-29/30 cascade in PRs #338/#343
that wasted ~2 hours debug time across multiple Claude instances.

What landed (10 files):
- scripts/safe-dashboard-deploy.sh — 6-gate orchestrator, exit codes 2-9
- scripts/lint-browser-context-refs.mjs — Acorn AST walker catches client-side-ref-to-server-side-import
- scripts/lint-backtick-in-comment.mjs — Acorn parse-based outer-template-unescape detector
- scripts/dashboard-headless-canary.mjs — Playwright headless, forbidden-pattern console check
- scripts/hooks/commit-msg — extended with safe-deploy guard refusing ad-hoc dashboard/index.html commits
- tests/safe-dashboard-deploy.test.mjs — 17/17 pass
- AGENTS.md — 3 new bug-class entries (client-side-reference-to-server-side-import,
  stale-worktree-cp-backward-merge, ad-hoc-cp-of-build-artifact)
- CLAUDE.md — Session Notes 2026-05-30
- package.json + package-lock.json — acorn-walk@8.3.5 dev dep

Verified gate-by-gate:
- Gate 1 (exit 2): synthetic stale-worktree at HEAD~5 → "source HEAD is BEHIND origin/main"
- Gate 2 (exit 3): missing data symlink → actionable ln -sfn recovery
- Gate 3 (exit 5): end-to-end run on current main → catches the PR #343 sanitizeObjectStrings bug
- Gate 4 (exit 6): unit-tested with synthetic backtick-in-comment
- Gates 5+6: unit-tested at contract level

PR #343 (sanitizer hotfix) is the precondition for the wrapper's first clean
end-to-end run. Gate 3 correctly refuses current main until #343 lands —
this is the wrapper working as designed.

Spec: data/spec-safe-dashboard-deploy-2026-05-30.md

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(lint): regex matches </script> with trailing whitespace (CodeQL HIGH)

Line 79 — CWE-184 — `</script>` extended to `</script\s*>` so the
lint catches HTML5-valid `</script >` variant. Closes CodeQL alert.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(lint): broaden SCRIPT_RE end-tag regex to satisfy CodeQL

CWE-184 alert reproposed after \s* fix landed: CodeQL flags any
regex that doesn't match `</script\t\n bar>` (technically
invalid HTML, but the CodeQL rule is strict). Broaden to
`</script\b[^>]*>` to catch the edge case + close the alert.

The \b prevents matching things like `</scriptlet>`. The [^>]*
matches any content between `script` and `>` including whitespace,
tabs, newlines, and bogus attributes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Mitchell Williams <mitchellwilliams@Mitchells-MacBook-Air.local>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant