Skip to content

fix(platform): preserve bulk Inbox plaintext in HTML sends - #4389

Merged
yannickmonney merged 1 commit into
mainfrom
fix/bulk-inbox-plaintext
Oct 9, 2026
Merged

yannickmonney merged 1 commit into
mainfrom
fix/bulk-inbox-plaintext

Conversation

@yannickmonney

@yannickmonney yannickmonney commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Bulk Inbox messages came from a plain textarea but were sent through the HTML reply contract unchanged, losing literal tags and collapsing line breaks. Escape the textarea text with the existing he library and serialize newlines as <br> inside a paragraph before the real reply mutation. Stored messages and queued HTML emails receive that same safe body; rich editor sends retain their existing behavior.

Validation:

  • Focused component tests: 49 passing (bulk action, HTTP adapter payload, dialog, summaries).
  • Native send/watchdog tests: 46 passing; stored content and queued HTML body regressions cover placeholders, ampersands, script/bold markup and blank lines. No provider delivery.
  • Real Chromium EmailPreview: 5 passing, including a rich HTML control (review-only config uses the installed Chromium; installed Vitest/browser version mismatch warning retained).
  • Scoped oxlint, oxfmt, TypeScript diagnostics, local SAST and platform manual-register validation pass.
  • Final rendered-preview review using the visual-aspect-analyzer skill: 100/100; evidence retained with the task deliverables.

Sweep: single replies and compose use the Milkdown snapshot serialized/sanitized by toOutboundHtml; backend bulk reply forwards the caller's existing HTML contract. Only the plain bulk textarea boundary changes. No user-visible copy changed.

Register insertion anchor: the existing “Inbox reply and new-email sends read current HTML” editor-snapshot row. Inspected gh pr diff for every open PR touching this register (43); none touches that anchor. git merge-tree found no newly introduced register conflicts: 24 merge cleanly; 19 older PRs already conflict in that file against the unchanged base. Evidence is retained in task artifacts.

Closes #3920

Current-main rebase

Replayed the previously accepted source 71ceecc9fe72fdc7fedbd8f894909bd6962a0476 onto main d1373d84cd56972501403f62145ec52e6f65d44a, including the merged shared CI repair in #4625. The accepted feature payload and all current-main changes are preserved in one atomic commit. Configured commit and conflict checks pass; earlier behavioral proof remains recorded above. All seven native required checks and full merge-group validation remain required for this new source.

Maintenance replay: preserves the accepted feature payload on current main 7d178ca. Includes the merged #4649 Knip cleanup and the exact independently accepted one-line shared CLI inventory repair from #4654 (252f0df), which is still pending native merge on main. The fixed suite inventory keeps its discovery and source/compiled phase guards. Existing feature proof is retained; no fresh full-feature/full-workspace test or hosted-green claim. Native required checks remain mandatory.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

TALE-336 / TALE-359 fleet handoff: PR #4389 fixes #3920 by escaping bulk textarea input before the HTML send contract, with line-preserving markup. Stored messages and queued outbound email use the same safe HTML. Single reply and compose rich editor paths are unchanged; backend bulk reply retains the existing HTML caller contract.

Local proof: 49 component tests, 46 native send/watchdog tests and 5 real Chromium preview tests pass. Scoped types, oxlint, format, SAST and manual register checks pass. Visual-aspect-analyzer final rendered-preview score: 100/100. No provider delivery was attempted. No copy changed.

Register anchor: the “Inbox reply and new-email sends read current HTML” snapshot row. All 43 open PR register diffs inspected; none touches this anchor. Merge-tree verifies no new register conflicts (19 older branches already conflict against the unchanged base).

CI is running. Independent review remains required; this PR has not been merged or self-accepted. Workspace task reporting tools returned unavailable, so this PR comment is the requested fallback report for both TALE-336 and TALE-359 (3729d02a-eb44-42b2-a68e-1bec2cebbcd6).

@yannickmonney

Copy link
Copy Markdown
Contributor Author

TALE-336 / TALE-359 final handoff at head 6516048: local validation is complete, including scoped type-aware oxlint and zero scoped TypeScript diagnostics. The test-only follow-up narrows the recorded JSON body; its 15 focused cases pass. Total validated coverage remains 49 component + 46 native + 5 Chromium cases. Final rendered-preview visual review: 100/100.

All seven current-head workflow runs are still pending; Checks has no jobs yet. No CI failure has been observed or attributed to main. Watching was attempted. Cancelling obsolete first-head runs was refused with HTTP 403 (token lacks Actions-management permission); no runs changed. CI completion and independent review remain open. The PR is mergeable and has not been merged or self-accepted.

This is the fallback report for TALE-336 and TALE-359 (3729d02a-eb44-42b2-a68e-1bec2cebbcd6), since workspace reporting tools returned unavailable. Task artifacts contain the patch, all local verification logs, screenshot/visual report, register diff/merge-tree evidence and CI snapshots.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

