Skip to content

fix(duckduckgo): decode & last in decodeHtmlEntities to avoid double-decoding#96348

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
ly-wang19:fix/ddg-decode-amp-last
Jun 24, 2026
Merged

fix(duckduckgo): decode & last in decodeHtmlEntities to avoid double-decoding#96348
vincentkoc merged 2 commits into
openclaw:mainfrom
ly-wang19:fix/ddg-decode-amp-last

Conversation

@ly-wang19

@ly-wang19 ly-wang19 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

decodeHtmlEntities in extensions/duckduckgo/src/ddg-client.ts decodes &amp; first when un-escaping DuckDuckGo result HTML. When result text literally contains an HTML entity — e.g. a page titled How to escape &lt; in HTML — DuckDuckGo returns it double-encoded as &amp;lt;. Decoding &amp; first un-escapes it to &lt;, which a later pass then re-decodes into a real <, corrupting the text:

Input : "How to escape &amp;lt; in HTML"
Before: "How to escape < in HTML"     ❌ (fabricates a tag the page never had)
After : "How to escape &lt; in HTML" ✅

decodeHtmlEntities runs on every result's title, url, and snippet (ddg-client.ts:104-106), which feed runDuckDuckGoSearch — the execute path of the DuckDuckGo web-search tool (ddg-search-provider.ts). So the corrupted text becomes tool output the model consumes and may surface to the user.

Fix

Decode &amp; last, matching the established convention everywhere else in this codebase — extensions/msteams/src/inbound.ts, src/agents/openai-transport-stream.ts, src/daemon/launchd-plist.ts, and src/commands/doctor-session-snapshots.ts all decode &amp; last (msteams/inbound.ts even documents "must be last to prevent double-decoding"). The DDG decoder was the lone outlier. Behavior-preserving for all singly-encoded input.

Evidence

Added a regression test (does not double-decode escaped entities) in extensions/duckduckgo/src/ddg-search-provider.test.ts, and verified red → green by executing the exact decodeHtmlEntities chain against the bug cases and the existing test:

[literal entity in a result — the bug]
  input : "How to escape &amp;lt; in HTML"
  before: "How to escape < in HTML"        <-- RED  (current code on main)
  after : "How to escape &lt; in HTML"     <-- GREEN (this PR)

[literal numeric entity — the bug]
  input : "a&amp;#39;b"
  before: "a'b"                            <-- RED
  after : "a&#39;b"                         <-- GREEN

[existing decode test — must stay green]
  input : "Fish &amp; Chips&nbsp;&hellip; &#39;ok&#39;"
  before: "Fish & Chips ... 'ok'"   (unchanged)
  after : "Fish & Chips ... 'ok'"   (unchanged)

The reorder only affects the buggy double-decode path; every singly-encoded input (everything the existing tests exercise) is unchanged.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open: current main and latest release still have the DuckDuckGo amp-first decoder, while this PR fixes a real user-visible result-text corruption with no blocking review findings.

Canonical path: Close this PR as superseded by #39.

So I’m closing this here and keeping the remaining discussion on #39.

Review details

Best possible solution:

Close this PR as superseded by #39.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main and v2026.6.10 decode &amp; before later DuckDuckGo entity replacements, and a focused probe shows &amp;lt; becomes < before this PR.

Is this the best way to solve the issue?

Yes. The best fix is plugin-local: repair decodeHtmlEntities where DuckDuckGo title, URL, and snippet normalization already happens; a shared decoder or core change would be broader than needed.

Security review:

Security review cleared: The diff only changes local string decoding and a focused test, with no dependency, lockfile, CI, secret, install, or code-execution surface change.

AGENTS.md: found and applied where relevant.

What I checked:

  • linked superseding PR: Fix CI #39 (Fix CI) is merged at 2025-12-20T19:32:04Z.
  • cluster evidence: the durable review links that PR in the work cluster or recommended risk path.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • vincentkoc: Merged feat(web-search): add DuckDuckGo bundled plugin #52629 introduced the DuckDuckGo bundled plugin and client, and the PR head also contains a maintainer follow-up commit by this account. (role: introduced behavior and recent follow-up owner; confidence: high; commits: c6ca11e5a5d0, 2f8d32c5cd5c; files: extensions/duckduckgo/src/ddg-client.ts, extensions/duckduckgo/src/ddg-search-provider.ts, extensions/duckduckgo/src/ddg-search-provider.test.ts)
  • steipete: Recent DuckDuckGo-area history includes provider runtime caching and search helper test consolidation touching the provider/test surface. (role: recent adjacent contributor; confidence: medium; commits: a7e029fde9be, f7de5c3b837c, af62a2c2e46b; files: extensions/duckduckgo/src/ddg-search-provider.ts, extensions/duckduckgo/src/ddg-search-provider.test.ts)
  • Gustavo Madeira Santana: Authored the DuckDuckGo web-search runtime lazy-load refactor that touched the provider entry path used by this client. (role: recent adjacent contributor; confidence: medium; commits: c9dfb190016b; files: extensions/duckduckgo/src/ddg-search-provider.ts)

