fix(media): transcript echo mangles $ sequences in transcribed audio#100181
fix(media): transcript echo mangles $ sequences in transcribed audio#100181UditDewan wants to merge 2 commits into
Conversation
sendTranscriptEcho and the Matrix preflight echo insert the transcript into
the echo format via String.prototype.replace with a plain string replacement,
so JS substitution patterns in the transcribed speech are interpreted instead
of echoed literally: $$ collapses to $, $& duplicates the {transcript}
placeholder, and $` / $' splice in surrounding format text.
Use a function replacer so the transcript is always inserted verbatim.
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 1:54 AM ET / 05:54 UTC. Summary PR surface: Source +4, Tests +32. Total +36 across 4 files. Reproducibility: yes. Current main has direct string replacement in both echo formatters, and a local Node check reproduces the reported replacement-token behavior for $$ and $&. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this narrow formatter fix once the current-head CI and normal maintainer merge gates are satisfied. Do we have a high-confidence way to reproduce the issue? Yes. Current main has direct string replacement in both echo formatters, and a local Node check reproduces the reported replacement-token behavior for $$ and $&. Is this the best way to solve the issue? Yes. A function replacer is the narrowest maintainable fix because it preserves first-placeholder replacement semantics while preventing JavaScript replacement-token parsing of transcript text. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3cf0d00d90f9. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +4, Tests +32. Total +36 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
proof: sufficient
|
Thank you for the contribution. This fix landed with contributor co-authorship preserved in #100258 ( I consolidated the source fixes into a maintainer takeover because the contributor branches were based on rewritten pre- |
2 participants
What Problem This Solves
Fixes an issue where users with
tools.media.audio.echoTranscriptenabled receive a corrupted transcript echo when the transcribed speech contains$sequences. Sending a voice note transcribed astickets cost $$40, confirm with $&gets echoed back as:$$collapses to$,$&is replaced with the{transcript}placeholder itself, and$`/$'splice in surrounding text from the echo format. The same defect exists in the Matrix plugin's preflight audio transcript echo.Why This Change Was Made
Both echo paths insert the transcript into the format template via
String.prototype.replace(pattern, string), which interprets JS substitution patterns ($$,$&,$`,$') in the replacement string. Switching to a function replacer (.replace("{transcript}", () => transcript)) inserts the transcript verbatim while keeping the existing replace-first-occurrence semantics. No config, API, or default changes.Both sibling surfaces with this pattern are fixed in this PR (
src/media-understanding/echo-transcript.tsandextensions/matrix/src/matrix/monitor/preflight-audio.ts); a repo-wide search for{transcript}replacement sites found no others.User Impact
Voice-note transcript echoes now always show exactly what was transcribed, including dollar amounts and
$sequences, on all channels using the shared echo path and on Matrix preflight audio.Evidence
Repro of the defect (Node REPL, before vs. after):
Regression tests added next to the existing echo tests in both files ("keeps dollar sequences in the transcript literal"). Verified they fail against the unfixed code and pass with the fix:
npx vitest run src/media-understanding/echo-transcript.test.ts→ 8 passed (with fix); the new test fails withAssertionErrorwhen the fix is stashed.npx vitest run extensions/matrix/src/matrix/monitor/preflight-audio.test.ts→ 9 passed.pnpm test:extension matrix: the touched test file passes; the lane reports failures only in unrelated matrix auth/crypto suites (e.g.legacy-crypto-restore.test.ts) that this diff does not touch and that correspond to lanes currently failing onmainCI.oxlinton the four touched files: clean.This PR is AI-assisted.