TALE-907 independent exact-head verdict — request changes (P2) at 6516048d462879ebf269e3d85fe35b89f48df16e. Reviewer: Codex #8, independent of implementer Codex #7.

API-channel bulk replies lose content. use-bulk-actions.ts:212 now converts every selected conversation’s text to HTML without a channel distinction or sourceMarkdown. API conversations with a contact email are eligible for this action. At send.ts:474, the API branch strips tags without decoding entities or retaining <br>. Independent reproduction through the actual replyToConversation boundary: A&B. Second line is queued as A&amp;B.Second line; the same raw input previously retained its ampersand and newline on that branch. Preserve the API reply’s plaintext/Markdown body while retaining the email escaping, and add a bulk-hook-to-API regression test. The review-only native case fails at the queued-body assertion; its 44 existing cases pass.

The email fix itself withstands the injection review: escaping precedes the HTML payload, and the backend stores/queues that same body. Preview preserves literal script/event-handler markup, entities, ampersands, angle brackets and line breaks. Intentional rich HTML remains rich; single reply/compose use the existing sanitizer. The server bulk endpoint keeps its HTML caller contract; no additional plaintext caller was found. No locale copy changed; EN/DE/FR summary cases pass. All 46 other open PR register diffs were inspected: none touches the insertion anchor.

Validation: 88 UI cases across six files (including a review-only fixture with four added adversarial cases and replayed summary cases), 44 committed native send cases and five real Chromium preview cases pass, with one worker and /opt/node/bin/node. Scoped type-aware oxlint and oxfmt pass for all six changed code files; the platform manual-register validator passes. The new helper type-checks cleanly. The scoped TypeScript import closure has zero changed-file diagnostics, but is blocked by the installed environment’s missing eventsource-parser and resulting imported diagnostics; the watchdog suite is blocked by the same missing dependency. Review-only server.fs.allow resolved the font restriction. The existing Vitest/browser version warning is retained. No dependencies installed or provider delivery attempted.

CI has not settled: at 19:51Z, Build/Commitlint/SAST jobs remain queued and the other four current-head workflows remain pending. No CI failure has been observed, so none is attributed to this patch or to main’s use-compact-age issue. Main’s fix ac7f3f55 is absent from this exact head. This is a source-level request-changes verdict, not a green-CI acceptance.

Fallback report for TALE-336 and TALE-359 (3729d02a-eb44-42b2-a68e-1bec2cebbcd6): workspace_status returned unavailable/not_granted, preventing task comments. Evidence and review-only reproductions are retained in TALE-907’s delivery box. No push, merge, CI rerun, or task/status change.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

TALE-913 repair report for TALE-336 and TALE-359 (3729d02a-eb44-42b2-a68e-1bec2cebbcd6), for C8 re-review.

Updated PR #4389 with fast-forward commit 638e2527703a4710aa61d5f6618770b493144502 on fix/bulk-inbox-plaintext. Confirmed its original head was exactly 6516048d462879ebf269e3d85fe35b89f48df16e before editing and again before commit. No main merge or force-push.

Bulk replies now send the original trimmed textarea body through the existing sourceMarkdown field alongside escaped email HTML. replyToConversation uses the original body for API conversations, preserving ampersands, line breaks, angle brackets and literal injection text; email still stores and queues escaped HTML. The regression exercises the bulk hook, real mutation hook/client adapter, HTTP payload, reply door, API queue boundary and email job boundary. Removing the fix makes the regression fail. Updated the automation coverage register.

Verification (existing dependencies; /opt/node/bin/node; one test worker):

  • Server: send.bulk-reply.test.tsx, send.test.ts, app/lib/backend/conversations.test.ts — 55 tests passed.
  • UI: bulk hook, send-summary and bulk-send-dialog tests — 50 tests passed, including the regression before it was moved into the server suite.
  • Home Inbox: home-inbox-list.test.tsx and home-inbox-list.wire.test.tsx — 22 tests passed.
  • Scoped type-aware oxlint — passed for all changed TypeScript files.
  • Scoped TypeScript semantic/syntactic checks — 4 files, zero errors (changed TypeScript files and use-inbox-list importer).
  • Scoped oxfmt and git diff --check — passed. Markdown is intentionally excluded by the repository formatter.
  • Manual-register gate — passed: 5 trees, 1471 boxes.
  • Review-only Vite server.fs.allow config admitted the shared font asset; repository config unchanged.

CI is pending. After push, both gh pr checks --watch and the subsequent snapshot reported no checks yet registered for the new head; earlier-head Actions checks were queued. No workflows were rerun. C8 re-review and CI completion remain open.

Reporting fallback: workspace_status returned unavailable/not_granted, so direct TALE-336/TALE-359 task comments could not be posted. This report is posted to PR #4389 for both cards.