Codex review notes: model internal, reasoning high; reviewed against 2aa9d6763564.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jun 24, 2026
ly-wang19 and others added 2 commits June 24, 2026 21:18
@ly-wang19 @claude @vincentkoc
fix(duckduckgo): decode &amp; last in decodeHtmlEntities to avoid dou…
7285a97 
…ble-decoding

decodeHtmlEntities decoded &amp; FIRST, so result text that literally contains
an entity (e.g. a page title 'How to escape &lt; in HTML', which DuckDuckGo
returns double-encoded as '&amp;lt;') was re-decoded into markup: '&amp;lt;'
became '<' instead of the literal '&lt;', corrupting the titles, snippets, and
URLs the web-search tool returns to the model.

Reorder so &amp; is decoded last, matching the established convention elsewhere
in the codebase (msteams/inbound.ts, openai-transport-stream.ts,
launchd-plist.ts, doctor-session-snapshots.ts all decode &amp; last).
Behavior-preserving for all singly-encoded input.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@vincentkoc
fix(duckduckgo): decode html entities in one pass
2f8d32c
@vincentkoc
vincentkoc force-pushed the fix/ddg-decode-amp-last branch from 281b928 to 2f8d32c Compare June 24, 2026 13:18
@vincentkoc
vincentkoc merged commit f0be8e7 into openclaw:main Jun 24, 2026
85 checks passed
@vincentkoc

vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
Open
This was referenced Jun 24, 2026
Closed
Merged
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 25, 2026
@ly-wang19 @claude @vincentkoc
fix(duckduckgo): decode &amp; last in decodeHtmlEntities to avoid dou…
3e62a92 
…ble-decoding (openclaw#96348)

* fix(duckduckgo): decode &amp; last in decodeHtmlEntities to avoid double-decoding

decodeHtmlEntities decoded &amp; FIRST, so result text that literally contains
an entity (e.g. a page title 'How to escape &lt; in HTML', which DuckDuckGo
returns double-encoded as '&amp;lt;') was re-decoded into markup: '&amp;lt;'
became '<' instead of the literal '&lt;', corrupting the titles, snippets, and
URLs the web-search tool returns to the model.

Reorder so &amp; is decoded last, matching the established convention elsewhere
in the codebase (msteams/inbound.ts, openai-transport-stream.ts,
launchd-plist.ts, doctor-session-snapshots.ts all decode &amp; last).
Behavior-preserving for all singly-encoded input.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* fix(duckduckgo): decode html entities in one pass

---------

Co-authored-by: ly-wang19 <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Vincent Koc <[email protected]>
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@ly-wang19 @claude
@vincentkoc @QiuYuang
fix(duckduckgo): decode &amp; last in decodeHtmlEntities to avoid dou…
d902505 
…ble-decoding (openclaw#96348)

* fix(duckduckgo): decode &amp; last in decodeHtmlEntities to avoid double-decoding

decodeHtmlEntities decoded &amp; FIRST, so result text that literally contains
an entity (e.g. a page title 'How to escape &lt; in HTML', which DuckDuckGo
returns double-encoded as '&amp;lt;') was re-decoded into markup: '&amp;lt;'
became '<' instead of the literal '&lt;', corrupting the titles, snippets, and
URLs the web-search tool returns to the model.

Reorder so &amp; is decoded last, matching the established convention elsewhere
in the codebase (msteams/inbound.ts, openai-transport-stream.ts,
launchd-plist.ts, doctor-session-snapshots.ts all decode &amp; last).
Behavior-preserving for all singly-encoded input.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* fix(duckduckgo): decode html entities in one pass

---------

Co-authored-by: ly-wang19 <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Vincent Koc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: duckduckgo P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ly-wang19 @vincentkoc