fix(desktop): stop expanding $ replacement patterns in export titles by codeAnqiang-ma · Pull Request #6796 · nexu-io/open-design

codeAnqiang-ma · 2026-08-12T17:17:45Z

Why

Exporting an artifact whose title contains JavaScript replacement-pattern sequences corrupts the rendered document. injectTitle() in both desktop export paths passes the user-derived <title> tag as the string replacement argument of String.prototype.replace, so ECMA-262 GetSubstitution expands $$, $&, $`, $' coming from the artifact title:

Save $$$ This Quarter → <title>Save $$ This Quarter</title> (silently drops a $, wrong PDF Title metadata)
Before $& After → the matched <title>Old</title> is spliced in and the tag closes early; the leftover amp; After lands outside <head> and renders as visible text in the exported PDF/image
Rock $'n Roll Tour → the whole document tail is spliced into the title (content duplicated, title becomes HTML soup)

I hit this while exercising the export paths with $-bearing deck titles (pricing decks like "Save $$$ …" are a realistic shape). The escape helpers around the call (escapeHtmlText / escapeText) only cover & < >, so they clearly intend "insert the title verbatim" — the string-replacement expansion defeats that intent. The sibling helpers injectBaseHref / injectStyle already use function replacements and are unaffected; only the "document already has a <title>" branch of injectTitle used a string, and generated artifacts virtually always carry a <title>, so that is the branch that runs.

The fix mirrors injectBaseHref: use a function replacement (() => tag), whose return value is inserted literally. One line in each file:

apps/desktop/src/main/pdf-export.ts (desktop "Export as PDF" path)
apps/desktop/src/main/artifact-export.ts (od export pdf/image path)

What users will see

Exported PDFs and images now render the artifact title exactly as typed. A deck named Save $$$ This Quarter exports with that title (previously Save $$ This Quarter in the PDF Title metadata), and titles containing $& / $` / $' no longer splice document HTML into the <title> or leak stray text into the visible exported page. No settings, UI, or defaults change.

Surface area

UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
Keyboard shortcut — new or changed
CLI / env var — new od subcommand or flag, new tools-dev / tools-pack flag, or new OD_* env var
API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
i18n keys — added new translation keys (see TRANSLATIONS.md for the locale workflow)
New top-level dependency — adding any new entry to the root package.json
Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
None — bug fix in export rendering plus its regression test; no new surface

Screenshots

Not a UI change (no new entry point); the observable change is the exported document's <title> content, covered by the regression test below.

Bug fix verification

Test path that reproduces the bug: apps/desktop/tests/main/export-title-replacement-patterns.test.ts — six cases (three $-pattern titles × both exporters), capturing the document each exporter loads into its hidden render window and asserting the title lands verbatim (HTML-escaped only) with no duplicated document content.

Did the test go red on main and green on this branch? Yes. On main (028bde5) all six cases fail, e.g.:

FAIL  … > exportArtifact renders the title 'Before $& After' verbatim
Expected: "<title>Before $&amp; After</title>"
Received: "…<title>Before <title>Old</title>amp; After</title>…"

FAIL  … > exportArtifact renders the title 'Rock $'n Roll Tour' verbatim
Expected: "<title>Rock $'n Roll Tour</title>"
Received: "…<title>Rock </head><body><p>BODY MARKER</p></body></html>n Roll Tour</title>…"

Test Files  1 failed (1)   Tests  6 failed (6)

With the fix, Test Files 1 passed (1) Tests 6 passed (6).

Validation

pnpm --filter @open-design/desktop exec vitest run -c vitest.config.ts tests/main/export-title-replacement-patterns.test.ts — red on main (6 failed), green on this branch (6 passed).
pnpm --filter @open-design/desktop test — 36 files passed, 332 tests passed | 1 skipped. Baseline on main in the same tree before the change: 35 files, 326 passed | 1 skipped — the delta is exactly this PR's six new cases, no regressions.
pnpm guard — all design-system guards pass.
pnpm typecheck — whole-workspace typecheck completes cleanly (exit 0).

Environment note: run on Node v22 (engines want ~24), which only produces pnpm engine warnings; the same suites pass identically before and after the change on this machine.

Adjacent issues (not in this PR)

assembleExample() in apps/daemon/src/routes/static-resource.ts (L1488-L1492) interpolates a skill-derived title through the same string-replacement mechanism. Its trigger surface is much smaller (skill names), so per the bug follow-up workflow it belongs in its own follow-up rather than widening this diff.

This PR was prepared with AI assistance; I reproduced the bug locally, wrote and ran the red/green regression test, and reviewed every line.

lefarcen · 2026-08-12T17:38:51Z

Hey @codeAnqiang-ma — this change affects exported artifact output, so we’re marking it for manual QA before merge. Nothing needed from you right now; we’ll update here once that pass is done. Thanks for the careful fix and regression coverage.

PerishCode

@codeAnqiang-ma The two desktop export paths now insert escaped artifact titles through callback replacements, preventing JavaScript replacement-pattern expansion without changing the surrounding document handling. I reviewed every changed range and verified the six-case regression suite plus the desktop source/test typecheck on Node 24; both pass. Nicely scoped fix and strong coverage of the affected substitution patterns—thank you for making the failure mode and verification so clear.

_{🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.}

ivy-ting · 2026-08-13T03:23:11Z

@codeAnqiang-ma

Thanks for the contribution. I completed QA validation for this PR at 2845e06a203d8ef2a1222a0aac69602cd35802c8.