Deliverables: repair.patch, this report, the planning note and scoped verification helpers in the task delivery directory. The isolated checkout was disposable; the pushed commit and patch preserve the source change.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

Independent security re-review (Codex #8 / TALE-920), exact head 638e252: PASS within the requested scope; TALE-907's P2 is repaired. No new blocking security finding. CI remains pending.

For TALE-336 and TALE-359 (3729d02a-eb44-42b2-a68e-1bec2cebbcd6): this PR comment is the authorized fallback because the workspace tool returned workspace_status unavailable / not_granted, preventing task-comment access.

  • Confirmed the remote head before review and again before posting. git range-diff from the common base shows the previous two commits unchanged and only the added repair commit 638e25277 after 6516048d4 (four changed files).
  • Bulk replies now send sourceMarkdown: body alongside content: plaintextToEmailHtml(body). The adapter and validated reply route forward the source string. The API branch selects it with ??, so plaintext and Markdown, literal &, newlines and angle brackets bypass the lossy tag-stripping fallback. queueApiReply persists that exact body into the API delivery record. This repairs the bulk path without changing the legacy fallback for callers that omit sourceMarkdown.
  • Email still escapes using he.escape BEFORE inserting <p>/<br> markup. splitHtmlText retains that escaped HTML; the backend stores args.content and queues the same HTML as the job body with contentType: 'HTML'. The source Markdown does not replace the email job body.
  • The new hook-to-backend regression exercises API and email selections with A&B.\nSecond line <price> <script>alert("x")</script>. It checks the exact original API queue body, escaped request payload, stored email body and queued email HTML. Existing real-browser tests also verify literal injection text, line breaks and intentional rich HTML.

Local verification, existing dependencies only:

  • /opt/node/bin/node Vitest, one worker: four targeted files, 87/87 tests passed (send.bulk-reply, send, use-bulk-actions, use-bulk-actions.send-summary).
  • Real Chromium browser regression: 5/5 passed. Review-only config outside the clone uses server.fs.allow, the already installed Chromium 1194 executable, and scoped dependency prebundling with React/testing-library included. No dependency installation or source edits. Installed Vitest/browser versions differ (4.1.11/4.1.10); the successful final run is recorded with that warning.
  • Scoped type-aware oxlint: 7 files, 226 rules, zero diagnostics, one thread. Review-only lint config removes the shared-helper ignore so it is checked too.
  • oxfmt: all seven changed TypeScript files pass; the repository formatter does not select the Markdown register.
  • Scoped TypeScript compiler diagnostics with the repository options and imported dependencies: 7 changed files, zero diagnostics, 2 GiB heap; no whole-workspace check.
  • Additional adapter/route/API-sync checks: 10 tests passed in the adapter wire suite; three suites blocked at import/setup by missing eventsource-parser in the existing installation. This is retained as a local verification limitation, not reported green.

CI: pending, per GitHub Actions incident 3q1yb5m7ltvb. Read-only check-runs query for this exact head returned zero attached check runs. No rerun, cancellation, push, merge or status change performed.

@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney removed this pull request from the merge queue due to the queue being cleared Oct 9, 2026
@yannickmonney
yannickmonney force-pushed the fix/bulk-inbox-plaintext branch from 638e252 to 30dbe80 Compare October 9, 2026 04:29
@yannickmonney
yannickmonney force-pushed the fix/bulk-inbox-plaintext branch 2 times, most recently from 71ceecc to 7849ac2 Compare October 9, 2026 14:08
@yannickmonney
yannickmonney force-pushed the fix/bulk-inbox-plaintext branch from 7849ac2 to 071beb4 Compare October 9, 2026 15:03
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney merged commit 474e6e9 into main Oct 9, 2026
64 checks passed
@yannickmonney
yannickmonney deleted the fix/bulk-inbox-plaintext branch October 9, 2026 22:18
yannickmonney added a commit that referenced this pull request Oct 11, 2026
Bulk Send refused every conversation without a contact email before
dispatch, although the single reply and the reply door answer a
conversation mirrored over the REST API through its source, without an
address. The three copies of that rule now share one,
hasReplyRecipient in lib/shared/conversations/reply-recipient.ts, used
by the reply door, the Inbox row projection, the single reply and the
bulk send.

A refused bulk send closed its dialog and cleared the selection, so the
message and the failed recipients were lost, and selecting the same
conversations again re-sent to those that already had the message. The
summary toast stays as it was. On a refusal the dialog now stays open
with the message, names each refused conversation and why, and returns
focus to the message. Once something went out, only the refused
conversations stay selected, so Send tries those alone; when nothing
went out, the selection stays as it was. A full success still closes
the dialog and clears the selection.

The plain-text escaping from #4389 is kept and is now also covered for
an API conversation without an address.

Closes #3912
Closes #3924
Refs #3920
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.

Bug: bulk Inbox messages interpret plain text as HTML

1 participant