QA Acceptance Record

Scope:

Desktop PDF export and od export PDF/image title handling for JavaScript replacement-pattern sequences.

Verified:

The corruption reproduces on the base commit for $$, $&, and $'; this PR preserves each title literally after HTML escaping.
The focused regression suite passed (6/6), and the full desktop suite passed (332 passed, 1 skipped).
Desktop typecheck, build, pnpm guard, and current-head CI passed.
A real Electron run generated a valid PDF with metadata title Save $$$ This Quarter and a valid PNG with no leaked or duplicated document text.

Not verified / blocking:

The commit contains a Co-authored-by trailer, which the repository Git commit policy explicitly prohibits.

Risks / notes:

Please amend the commit to remove the co-author metadata and let CI run again on the new head.

Conclusion:

Functionally accepted; not accepted for merge until the commit metadata is corrected.

lefarcen · 2026-08-13T03:29:29Z

Thanks @ivy-ting — that closes the QA loop clearly.

@codeAnqiang-ma, the functional validation looks good from QA's side. The remaining blocker is the prohibited Co-authored-by trailer in the commit metadata; once that's removed and CI reruns on the new head, we can continue from there.

injectTitle() in pdf-export.ts and artifact-export.ts passed the user-derived <title> tag as the string replacement argument of String.replace(), so GetSubstitution expanded $$, $&, $`, $' sequences from the artifact title -- dropping characters or splicing document HTML into the title. Use a function replacement like the sibling injectBaseHref already does, so the tag is inserted literally.

codeAnqiang-ma · 2026-08-13T03:44:31Z

Removed the Co-authored-by trailer as requested; CI is re-running on the new head.

lefarcen · 2026-08-13T03:53:27Z

Thanks @codeAnqiang-ma — I can see the metadata-only follow-up landed on the new head and CI is green again.

Nothing else is needed from you right now. We’ll let the requested reviewers take the refreshed head from here.

lefarcen · 2026-08-14T07:33:48Z

Heads-up: PR #6891 is also open against this same bug and touches the same two desktop export files (apps/desktop/src/main/artifact-export.ts and apps/desktop/src/main/pdf-export.ts). You and @xxiaoxiong may want to compare approaches so we don’t duplicate the same fix work.

ivy-ting · 2026-08-14T08:54:40Z

@codeAnqiang-ma

Thanks for the update. I refreshed QA validation for this PR at 70c854b8918f841266ead0d88478b8340add67b8.

QA Acceptance Record

Scope:

Preserve literal replacement-pattern characters in Desktop PDF and image export titles.

Verified:

This head has the same implementation tree as the functionally validated version.
The prohibited co-author metadata was removed and current-head CI is green.
The focused regression suite, full Desktop suite, typecheck, build, and real Electron PDF/PNG export evidence remain applicable.

Conclusion:

Accepted.

codeAnqiang-ma requested a review from a team as a code owner August 12, 2026 17:17

lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps labels Aug 12, 2026

lefarcen requested a review from PerishCode August 12, 2026 17:35

lefarcen added the type/bugfix Bug fix label Aug 12, 2026

lefarcen mentioned this pull request Aug 12, 2026

[Bug]: Desktop PDF/image export corrupts titles containing $$, $&, $` or $' (injectTitle expands replace() patterns) #6795

Open

lefarcen added the needs-validation Runtime change detected; needs human or /explore agent validation. label Aug 12, 2026

PerishCode approved these changes Aug 12, 2026

View reviewed changes

lefarcen requested review from PerishCode and ivy-ting August 12, 2026 17:43

codeAnqiang-ma force-pushed the fix/6795-export-title-replacement-patterns branch from 2845e06 to 70c854b Compare August 13, 2026 03:43

lefarcen mentioned this pull request Aug 14, 2026

fix(desktop): prevent injectTitle from expanding JS replacement patterns in titles #6891

Open

ivy-ting added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Aug 14, 2026

ivy-ting approved these changes Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(desktop): stop expanding $ replacement patterns in export titles - #6796

fix(desktop): stop expanding $ replacement patterns in export titles#6796
codeAnqiang-ma wants to merge 1 commit into
nexu-io:mainfrom
codeAnqiang-ma:fix/6795-export-title-replacement-patterns

codeAnqiang-ma commented Aug 12, 2026

Uh oh!

lefarcen commented Aug 12, 2026

Uh oh!

PerishCode left a comment

Uh oh!

ivy-ting commented Aug 13, 2026

Uh oh!

lefarcen commented Aug 13, 2026

Uh oh!

codeAnqiang-ma commented Aug 13, 2026

Uh oh!

lefarcen commented Aug 13, 2026

Uh oh!

lefarcen commented Aug 14, 2026

Uh oh!

ivy-ting commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

codeAnqiang-ma commented Aug 12, 2026

Why

What users will see

Surface area

Screenshots

Bug fix verification

Validation

Adjacent issues (not in this PR)

Uh oh!

lefarcen commented Aug 12, 2026

Uh oh!

PerishCode left a comment

Choose a reason for hiding this comment

Uh oh!

ivy-ting commented Aug 13, 2026

QA Acceptance Record

Uh oh!

lefarcen commented Aug 13, 2026

Uh oh!

codeAnqiang-ma commented Aug 13, 2026

Uh oh!

lefarcen commented Aug 13, 2026

Uh oh!

lefarcen commented Aug 14, 2026

Uh oh!

ivy-ting commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants